From ab921791992c1a679e77e3d8eec77495b1039b6d Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Thu, 2 Jul 2026 23:33:21 -0300 Subject: [PATCH] fix(dashboard): render Update-now API errors as text, not the raw envelope object (#5991) Clicking 'Update now' could crash the page with 'Internal Server Error' (Minified React error #31). The handler POSTs the loopback-only /api/system/version endpoint and, on a non-OK JSON response (e.g. a 403 when the dashboard is reached via a reverse proxy), passed the raw error envelope { error: { code, message, correlation_id } } to notify.error(), which rendered the object as a React child and threw #31. Funnel the body through extractApiErrorMessage() (the #5340 helper) so a string always reaches the toast. Regression guard: tests/unit/ui/home-update-error-render-5991.test.ts (3/3 pass with the fix, 3/3 fail on the pre-fix source). --- CHANGELOG.md | 2 +- .../(dashboard)/dashboard/HomePageClient.tsx | 20 ++++---- .../ui/home-update-error-render-5991.test.ts | 48 +++++++++++++++++++ 3 files changed, 60 insertions(+), 10 deletions(-) create mode 100644 tests/unit/ui/home-update-error-render-5991.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 17506c549e2..768e191abf5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ ### 🔧 Bug Fixes -_TBD_ +- **dashboard ("Update now" → Internal Server Error):** clicking **Update now** on the dashboard home could crash the page with a blank "Internal Server Error" screen (`Minified React error #31`). The handler POSTs the loopback-only `/api/system/version` auto-update endpoint and, on a non-OK JSON response (e.g. a `403` when the dashboard is reached through a reverse proxy / non-loopback origin), passed the raw error envelope object `{ error: { code, message, correlation_id } }` straight to `notify.error()`, which rendered the object as a React child and threw #31. The update-error path now funnels the body through `extractApiErrorMessage()` (the same safe extractor added in #5340), so a readable string always reaches the toast. Regression guard: `tests/unit/ui/home-update-error-render-5991.test.ts`. ([#5991](https://github.com/diegosouzapw/OmniRoute/issues/5991)) ### 📝 Maintenance diff --git a/src/app/(dashboard)/dashboard/HomePageClient.tsx b/src/app/(dashboard)/dashboard/HomePageClient.tsx index 017d14de00c..37db2baa6b5 100644 --- a/src/app/(dashboard)/dashboard/HomePageClient.tsx +++ b/src/app/(dashboard)/dashboard/HomePageClient.tsx @@ -10,6 +10,7 @@ import { Card, CardSkeleton, Button, Modal } from "@/shared/components"; import ProviderIcon from "@/shared/components/ProviderIcon"; import { AI_PROVIDERS, NOAUTH_PROVIDERS, OAUTH_PROVIDERS } from "@/shared/constants/providers"; import { useNotificationStore } from "@/store/notificationStore"; +import { extractApiErrorMessage } from "@/shared/http/apiErrorMessage"; import { copyToClipboard } from "@/shared/utils/clipboard"; import { getProviderDisplayLabel } from "@/shared/utils/providerDisplayLabel"; import { useIsElectron, useOpenExternal } from "@/shared/hooks/useElectron"; @@ -161,13 +162,7 @@ export default function HomePageClient({ machineId }: HomePageClientProps) { // Electron internal auto-updater state and listeners const [electronUpdateStatus, setElectronUpdateStatus] = useState<{ status: - | "idle" - | "checking" - | "available" - | "not-available" - | "downloading" - | "downloaded" - | "error"; + "idle" | "checking" | "available" | "not-available" | "downloading" | "downloaded" | "error"; version?: string; percent?: number; message?: string; @@ -685,7 +680,11 @@ export default function HomePageClient({ machineId }: HomePageClientProps) { if (contentType.includes("application/json")) { const data = await res.json(); if (!res.ok || !data.success) { - notify.error(data.error || "Failed to start update."); + // #5991: the error envelope is `{ error: { code, message, correlation_id } }`. + // Passing the raw object to notify.error() rendered it as a React child → + // "Minified React error #31" crash ("Internal Server Error" screen), e.g. on + // the 403 from the loopback-only /api/system/version. Extract the string. + notify.error(extractApiErrorMessage(data, "Failed to start update.")); setUpdating(false); setUpdatePhase("idle"); return; @@ -1109,7 +1108,10 @@ export default function HomePageClient({ machineId }: HomePageClientProps) {

{t.rich("step1Desc", { endpoint: (chunks) => ( - + {chunks} ), diff --git a/tests/unit/ui/home-update-error-render-5991.test.ts b/tests/unit/ui/home-update-error-render-5991.test.ts new file mode 100644 index 00000000000..fc598c01387 --- /dev/null +++ b/tests/unit/ui/home-update-error-render-5991.test.ts @@ -0,0 +1,48 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { dirname, resolve } from "node:path"; + +// Regression guard for #5991 — clicking "Update now" showed an "Internal Server +// Error" screen (Minified React error #31). The handler POSTs /api/system/version +// (a loopback-only auto-update endpoint) and, on a non-OK JSON response, did: +// notify.error(data.error || "Failed to start update."); +// OmniRoute's error envelope is `{ error: { code, message, correlation_id } }`, so +// `data.error` is an OBJECT. notify.error rendered that object as a React child → +// React #31 crash. The fix funnels the body through extractApiErrorMessage() (the +// same helper introduced in #5340) so a string always reaches the toast. + +const here = dirname(fileURLToPath(import.meta.url)); +const source = readFileSync( + resolve(here, "../../../src/app/(dashboard)/dashboard/HomePageClient.tsx"), + "utf8" +); + +test("HomePageClient imports the safe API error extractor", () => { + assert.match( + source, + /import\s*\{\s*extractApiErrorMessage\s*\}\s*from\s*["']@\/shared\/http\/apiErrorMessage["']/, + "HomePageClient must import extractApiErrorMessage to render API errors safely (#5991)" + ); +}); + +test("the update-error handler funnels the body through extractApiErrorMessage (#5991)", () => { + // The update failure path must extract a string, not hand the raw envelope object + // (which triggers React #31) to notify.error. + assert.match( + source, + /notify\.error\(\s*extractApiErrorMessage\(\s*data\s*,/, + "the update-error notify.error must use extractApiErrorMessage(data, …) (#5991)" + ); +}); + +test("the update-error handler never passes the raw error object to notify.error (#5991)", () => { + // The pre-fix pattern `notify.error(data.error || …)` rendered an object as a React + // child. It must not come back. + assert.doesNotMatch( + source, + /notify\.error\(\s*data\.error\b/, + "notify.error(data.error …) renders the error envelope object as a React child → React #31 (#5991)" + ); +});