fix(frontend): keep non-string API error bodies out of React children

frontend/src/lib/api-error.test.ts

@@ -0,0 +1,30 @@
+import { ApiError } from 'lib/api-error'
+
+describe('ApiError', () => {
+    it('keeps string fields from the response body', () => {
+        const error = new ApiError('failed', 400, undefined, {
+            detail: 'Enter a valid email address.',
+            code: 'invalid',
+            attr: 'email',
+            statusText: 'Bad Request',
+            link: 'https://example.com',
+        })
+
+        expect(error.detail).toBe('Enter a valid email address.')
+        expect(error.code).toBe('invalid')
+        expect(error.attr).toBe('email')
+        expect(error.statusText).toBe('Bad Request')
+        expect(error.link).toBe('https://example.com')
+    })
+
+    it.each([
+        ['nested field errors', { email: ['Enter a valid email address.'] }],
+        ['a list', ['Enter a valid email address.']],
+        ['a number', 42],
+    ])('drops a non-string detail (%s) so it cannot reach a React child', (_name, detail) => {
+        const error = new ApiError('failed', 400, undefined, { detail })
+
+        expect(error.detail).toBeNull()
+        expect(error.data.detail).toEqual(detail)
+    })
+})

frontend/src/lib/api-error.ts

@@ -6,6 +6,10 @@ export function isAccessDeniedError(error: { status?: number; code?: string | nu
     return error.status === 403 && error.code === 'permission_denied'
 }
 
+function asString(value: unknown): string | null {
+    return typeof value === 'string' && value ? value : null
+}
+
 export class ApiError extends Error {
     /** Django REST Framework `detail` - used in downstream error handling. */
     detail: string | null
@@ -27,11 +31,13 @@ export class ApiError extends Error {
     ) {
         message = message || `API request failed with status: ${status ?? 'unknown'}`
         super(message)
-        this.statusText = data?.statusText || null
-        this.detail = data?.detail || null
-        this.code = data?.code || null
-        this.link = data?.link || null
-        this.attr = data?.attr || null
+        // Only keep string values: these fields are declared `string | null` and consumers render them
+        // directly into JSX, so a non-string body value would crash React. The raw body stays on `data`.
+        this.statusText = asString(data?.statusText)
+        this.detail = asString(data?.detail)
+        this.code = asString(data?.code)
+        this.link = asString(data?.link)
+        this.attr = asString(data?.attr)
     }
 
     /**

frontend/src/queries/nodes/InsightViz/utils.ts

@@ -202,8 +202,12 @@ export const extractValidationError = (error: Error | Record<string, any> | null
     if (hasValidationErrorStatus(error)) {
         // Async queries put the error message on data.error_message, while synchronous ones use detail
         const anyError = error as Record<string, any>
+        const message = anyError.detail || anyError.data?.error_message
+        if (typeof message !== 'string') {
+            return null
+        }
         // Add unbreakable space for better line breaking
-        return (anyError.detail || anyError.data?.error_message)?.replace('Try ', 'Try\u00A0') ?? null
+        return message.replace('Try ', 'Try\u00A0')
     }
 
     return null

frontend/src/queries/query.test.ts

@@ -307,7 +307,8 @@ describe('query', () => {
             })
         })
 
-        it('handles non-string error message', async () => {
+        // A non-string detail used to pass straight through and crash React when rendered
+        it('drops a non-string error message', async () => {
             jest.spyOn(api.queryStatus, 'get').mockRejectedValueOnce({
                 data: {
                     query_status: {
@@ -317,7 +318,7 @@ describe('query', () => {
             })
 
             await expect(pollForResults('test-query-id')).rejects.toMatchObject({
-                detail: { nested: 'object' },
+                detail: '',
             })
         })
     })

frontend/src/queries/query.ts

@@ -72,9 +72,10 @@ export const QUERY_TIMEOUT_ERROR_MESSAGE = 'Query timed out'
  * This function safely extracts the message and code, falling back to the
  * original string if parsing fails.
  */
-export function parseErrorMessage(errorMessage: string | undefined): { message: string; code: string | null } {
+export function parseErrorMessage(errorMessage: string | null | undefined): { message: string; code: string | null } {
     if (!errorMessage || typeof errorMessage !== 'string') {
-        return { message: errorMessage || '', code: null }
+        // Never hand back a non-string: callers render this straight into JSX.
+        return { message: typeof errorMessage === 'string' ? errorMessage : '', code: null }
     }
 
     // Try to match list format: [ErrorDetail(string='...', code='...')]

frontend/src/scenes/experiments/MetricsView/new/MetricErrorState.tsx

@@ -12,7 +12,7 @@ import { ExperimentMetric } from '~/queries/schema/schema-general'
 
 type MetricErrorStateProps = {
     error: {
-        detail: string | object | any
+        detail?: string | null
         statusCode?: number
         queryId?: string
     }

frontend/src/scenes/experiments/legacy/metricsView/LegacyErrorChecklist.tsx

@@ -169,5 +169,5 @@ export function LegacyErrorChecklist({ error, metric }: { error: any; metric: an
     }
 
     // Other unexpected errors
-    return <div>{error.detail}</div>
+    return <div>{typeof error.detail === 'string' ? error.detail : 'An unexpected error occurred'}</div>
 }

frontend/src/scenes/insights/EmptyStates/EmptyStates.tsx

@@ -2,7 +2,7 @@ import './EmptyStates.scss'
 
 import clsx from 'clsx'
 import { useActions, useValues } from 'kea'
-import { useEffect, useState } from 'react'
+import { isValidElement, useEffect, useState } from 'react'
 import { TextMorph } from 'torph/react'
 
 import * as construction2Png from '@posthog/brand/hoggies/png/construction-2'
@@ -699,6 +699,9 @@ export function InsightErrorState({
         </Link>
     )
 
+    // `title` is often fed straight from an API error body, so fall back rather than trust the declared type
+    const renderableTitle = typeof title === 'string' || isValidElement(title) ? title : null
+
     return (
         <div
             data-attr="insight-empty-state"
@@ -709,7 +712,7 @@ export function InsightErrorState({
             <h2 className="text-xl text-danger leading-tight mb-6" data-attr="insight-loading-too-long">
                 {/* Note that this default phrasing signals the issue is intermittent, */}
                 {/* and that perhaps the query will complete on retry */}
-                {title || <span>There was a problem completing this query</span>}
+                {renderableTitle || <span>There was a problem completing this query</span>}
             </h2>
 
             {!excludeDetail && !supportOnly && (

frontend/src/scenes/onboarding/shared/wizard-sync/InstallationProgressView.tsx

@@ -273,7 +273,7 @@ export function InstallationProgressContent({
                     </div>
                 )}
 
-            {phase === 'error' && error?.detail && (
+            {phase === 'error' && typeof error?.detail === 'string' && (
                 <div className="text-sm text-danger bg-danger-highlight rounded p-2">{error.detail}</div>
             )}