From cd067ad58473b6c08d7ccfa56d1ea20066164c13 Mon Sep 17 00:00:00 2001 From: Xavier Karma Date: Tue, 6 Oct 2026 09:35:10 +0530 Subject: [PATCH] Fix the blank window after an export Every successful export left the window blank and unresponsive, so the app had to be killed. The "Saved to ..." notice (and the export-failed notice) held buttons inside a Carbon InlineNotification. Carbon throws "component should have no interactive child nodes" in an effect, and an uncaught throw makes React unmount the whole tree. Found by driving the real debug binary through WebKitWebDriver: the page body was empty and the error listener showed that throw. - The buttons now sit beside the notice, and the failed-export notice uses ActionableNotification's own Retry action. - ErrorBoundary around the app (reload screen instead of a blank window) and around each export notice. - A lint test that fails if a Carbon notification wraps an interactive element. - The real-webview self-test mounts the saved and failed notices and fails on an uncaught React error, and issues and exports a second invoice after the first. Claude-Session: https://claude.ai/code/session_01PZypiWDfMkDTeEPeXjRhW5 --- src/components/ErrorBoundary.tsx | 51 ++++++++++++++++++ src/components/ExportFeedback.tsx | 41 ++++++++------- src/components/notifications.lint.test.ts | 63 +++++++++++++++++++++++ src/lib/selfTestE2e.ts | 13 +++++ src/lib/selfTestUi.tsx | 59 +++++++++++++++++++++ src/main.tsx | 5 +- src/views/InvoiceDetail.tsx | 5 +- src/views/InvoiceHistory.tsx | 5 +- src/views/NewInvoice.tsx | 5 +- 9 files changed, 225 insertions(+), 22 deletions(-) create mode 100644 src/components/ErrorBoundary.tsx create mode 100644 src/components/notifications.lint.test.ts create mode 100644 src/lib/selfTestUi.tsx diff --git a/src/components/ErrorBoundary.tsx b/src/components/ErrorBoundary.tsx new file mode 100644 index 0000000..9414f46 --- /dev/null +++ b/src/components/ErrorBoundary.tsx @@ -0,0 +1,51 @@ +import { Component, type ErrorInfo, type ReactNode } from "react"; + +interface Props { + children: ReactNode; + /** What to show instead of the children. Without it, a full-window notice with a Reload button. */ + fallback?: (error: Error, reset: () => void) => ReactNode; +} + +interface State { + error: Error | null; +} + +/** + * React unmounts the whole tree when a component throws and nothing catches it, which leaves a blank window that + * looks like a hang. This keeps the failure to the part that threw (or, at the top, shows what happened and a way back). + * Plain elements on purpose: the component library may be what threw. + */ +export default class ErrorBoundary extends Component { + state: State = { error: null }; + + static getDerivedStateFromError(error: Error): State { + return { error }; + } + + componentDidCatch(error: Error, info: ErrorInfo) { + console.error("Voiced UI error:", error, info.componentStack); + } + + reset = () => this.setState({ error: null }); + + render() { + const { error } = this.state; + if (!error) return this.props.children; + if (this.props.fallback) return this.props.fallback(error, this.reset); + return ( +
+

Something went wrong

+

+ Voiced hit an unexpected error. Your invoices and drafts are saved. Reload to carry on. +

