diff --git a/doc/observability.md b/doc/observability.md index e4fd4d61787..7b5bef23c88 100644 --- a/doc/observability.md +++ b/doc/observability.md @@ -411,6 +411,25 @@ not inherit a previous run's identity or fingerprint. The browser sends no page URL, no referrer, no user agent, and no breadcrumb. +Application and route error-boundary reports also include: + +- `react_error_boundary`: `app` or `route`. +- `react.componentStack`: up to 40 React component names. Frame locations, + URLs, arguments, and unrecognized lines are omitted. Parsing examines at + most 16 KiB of input. Production builds preserve function names so this + trace remains useful after minification; this adds some bundle size. +- `browser_state`: document readiness, visibility, and a boolean indicating + the `translated-ltr` or `translated-rtl` root class used by browser translation. + The marker is evidence of DOM translation, not proof of the error's cause; + its absence does not exclude other translators or DOM-changing extensions. + +These fields are captured at the failure, before asynchronous reporting, and +attached only to that event. They include no component props, DOM text, HTML, +element identifiers, arbitrary CSS classes, route, or query string. Failed +diagnostic reads do not prevent the original exception from being reported. +The monitoring gate and sign-out behavior still apply. This context does not +suppress errors, change DOM operations, or disable browser translation. + ### Fail-open behavior A failed Sentry import or a failed init never stops the server and never diff --git a/ui/src/components/AppErrorBoundary.test.tsx b/ui/src/components/AppErrorBoundary.test.tsx index 285b9c595fa..ecb0d9a2dfe 100644 --- a/ui/src/components/AppErrorBoundary.test.tsx +++ b/ui/src/components/AppErrorBoundary.test.tsx @@ -8,7 +8,7 @@ import { AppErrorBoundary } from "./AppErrorBoundary"; const captureBrowserExceptionMock = vi.hoisted(() => vi.fn()); vi.mock("@/lib/sentry", () => ({ - captureBrowserException: (error: unknown) => captureBrowserExceptionMock(error), + captureBrowserException: (...args: unknown[]) => captureBrowserExceptionMock(...args), })); function BoomRender(): never { @@ -36,6 +36,7 @@ describe("AppErrorBoundary", () => { afterEach(() => { consoleErrorSpy.mockRestore(); container.remove(); + document.documentElement.classList.remove("translated-ltr"); captureBrowserExceptionMock.mockClear(); }); @@ -50,7 +51,9 @@ describe("AppErrorBoundary", () => { }); expect(captureBrowserExceptionMock).toHaveBeenCalledTimes(1); - expect(captureBrowserExceptionMock).toHaveBeenCalledWith(expect.any(Error)); + expect(captureBrowserExceptionMock).toHaveBeenCalledWith(expect.any(Error), { + boundary: "app", componentStack: expect.stringContaining("BoomRender"), + }); expect( Array.from(container.querySelectorAll("button")).some( (button) => button.textContent === "Reload page", @@ -119,4 +122,45 @@ describe("AppErrorBoundary", () => { root.unmount(); }); }); + + it("preserves the original DOM insertion error and its failing component", () => { + function BrokenDomInsertion() { + useEffect(() => { + document.createElement("div").insertBefore( + document.createElement("span"), document.createElement("span"), + ); + }, []); + return null; + } + const root = createRoot(container); + act(() => root.render()); + expect(captureBrowserExceptionMock).toHaveBeenCalledWith( + expect.objectContaining({ name: "NotFoundError" }), + { boundary: "app", componentStack: expect.stringContaining("BrokenDomInsertion") }, + ); + expect(container.textContent).toContain("Paperclip hit an error"); + act(() => root.unmount()); + }); + + it("reports the component when translation replaces a React-owned insertion anchor", () => { + function TranslatedStatus({ showIcon }: { showIcon: boolean }) { + return
{showIcon &&
; + } + const root = createRoot(container); + act(() => root.render()); + const parent = container.querySelector('[data-testid="translated-status"]')!; + // Translation replaces the text node; React still holds that old node as + // the insertion anchor when it later adds a sibling before the text. + const translated = document.createElement("font"); + translated.textContent = "Translated status"; + parent.replaceChild(translated, parent.firstChild!); + document.documentElement.classList.add("translated-ltr"); + act(() => root.render()); + expect(captureBrowserExceptionMock).toHaveBeenCalledWith( + expect.objectContaining({ name: "NotFoundError" }), + { boundary: "app", componentStack: expect.stringContaining("TranslatedStatus") }, + ); + expect(container.textContent).toContain("Reload page"); + act(() => root.unmount()); + }); }); diff --git a/ui/src/components/AppErrorBoundary.tsx b/ui/src/components/AppErrorBoundary.tsx index 68884f06141..671259edbdb 100644 --- a/ui/src/components/AppErrorBoundary.tsx +++ b/ui/src/components/AppErrorBoundary.tsx @@ -25,7 +25,7 @@ export class AppErrorBoundary extends Component<{ children: ReactNode }, AppErro override componentDidCatch(error: unknown, info: ErrorInfo): void { console.error("App shell crashed", { error, componentStack: info.componentStack }); - captureBrowserException(error); + captureBrowserException(error, { boundary: "app", componentStack: info.componentStack }); } override render() { diff --git a/ui/src/components/RouteErrorBoundary.test.tsx b/ui/src/components/RouteErrorBoundary.test.tsx index ba8f7d34cdd..6177da5f2a5 100644 --- a/ui/src/components/RouteErrorBoundary.test.tsx +++ b/ui/src/components/RouteErrorBoundary.test.tsx @@ -15,7 +15,7 @@ vi.mock("@/lib/router", () => ({ })); vi.mock("@/lib/sentry", () => ({ - captureBrowserException: (error: unknown) => captureBrowserExceptionMock(error), + captureBrowserException: (...args: unknown[]) => captureBrowserExceptionMock(...args), })); function Boom(): never { @@ -52,7 +52,9 @@ describe("RouteErrorBoundary", () => { }); expect(captureBrowserExceptionMock).toHaveBeenCalledTimes(1); - expect(captureBrowserExceptionMock).toHaveBeenCalledWith(expect.any(Error)); + expect(captureBrowserExceptionMock).toHaveBeenCalledWith(expect.any(Error), { + boundary: "route", componentStack: expect.stringContaining("Boom"), + }); expect( Array.from(container.querySelectorAll("button")).some( (button) => button.textContent === "Go back", diff --git a/ui/src/components/RouteErrorBoundary.tsx b/ui/src/components/RouteErrorBoundary.tsx index 2c2bd9403aa..5787df15561 100644 --- a/ui/src/components/RouteErrorBoundary.tsx +++ b/ui/src/components/RouteErrorBoundary.tsx @@ -22,7 +22,7 @@ class RouteErrorBoundaryInner extends Component { + vi.restoreAllMocks(); + vi.unstubAllGlobals(); + document.documentElement.className = ""; +}); + +describe("browser error context", () => { + it("keeps React component names without locations or unrecognized content", () => { + const stack = ` + at TaskDetail (https://tenant.example/assets/app.js?token=private-value:42:7) + at div () + in AppShell (created by Router) +TaskPanel@https://tenant.example/tasks/private-task:12:3 + at ErrorBoundary (/workspace/private/src/Boundary.tsx:15:9) + at https://tenant.example/private-page:17:3 +unrecognized private content`; + expect(sanitizeComponentStack(stack)).toBe( + "\n at TaskDetail\n at div\n at AppShell\n at TaskPanel\n at ErrorBoundary", + ); + }); + + it("bounds malformed and oversized component stacks", () => { + expect(sanitizeComponentStack(` at ${"a".repeat(101)} (private)`)).toBeUndefined(); + expect(sanitizeComponentStack("x".repeat(16_384) + "\n at SecretSuffix")).toBeUndefined(); + expect(sanitizeComponentStack("\n at TaskDetail (private)".repeat(100))?.match(/at TaskDetail/g)).toHaveLength(40); + }); + + it.each([undefined, null, "", "private unrecognized data"])("omits absent or unrecognized stacks: %s", (stack) => { + expect(sanitizeComponentStack(stack)).toBeUndefined(); + }); + + it.each(["translated-ltr", "translated-rtl"])("records only a translation-marker boolean for %s", (marker) => { + document.documentElement.className = `${marker} private-customer-class`; + const result = buildBrowserErrorContext({ boundary: "route", componentStack: "\n at TaskDetail" }); + expect(result).toMatchObject({ + tags: { react_error_boundary: "route" }, + contexts: { react: { componentStack: "\n at TaskDetail" }, browser_state: { translation_marker: true } }, + }); + expect(JSON.stringify(result)).not.toContain("private-customer-class"); + }); + + it("captures a snapshot rather than reading changed DOM state later", () => { + vi.spyOn(document, "visibilityState", "get").mockReturnValue("hidden"); + vi.spyOn(document, "readyState", "get").mockReturnValue("interactive"); + const result = buildBrowserErrorContext({ boundary: "app" }); + document.documentElement.classList.add("translated-ltr"); + expect(result.contexts.browser_state).toEqual({ + ready_state: "interactive", visibility_state: "hidden", translation_marker: false, + }); + }); + + it("keeps the component trace when browser-state reads throw", () => { + vi.spyOn(document, "readyState", "get").mockImplementation(() => { throw new Error("unavailable"); }); + vi.spyOn(document.documentElement, "classList", "get").mockImplementation(() => { throw new Error("unavailable"); }); + expect(buildBrowserErrorContext({ boundary: "app", componentStack: "\n at TaskDetail" })).toEqual({ + tags: { react_error_boundary: "app" }, + contexts: { + react: { componentStack: "\n at TaskDetail" }, + browser_state: { visibility_state: document.visibilityState }, + }, + }); + }); + + it("does not require a browser document", () => { + vi.stubGlobal("document", undefined); + expect(buildBrowserErrorContext({ boundary: "app" }).contexts.browser_state).toEqual({}); + }); +}); diff --git a/ui/src/lib/browser-error-context.ts b/ui/src/lib/browser-error-context.ts new file mode 100644 index 00000000000..b0b7b896448 --- /dev/null +++ b/ui/src/lib/browser-error-context.ts @@ -0,0 +1,57 @@ +export interface BrowserErrorDetails { + boundary: "app" | "route"; + componentStack?: string | null; +} + +const MAX_STACK_LENGTH = 16_384; +const MAX_COMPONENT_FRAMES = 40; + +/** Keep component names only: React stack locations can contain tenant URLs. */ +export function sanitizeComponentStack(stack: string | null | undefined): string | undefined { + if (!stack) return undefined; + const frames: string[] = []; + for (const line of stack.slice(0, MAX_STACK_LENGTH).split("\n").slice(0, MAX_COMPONENT_FRAMES + 1)) { + // V8/React and Firefox/Safari component-frame formats. Never retain the + // location, arguments, or an unrecognized line as a fallback. + const name = /^\s*(?:at|in) ([A-Za-z_$][\w$.-]{0,99})(?:\s|$)/.exec(line)?.[1] + ?? /^([A-Za-z_$][\w$.-]{0,99})@/.exec(line)?.[1]; + if (name) frames.push(` at ${name}`); + if (frames.length === MAX_COMPONENT_FRAMES) break; + } + return frames.length ? `\n${frames.join("\n")}` : undefined; +} + +/** A bounded snapshot, taken at the error rather than when the capture queue runs. */ +export function buildBrowserErrorContext(details: BrowserErrorDetails) { + const componentStack = sanitizeComponentStack(details.componentStack); + const browserState: { + ready_state?: "loading" | "interactive" | "complete"; + visibility_state?: "visible" | "hidden"; + translation_marker?: boolean; + } = {}; + if (typeof document !== "undefined") { + // Each read is optional. A DOM wrapper or extension must not prevent the + // original exception and component trace from reaching the error monitor. + try { + const state = document.readyState; + if (state === "loading" || state === "interactive" || state === "complete") { + browserState.ready_state = state; + } + } catch { /* diagnostic unavailable */ } + try { + const state = document.visibilityState; + if (state === "visible" || state === "hidden") browserState.visibility_state = state; + } catch { /* diagnostic unavailable */ } + try { + const classes = document.documentElement.classList; + browserState.translation_marker = classes.contains("translated-ltr") || classes.contains("translated-rtl"); + } catch { /* diagnostic unavailable */ } + } + return { + tags: { react_error_boundary: details.boundary }, + contexts: { + react: componentStack ? { componentStack } : {}, + browser_state: browserState, + }, + }; +} diff --git a/ui/src/lib/sentry.test.ts b/ui/src/lib/sentry.test.ts index a887cff0965..734be7edf19 100644 --- a/ui/src/lib/sentry.test.ts +++ b/ui/src/lib/sentry.test.ts @@ -276,6 +276,103 @@ describe("captureBrowserException", () => { expect(mocks.captureException).toHaveBeenCalledWith(expect.any(Error)); }); + + it("sends sanitized per-error context captured before the queue runs", async () => { + const mocks = mockSentryPackage(); + const { initBrowserErrorMonitoring, captureBrowserException } = await importFreshSentry(); + await initBrowserErrorMonitoring(DSN); + document.documentElement.classList.add("translated-ltr"); + const error = new DOMException("insertBefore failed", "NotFoundError"); + captureBrowserException(error, { + boundary: "route", componentStack: "\n at TaskDetail (https://tenant.example/tasks/private-id?token=private-value:1:2)", + }); + document.documentElement.classList.remove("translated-ltr"); + await Promise.resolve(); + await Promise.resolve(); + expect(mocks.captureException).toHaveBeenCalledWith(error, { + tags: { react_error_boundary: "route" }, + contexts: { + react: { componentStack: "\n at TaskDetail" }, + browser_state: expect.objectContaining({ translation_marker: true }), + }, + }); + expect(JSON.stringify(mocks.captureException.mock.calls)).not.toContain("private-value"); + captureBrowserException(new Error("unrelated error")); + // Initialization is idempotent and provides a barrier on the same queue. + await initBrowserErrorMonitoring(DSN); + expect(mocks.captureException).toHaveBeenLastCalledWith(expect.any(Error)); + }); + + it("still captures the original error if component diagnostics fail", async () => { + const mocks = mockSentryPackage(); + const { initBrowserErrorMonitoring, captureBrowserException } = await importFreshSentry(); + await initBrowserErrorMonitoring(DSN); + const error = new Error("original error"); + expect(() => captureBrowserException(error, { + boundary: "app", + get componentStack(): string { throw new Error("diagnostic failure"); }, + })).not.toThrow(); + await Promise.resolve(); + await Promise.resolve(); + expect(mocks.captureException).toHaveBeenCalledWith(error); + }); +}); + +describe("browser error diagnostics with the real SDK", () => { + it("emits bounded component and translation context without page data or scope leakage", async () => { + const Sentry = await vi.importActual("@sentry/browser"); + const events: Array> = []; + vi.doMock("@sentry/browser", () => ({ + ...Sentry, + init: (options: Parameters[0]) => Sentry.init({ + ...options, + transport: () => ({ send: async () => ({}), flush: async () => true }), + beforeSend: (event) => { events.push(event as unknown as Record); return event; }, + }), + })); + const gate = await importFreshSentry(); + await gate.initBrowserErrorMonitoring(DSN, "staging"); + const previousPath = window.location.pathname + window.location.search; + window.history.replaceState({}, "", "/tasks/private-task?token=private-capability"); + document.documentElement.classList.add("translated-ltr", "private-customer-class"); + try { + gate.captureBrowserException(new DOMException("insertBefore failed", "NotFoundError"), { + boundary: "route", + componentStack: "\n at TaskDetail (https://tenant.example/tasks/private-task?token=private-capability:3:4)", + }); + document.documentElement.classList.remove("translated-ltr"); + gate.captureBrowserException(new Error("unrelated failure")); + await gate.initBrowserErrorMonitoring(DSN, "staging"); + await Sentry.flush(2000); + expect(events).toHaveLength(2); + expect(events[0]).toMatchObject({ + environment: "staging", + tags: { react_error_boundary: "route" }, + contexts: { + react: { componentStack: "\n at TaskDetail" }, + browser_state: { translation_marker: true }, + }, + }); + expect(events[1]).not.toHaveProperty("contexts.react"); + expect(events[1]).not.toHaveProperty("contexts.browser_state"); + expect(events[1]).not.toHaveProperty("tags.react_error_boundary"); + for (const event of events) { + expect(event).not.toHaveProperty("request"); + expect(event.breadcrumbs).toBeUndefined(); + } + for (const secret of ["tenant.example", "private-task", "private-capability", "private-customer-class"]) { + expect(JSON.stringify(events)).not.toContain(secret); + } + await gate.teardownBrowserErrorMonitoring(); + gate.captureBrowserException(new Error("after sign-out"), { boundary: "app", componentStack: "\n at TaskDetail" }); + await gate.teardownBrowserErrorMonitoring(); + expect(events).toHaveLength(2); + } finally { + await gate.teardownBrowserErrorMonitoring(); + document.documentElement.classList.remove("translated-ltr", "private-customer-class"); + window.history.replaceState({}, "", previousPath); + } + }); }); /** diff --git a/ui/src/lib/sentry.ts b/ui/src/lib/sentry.ts index 0c3a8e6c63b..7d714793dca 100644 --- a/ui/src/lib/sentry.ts +++ b/ui/src/lib/sentry.ts @@ -38,6 +38,8 @@ // (`GlobalHandlers`), the two React error boundaries, deduplicates a repeat // event (`Dedupe`), and links a caused-by chain (`LinkedErrors`). +import { buildBrowserErrorContext, type BrowserErrorDetails } from "./browser-error-context"; + let queue: Promise = Promise.resolve(); /** Run gate operations one at a time, in call order. */ @@ -135,10 +137,17 @@ export function teardownBrowserErrorMonitoring(): Promise { * control flow. A no-op before the gate opens, when the gate never opens (no * DSN on the session), or when bootstrap failed. */ -export function captureBrowserException(error: unknown): void { +export function captureBrowserException(error: unknown, details?: BrowserErrorDetails): void { + let context: ReturnType | undefined; + try { + if (details) context = buildBrowserErrorContext(details); + } catch { + // Diagnostics must never replace the original exception or its recovery UI. + } void enqueue(async () => { try { - sentry?.captureException(error); + if (context) sentry?.captureException(error, context); + else sentry?.captureException(error); } catch (err) { // eslint-disable-next-line no-console console.error("[paperclip] Sentry captureBrowserException failed", err); diff --git a/ui/src/lib/vite-component-names.test.ts b/ui/src/lib/vite-component-names.test.ts new file mode 100644 index 00000000000..bdccf05b82d --- /dev/null +++ b/ui/src/lib/vite-component-names.test.ts @@ -0,0 +1,41 @@ +import { mkdtemp, mkdir, readFile, rm, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { runInNewContext } from "node:vm"; +import { build } from "vite"; +import { expect, it } from "vitest"; + +it("keeps component names through the production bundle for React error traces", async () => { + const root = await mkdtemp(path.join(os.tmpdir(), "paperclip-component-names-")); + try { + await mkdir(path.join(root, "public")); + await writeFile(path.join(root, "public/sw.js"), 'const buildId = "__PAPERCLIP_BUILD_ID__";'); + const entry = path.join(root, "entry.js"); + await writeFile(entry, ` + function DiagnosticComponent() { return null; } + class DiagnosticBoundary { render() { return null; } } + const DiagnosticArrow = () => null; + globalThis.componentNames = [DiagnosticComponent.name, DiagnosticBoundary.name, DiagnosticArrow.name]; + `); + const result = await build({ + configFile: fileURLToPath(new URL("../../vite.config.ts", import.meta.url)), + root, + mode: "production", + logLevel: "silent", + build: { + outDir: path.join(root, "dist"), + rolldownOptions: { input: entry, output: { format: "iife" } }, + }, + }); + const output = (Array.isArray(result) ? result[0] : result) as { output: Array<{ type: string; fileName: string }> }; + const chunk = output.output.find((item) => item.type === "chunk"); + expect(chunk).toBeDefined(); + const code = await readFile(path.join(root, "dist", chunk!.fileName), "utf8"); + const scope: { componentNames?: string[] } = {}; + runInNewContext(code, scope); + expect(scope.componentNames).toEqual(["DiagnosticComponent", "DiagnosticBoundary", "DiagnosticArrow"]); + } finally { + await rm(root, { recursive: true, force: true }); + } +}, 30_000); diff --git a/ui/vite.config.ts b/ui/vite.config.ts index 2ecc84ec774..6ac1be642c9 100644 --- a/ui/vite.config.ts +++ b/ui/vite.config.ts @@ -22,6 +22,9 @@ export default defineConfig(({ mode }) => ({ esbuild: mode === "production" ? { + // React's component trace uses function names. Keep those useful in + // error reports without publishing source maps or page context. + keepNames: true, drop: ["console", "debugger"], legalComments: "none", }