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} />