+ 
{error.message}
+ +
+ ); + } +} diff --git a/src/components/ExportFeedback.tsx b/src/components/ExportFeedback.tsx index 5bd3caf..710e582 100644 --- a/src/components/ExportFeedback.tsx +++ b/src/components/ExportFeedback.tsx @@ -1,5 +1,5 @@ import { useEffect, useState } from "react"; -import { Button, InlineLoading, InlineNotification } from "@carbon/react"; +import { ActionableNotification, Button, InlineLoading, InlineNotification } from "@carbon/react"; import { api } from "../lib/api"; import type { InvoiceExport } from "../hooks/useInvoiceExport"; import FlattenProgressModal from "./FlattenProgressModal"; @@ -37,13 +37,18 @@ export default function ExportFeedback({ exp }: { exp: InvoiceExport }) { {busy && !modalOpen ? : null} {error && !flattenFailed ? ( - -
- -
- + // Carbon throws if a notification has buttons inside it, and an uncaught throw blanks the whole window. + // A single action goes through ActionableNotification's own button instead. + ) : null} {result ? ( @@ -54,16 +59,16 @@ export default function ExportFeedback({ exp }: { exp: InvoiceExport }) { title={`Saved to ${result.path}`} subtitle={result.identicalToIssued === true ? "The layout is identical to the issued invoice." : undefined} onCloseButtonClick={exp.dismiss} - > -
- - -
- + /> + {/* Beside the notification, not inside it: Carbon throws on interactive children. */} +
+ + +
{result.identicalToIssued === false ? ( { + const full = path.join(dir, name); + if (statSync(full).isDirectory()) return name === "pdf" ? [] : sourceFiles(full); + return /\.tsx$/.test(name) && !/\.test\.tsx$/.test(name) ? [full] : []; + }); +} + +/** Bodies of the notifications that have children, with the line they start on. */ +function notificationBodies(source: string): Array<{ tag: string; line: number; body: string }> { + const out: Array<{ tag: string; line: number; body: string }> = []; + for (const tag of NOTIFICATIONS) { + const open = new RegExp(`<${tag}\\b`, "g"); + for (let m = open.exec(source); m; m = open.exec(source)) { + let i = m.index + m[0].length; + let braces = 0; + for (; i < source.length; i++) { + const c = source[i]; + if (c === "{") braces++; + else if (c === "}") braces--; + else if (c === ">" && braces === 0) break; + } + if (source[i - 1] === "/") continue; // self-closing: no children + const end = source.indexOf(``, i); + out.push({ tag, line: source.slice(0, m.index).split("\n").length, body: source.slice(i + 1, end < 0 ? undefined : end) }); + } + } + return out; +} + +describe("Carbon notifications", () => { + it("never wrap an interactive element", () => { + const offenders: string[] = []; + for (const file of sourceFiles(SRC)) { + for (const { tag, line, body } of notificationBodies(readFileSync(file, "utf8"))) { + if (INTERACTIVE.test(body)) offenders.push(`${path.relative(SRC, file)}:${line} <${tag}>`); + } + } + expect(offenders).toEqual([]); + }); + + it("the scan finds an offender when there is one", () => { + const bad = `\n \n`; + const fine = `\n`; + expect(notificationBodies(bad).some((n) => INTERACTIVE.test(n.body))).toBe(true); + expect(notificationBodies(fine)).toEqual([]); + }); +}); diff --git a/src/lib/selfTestE2e.ts b/src/lib/selfTestE2e.ts index f9954a8..09c48d8 100644 --- a/src/lib/selfTestE2e.ts +++ b/src/lib/selfTestE2e.ts @@ -125,6 +125,8 @@ export async function runE2eSteps(log: StepLog, reportPath: string): Promise (await import("./selfTestUi")).checkExportFeedbackRenders()); + await log.run("e2e: export flattened original", async () => { const r = await exportWith("flattened", "original", flattenedPath); return { ok: r?.path === flattenedPath, detail: `wrote ${r?.path}` }; @@ -158,6 +160,17 @@ export async function runE2eSteps(log: StepLog, reportPath: string): Promise { + const first = inv.number; + const outcome = await issueAndArchive({ ...input, notes: "Self-test invoice 2" }, prefs, deps); + const next = `${outDir}/e2e-second.pdf`; + const r = await exportInvoice({ invoice: outcome.invoice, mode: "searchable", source: "original" }, { ...deps, save: async () => next }); + return { + ok: outcome.archived && outcome.invoice.number !== first && r?.path === next, + detail: `${first}, then ${outcome.invoice.number} (archived=${outcome.archived}), exported ${r?.path}`, + }; + }); + await log.run("e2e: archiving different bytes is refused", async () => { const other = new TextEncoder().encode("%PDF-1.4\n% a different file\n"); const before = await api.readArchive(inv.id); diff --git a/src/lib/selfTestUi.tsx b/src/lib/selfTestUi.tsx new file mode 100644 index 0000000..ef9694a --- /dev/null +++ b/src/lib/selfTestUi.tsx @@ -0,0 +1,59 @@ +import { createRoot } from "react-dom/client"; +import { Theme } from "@carbon/react"; +import ExportFeedback from "../components/ExportFeedback"; +import type { InvoiceExport } from "../hooks/useInvoiceExport"; +import type { Invoice } from "./types"; + +/** + * Part of the real-webview self-test: mounts the export notices the way the app does and fails if React reports an + * uncaught error. Carbon throws when a notification has a button inside it, and that took the whole window + * down after every successful export; nothing that runs without a real DOM can see it. + */ + +const noop = () => {}; +const idle: InvoiceExport = { + busy: false, + progress: null, + request: null, + error: null, + result: null, + start: async () => null, + cancel: noop, + retry: noop, + exportSearchableInstead: noop, + dismiss: noop, +}; + +const CASES: Array<{ name: string; expect: string; exp: InvoiceExport }> = [ + { + name: "saved", + expect: "Saved to", + exp: { ...idle, result: { path: "/tmp/voiced-selftest.pdf", mode: "searchable", invoice: {} as Invoice, identicalToIssued: false, archiveError: "not archived" } }, + }, + { name: "failed", expect: "was not exported", exp: { ...idle, error: { message: "disk full", flatten: false } } }, +]; + +export async function checkExportFeedbackRenders(): Promise<{ ok: boolean; detail: string }> { + const problems: string[] = []; + for (const c of CASES) { + const host = document.createElement("div"); + document.body.appendChild(host); + const errors: string[] = []; + const root = createRoot(host, { + onUncaughtError: (e) => errors.push(e instanceof Error ? e.message : String(e)), + onRecoverableError: (e) => errors.push(e instanceof Error ? e.message : String(e)), + }); + root.render( + + + , + ); + // Effects (where Carbon checks its children) run after the first paint. + await new Promise((r) => setTimeout(r, 400)); + if (errors.length > 0) problems.push(`${c.name}: ${errors.join("; ").slice(0, 300)}`); + else if (!(host.textContent ?? "").includes(c.expect)) problems.push(`${c.name}: the notice did not render`); + root.unmount(); + host.remove(); + } + return { ok: problems.length === 0, detail: problems.join(" | ") || "the saved and failed notices render without throwing" }; +} diff --git a/src/main.tsx b/src/main.tsx index 2d0b5b0..7b564d1 100644 --- a/src/main.tsx +++ b/src/main.tsx @@ -1,12 +1,15 @@ import React from "react"; import { createRoot } from "react-dom/client"; import App from "./App"; +import ErrorBoundary from "./components/ErrorBoundary"; import "./styles/carbon.scss"; import { maybeRunSelfTest } from "./selftestBoot"; createRoot(document.getElementById("root") as HTMLElement).render( - + + + , ); diff --git a/src/views/InvoiceDetail.tsx b/src/views/InvoiceDetail.tsx index 87bd60a..e8ddc2c 100644 --- a/src/views/InvoiceDetail.tsx +++ b/src/views/InvoiceDetail.tsx @@ -44,6 +44,7 @@ import { syncTag, } from "../lib/erpnextUi"; import { useReturnFocus } from "../hooks/useReturnFocus"; +import ErrorBoundary from "../components/ErrorBoundary"; import ExportFeedback from "../components/ExportFeedback"; import PdfPreview from "../components/PdfPreview"; import RecordPaymentModal from "../components/RecordPaymentModal"; @@ -385,7 +386,9 @@ export default function InvoiceDetail({ invoiceId, settings, onBack, onDuplicate - +

The export result could not be shown ({e.message}). The PDF was still saved.

}> + +
diff --git a/src/views/InvoiceHistory.tsx b/src/views/InvoiceHistory.tsx index fcf9266..cc16f1b 100644 --- a/src/views/InvoiceHistory.tsx +++ b/src/views/InvoiceHistory.tsx @@ -49,6 +49,7 @@ import { openInErpnext } from "../lib/erpnextActions"; import type { ErpnextConfig, ErpnextSyncStatus } from "../lib/erpnext"; import { canOpenInErpnext, indexSyncStatuses, isConfigured, pushDisabledReason, sendableSelection, syncTag } from "../lib/erpnextUi"; import SendToErpnextModal, { type SendTarget } from "../components/SendToErpnextModal"; +import ErrorBoundary from "../components/ErrorBoundary"; import ExportFeedback from "../components/ExportFeedback"; import { useToast } from "../components/ToastProvider"; import type { InvoiceSummary, Settings } from "../lib/types"; @@ -226,7 +227,9 @@ export default function InvoiceHistory({ settings, active, onOpen, onOpenSetting

Invoices

Every invoice you have issued. Open one to see its PDF, record payments or export it.

- +

The export result could not be shown ({e.message}). The PDF was still saved.

}> + + {loading ? ( diff --git a/src/views/NewInvoice.tsx b/src/views/NewInvoice.tsx index 41c3bd1..2689271 100644 --- a/src/views/NewInvoice.tsx +++ b/src/views/NewInvoice.tsx @@ -61,6 +61,7 @@ import { useInvoicePdf } from "../hooks/useInvoicePdf"; import { useInvoiceExport } from "../hooks/useInvoiceExport"; import { autoPushIssued } from "../lib/erpnextUi"; import ExportButton, { ExportModePicker } from "../components/ExportButton"; +import ErrorBoundary from "../components/ErrorBoundary"; import ExportFeedback from "../components/ExportFeedback"; import { createExportDeps } from "../lib/exportDeps"; import { issueAndArchive, renderAndArchive } from "../lib/exportFlow"; @@ -784,7 +785,9 @@ export default function NewInvoice({ settings, onSettingsChange, active, onActiv onActionButtonClick={() => void retryArchive()} /> ) : null} - +

The export result could not be shown ({e.message}). The PDF was still saved.

}> + +
void exp.start(exportable, mode, "original")} disabled={busy} />