From 10fb1ddf5d014440fb4bc6ac0687442ac9914643 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8C=85=E5=91=A8=E6=B6=9B?= Date: Thu, 16 Jul 2026 04:02:34 -0700 Subject: [PATCH] =?UTF-8?q?fix(app-shell,core):=20keep=20error-envelope=20?= =?UTF-8?q?objects=20out=20of=20toast.error=20=E2=80=94=20React=20#31=20pa?= =?UTF-8?q?ge=20crash=20(#2579)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A failed api/flow/server action returning the ObjectStack envelope `{ error: { code, message } }` rode the error OBJECT through `result.error` into ActionRunner's post-execution toast, and `toast.error(object)` rendered it as a React child — Minified React error #31 inside flushSync took down the whole page (reported from Setup → Create User's 400, objectstack-ai/framework#3031). Two layers so no single handler bug can crash the page again: - sources: shared errorDetail() in useConsoleActionRuntime flattens the envelope to a string (error string → error.message → message → HTTP fallback) across apiHandler / flowHandler / serverActionHandler; - sink: ActionRunner coerces a non-string result.error to its .message (or a generic string) before calling the toast handler. --- .../useConsoleActionRuntime.test.tsx | 42 +++++++++++++++++++ .../src/hooks/useConsoleActionRuntime.tsx | 24 ++++++++--- packages/core/src/actions/ActionRunner.ts | 12 +++++- .../actions/__tests__/ActionRunner.test.ts | 32 ++++++++++++++ 4 files changed, 104 insertions(+), 6 deletions(-) diff --git a/packages/app-shell/src/hooks/__tests__/useConsoleActionRuntime.test.tsx b/packages/app-shell/src/hooks/__tests__/useConsoleActionRuntime.test.tsx index e39b6f193..531817369 100644 --- a/packages/app-shell/src/hooks/__tests__/useConsoleActionRuntime.test.tsx +++ b/packages/app-shell/src/hooks/__tests__/useConsoleActionRuntime.test.tsx @@ -143,6 +143,48 @@ describe('useConsoleActionRuntime — authenticated handlers', () => { expect(onRefresh).not.toHaveBeenCalled(); }); + it('apiHandler flattens an ObjectStack `{ error: { code, message } }` envelope to the message string', async () => { + // The admin/create-user 400 returns `{ success: false, error: { code: + // 'invalid_request', message: '...' } }`. Pre-fix the whole error OBJECT + // rode `result.error` into the ActionRunner's toast → `toast.error(object)` + // rendered it as a React child and crashed the page (React #31). + authFetchSpy.mockResolvedValue({ + ok: false, + status: 400, + json: async () => ({ + success: false, + error: { code: 'invalid_request', message: 'Provide either password or generatePassword, not both' }, + }), + }); + const { result } = renderHook(() => + useConsoleActionRuntime({ dataSource: {}, objects: [] }), + ); + + let res: any; + await act(async () => { + res = await result.current.apiHandler({ type: 'api', name: 'x', target: '/api/v1/x' } as any); + }); + + expect(res).toEqual({ + success: false, + error: 'Provide either password or generatePassword, not both', + }); + }); + + it('apiHandler falls back to an HTTP-status message when the error body is unusable', async () => { + authFetchSpy.mockResolvedValue({ ok: false, status: 500, json: async () => ({ error: {} }) }); + const { result } = renderHook(() => + useConsoleActionRuntime({ dataSource: {}, objects: [] }), + ); + + let res: any; + await act(async () => { + res = await result.current.apiHandler({ type: 'api', name: 'x', target: '/api/v1/x' } as any); + }); + + expect(res).toEqual({ success: false, error: 'HTTP 500' }); + }); + it('apiHandler turns an entitlement 403 into the upgrade dialog, not a red error toast', async () => { // Free org that already has its production env clicks "create" → the control // plane 403s with DEV_ENV_PLAN_LOCKED. The runtime must open a friendly diff --git a/packages/app-shell/src/hooks/useConsoleActionRuntime.tsx b/packages/app-shell/src/hooks/useConsoleActionRuntime.tsx index eb54f9ab6..7687e722b 100644 --- a/packages/app-shell/src/hooks/useConsoleActionRuntime.tsx +++ b/packages/app-shell/src/hooks/useConsoleActionRuntime.tsx @@ -66,6 +66,22 @@ function isRecordScoped(action: ActionDef): boolean { l === 'list_item' || l === 'record_header' || l === 'record_more' || l === 'record_section'); } +/** + * Extract a human-readable message from an error response body. The + * ObjectStack envelope nests it as `{ error: { code, message } }` — passing + * that object through as `ActionResult.error` reaches `toast.error()` as a + * React child and crashes the page (React #31). Always resolve to a string. + */ +function errorDetail(body: unknown, fallback: string): string { + const b = body as { error?: unknown; message?: unknown } | null; + const err = b?.error; + if (typeof err === 'string' && err.length > 0) return err; + const nested = (err as { message?: unknown } | null)?.message; + if (typeof nested === 'string' && nested.length > 0) return nested; + if (typeof b?.message === 'string' && b.message.length > 0) return b.message; + return fallback; +} + export interface ConsoleActionRuntimeOptions { /** Adapter for generic CRUD / execute calls. */ dataSource: any; @@ -312,8 +328,7 @@ export function useConsoleActionRuntime(opts: ConsoleActionRuntimeOptions): Cons openEntitlementDialog(entitlementSpec); return { success: false }; } - const detail = (body as any)?.error || (body as any)?.message || `HTTP ${res.status}`; - return { success: false, error: detail }; + return { success: false, error: errorDetail(body, `HTTP ${res.status}`) }; } const json = await res.json().catch(() => ({})); if (action.refreshAfter !== false) refresh(); @@ -437,8 +452,7 @@ export function useConsoleActionRuntime(opts: ConsoleActionRuntimeOptions): Cons ); const json = await res.json().catch(() => null); if (!res.ok || (json && json.success === false)) { - const errMsg = json?.error || `Flow "${flowName}" failed (HTTP ${res.status})`; - return { success: false, error: errMsg }; + return { success: false, error: errorDetail(json, `Flow "${flowName}" failed (HTTP ${res.status})`) }; } // Screen-flow runtime: paused at a `screen` node awaiting input — open // the FlowRunner to render the form + resume. Refresh happens on complete. @@ -556,7 +570,7 @@ export function useConsoleActionRuntime(opts: ConsoleActionRuntimeOptions): Cons ); const json = await res.json().catch(() => null); if (!res.ok || (json && json.success === false)) { - const errMsg = json?.error || `Action "${targetName}" failed (HTTP ${res.status})`; + const errMsg = errorDetail(json, `Action "${targetName}" failed (HTTP ${res.status})`); if (preOpenedTab) { try { preOpenedTab.close(); } catch { /* ignore */ } } // Don't toast here — the ActionRunner's post-execution hook surfaces // `error` as a toast (see apiHandler/flowHandler, which likewise only diff --git a/packages/core/src/actions/ActionRunner.ts b/packages/core/src/actions/ActionRunner.ts index 7fdf4f002..f695ab78b 100644 --- a/packages/core/src/actions/ActionRunner.ts +++ b/packages/core/src/actions/ActionRunner.ts @@ -600,7 +600,17 @@ export class ActionRunner { } if (!result.success && showToast.showOnError !== false && result.error) { - const message = action.errorMessage || result.error; + // `result.error` is typed as a string, but a handler bug can leak an + // error OBJECT (e.g. the ObjectStack `{ code, message }` envelope) + // through here — and a non-string toast payload is rendered as a React + // child, crashing the page (React #31). Coerce defensively at this + // single sink so no handler can take the whole page down. + const raw: unknown = action.errorMessage || result.error; + const message = typeof raw === 'string' + ? raw + : (raw && typeof (raw as { message?: unknown }).message === 'string') + ? (raw as { message: string }).message + : 'Action failed'; this.toastHandler(message, { type: 'error', duration }); } } diff --git a/packages/core/src/actions/__tests__/ActionRunner.test.ts b/packages/core/src/actions/__tests__/ActionRunner.test.ts index 2cc7b7aae..cce2004c1 100644 --- a/packages/core/src/actions/__tests__/ActionRunner.test.ts +++ b/packages/core/src/actions/__tests__/ActionRunner.test.ts @@ -617,6 +617,38 @@ describe('ActionRunner', () => { expect(toastHandler).toHaveBeenCalledWith('Custom error', { type: 'error', duration: undefined }); }); + it('coerces a non-string result.error to its message before toasting (React #31 guard)', async () => { + const toastHandler = vi.fn(); + runner.setToastHandler(toastHandler); + // A buggy handler leaks the ObjectStack error envelope OBJECT through + // `result.error`. Passing it to toast.error() renders it as a React + // child and crashes the page — the sink must flatten it to a string. + runner.registerHandler('leaky', vi.fn().mockResolvedValue({ + success: false, + error: { code: 'invalid_request', message: 'Provide either password or generatePassword, not both' }, + })); + + await runner.execute({ type: 'leaky' }); + + expect(toastHandler).toHaveBeenCalledWith( + 'Provide either password or generatePassword, not both', + { type: 'error', duration: undefined }, + ); + }); + + it('falls back to a generic string when a non-string result.error has no message', async () => { + const toastHandler = vi.fn(); + runner.setToastHandler(toastHandler); + runner.registerHandler('leaky', vi.fn().mockResolvedValue({ + success: false, + error: { code: 'boom' }, + })); + + await runner.execute({ type: 'leaky' }); + + expect(toastHandler).toHaveBeenCalledWith('Action failed', { type: 'error', duration: undefined }); + }); + it('should suppress toast when showOnSuccess is false', async () => { const toastHandler = vi.fn(); runner.setToastHandler(toastHandler);