diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index e07ba3f82c0..317bdbbefee 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -1641,3 +1641,16 @@ disconnected, visible active queries refresh every 15 seconds. This fallback stops when the socket opens, the tab is hidden, or the provider unmounts. A reconnected socket also refreshes visible queries to recover missed events. Run log views retain their existing HTTP polling fallback. + +### Company context during hot reload + +The company React context retains only its object identity in Vite's per-module +`hot.data`. Provider values remain in the mounted React tree and keep their normal +account scope. This lets a refreshed consumer read a provider from the preceding +module version. Production builds do not use the development cache. + +Run `pnpm test:e2e:browser-context` to test this with real Vite modules and +Chromium. The test starts its own loopback Vite server and mocks API responses; +it needs no running Paperclip instance or provider credentials. The same spec lives +in the default `test:e2e` discovery tree, so the existing Chrome CI shards run it +on pull requests. diff --git a/doc/observability.md b/doc/observability.md index 7b5bef23c88..fb2a8b15eea 100644 --- a/doc/observability.md +++ b/doc/observability.md @@ -418,13 +418,21 @@ Application and route error-boundary reports also include: 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. + +All browser error reports, including global promise rejections, include: + +- `browser_build_mode`: `development` or `production`, from the loaded bundle. +- `browser_rejection_kind`: the primitive type of an unhandled rejected value + (or `null`). The diagnostic does not read object properties or copy the value. - `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, +Boundary state is captured at the failure, before asynchronous reporting. +Global reports without that snapshot read document state before sending. +All fields are 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 diff --git a/package.json b/package.json index f5566737bfe..fc3c8b0bae9 100644 --- a/package.json +++ b/package.json @@ -71,6 +71,7 @@ "test:e2e:runner:models:update": "node cli/node_modules/tsx/dist/cli.mjs tests/runner-e2e/openrouter-models-update.ts", "test:e2e:runner:history:publish": "node cli/node_modules/tsx/dist/cli.mjs tests/runner-e2e/history-publish.ts", "test:runner-recovery": "vitest run server/src/services/native-runtime/native-replacement-evidence.test.ts server/src/services/native-runtime/stopped-codex-turn.test.ts server/src/services/native-runtime/native-safe-replacement.test.ts", + "test:e2e:browser-context": "playwright test --config tests/e2e/playwright-company-context.config.ts", "test:e2e:runner:browser-support": "playwright test --config tests/runner-e2e/playwright-support.config.ts", "test:e2e:runner:unit": "vitest run --config tests/runner-e2e/vitest.config.ts", "test:e2e:runner:typecheck": "tsc -p tests/runner-e2e/tsconfig.json", diff --git a/tests/e2e/company-context-refresh.spec.ts b/tests/e2e/company-context-refresh.spec.ts new file mode 100644 index 00000000000..309bd841b88 --- /dev/null +++ b/tests/e2e/company-context-refresh.spec.ts @@ -0,0 +1,77 @@ +import path from "node:path"; +import { expect, test } from "@playwright/test"; +import { createServer, type ViteDevServer } from "../../ui/node_modules/vite/dist/node/index.js"; + +let server: ViteDevServer; +let origin: string; +const uiRoot = path.resolve(import.meta.dirname, "../../ui"); +const probePath = path.join(uiRoot, "src/__company_context_probe.tsx"); + +// Use real Vite module instances and React contexts. Re-importing a refreshed +// consumer must still see a provider retained from the previous module version. +test.beforeAll(async () => { + server = await createServer({ + root: uiRoot, + configFile: false, + resolve: { alias: { "@": path.join(uiRoot, "src") } }, + esbuild: { jsx: "automatic" }, + server: { host: "127.0.0.1", port: 0 }, + plugins: [{ + name: "company-context-regression", + resolveId(id) { if (id === "/src/__company_context_probe.tsx") return probePath; }, + load(id) { + if (id !== probePath) return; + return ` + import React from "react"; + import { createRoot } from "react-dom/client"; + import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + import { CompanyProvider, useCompany } from "/src/context/CompanyContext.tsx"; + const root = createRoot(document.getElementById("root")); + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + function render(hook, generation) { + function Consumer() { return
{generation + ":" + (hook().selectedCompanyId ?? "loading")}
; } + root.render(); + } + render(useCompany, "initial"); + window.refreshConsumer = async () => { + const refreshed = await import(/* @vite-ignore */ "/src/context/CompanyContext.tsx?t=" + Date.now()); + render(refreshed.useCompany, "refreshed"); + }; + `; + }, + configureServer(vite) { + vite.middlewares.use((req, res, next) => { + if (req.url !== "/") return next(); + res.setHeader("Content-Type", "text/html"); + res.end('
'); + }); + }, + }], + }); + await server.listen(); + const address = server.httpServer!.address(); + if (!address || typeof address === "string") throw new Error("Missing test server address"); + origin = `http://127.0.0.1:${address.port}`; +}); + +test.afterAll(async () => { await server?.close(); }); + +test("a refreshed company consumer reads the retained provider", async ({ page }) => { + await page.route("**/api/**", async (route) => { + const pathname = new URL(route.request().url()).pathname; + if (!pathname.startsWith("/api/")) return route.continue(); + const body = pathname === "/api/auth/get-session" + ? { user: { id: "test-user" }, session: { userId: "test-user" } } + : pathname === "/api/companies" + ? [{ id: "test-company", name: "Test company", status: "active" }] + : []; + await route.fulfill({ json: body }); + }); + const errors: string[] = []; + page.on("pageerror", (error) => { errors.push(error.message); }); + await page.goto(origin); + await expect(page.getByRole("main")).toHaveText("initial:test-company"); + await page.evaluate(() => (window as unknown as { refreshConsumer: () => Promise }).refreshConsumer()); + await expect(page.getByRole("main")).toHaveText("refreshed:test-company"); + expect(errors).toEqual([]); +}); diff --git a/tests/e2e/playwright-company-context.config.ts b/tests/e2e/playwright-company-context.config.ts new file mode 100644 index 00000000000..15c5f3a8300 --- /dev/null +++ b/tests/e2e/playwright-company-context.config.ts @@ -0,0 +1,16 @@ +import { defineConfig } from "@playwright/test"; + +export default defineConfig({ + testDir: ".", + testMatch: "company-context-refresh.spec.ts", + workers: 1, + timeout: 30_000, + use: { + headless: true, + ...(process.env.PAPERCLIP_PLAYWRIGHT_CHANNEL + ? { channel: process.env.PAPERCLIP_PLAYWRIGHT_CHANNEL } + : {}), + }, + outputDir: "./test-results", + reporter: "list", +}); diff --git a/ui/src/context/CompanyContext.tsx b/ui/src/context/CompanyContext.tsx index 64e41ae37ca..677875216fb 100644 --- a/ui/src/context/CompanyContext.tsx +++ b/ui/src/context/CompanyContext.tsx @@ -6,6 +6,7 @@ import { useMemo, useRef, useState, + type Context, type ReactNode, } from "react"; import { useMutation, useQueryClient } from "@tanstack/react-query"; @@ -46,7 +47,11 @@ interface CompanyContextValue { const STORAGE_KEY = "paperclip.selectedCompanyId"; -const CompanyContext = createContext(null); +// A refresh can replace consumers before a mounted provider is replaced. Keep +// their context identity in Vite's per-module data; never store account state. +const CompanyContext: Context = + import.meta.hot?.data?.companyContext ?? createContext(null); +if (import.meta.hot?.data) import.meta.hot.data.companyContext = CompanyContext; export function resolveBootstrapCompanySelection(input: { companies: Array>; diff --git a/ui/src/lib/browser-error-context.ts b/ui/src/lib/browser-error-context.ts index b0b7b896448..700e6f62ddc 100644 --- a/ui/src/lib/browser-error-context.ts +++ b/ui/src/lib/browser-error-context.ts @@ -21,9 +21,8 @@ export function sanitizeComponentStack(stack: string | null | undefined): string 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); +/** Bounded document state; never read page content, locations, or identifiers. */ +export function readBrowserErrorState() { const browserState: { ready_state?: "loading" | "interactive" | "complete"; visibility_state?: "visible" | "hidden"; @@ -47,11 +46,17 @@ export function buildBrowserErrorContext(details: BrowserErrorDetails) { browserState.translation_marker = classes.contains("translated-ltr") || classes.contains("translated-rtl"); } catch { /* diagnostic unavailable */ } } + return browserState; +} + +/** A bounded snapshot, taken at the error rather than when the capture queue runs. */ +export function buildBrowserErrorContext(details: BrowserErrorDetails) { + const componentStack = sanitizeComponentStack(details.componentStack); return { tags: { react_error_boundary: details.boundary }, contexts: { react: componentStack ? { componentStack } : {}, - browser_state: browserState, + browser_state: readBrowserErrorState(), }, }; } diff --git a/ui/src/lib/sentry.test.ts b/ui/src/lib/sentry.test.ts index 734be7edf19..4e87c29b20a 100644 --- a/ui/src/lib/sentry.test.ts +++ b/ui/src/lib/sentry.test.ts @@ -327,7 +327,11 @@ describe("browser error diagnostics with the real SDK", () => { init: (options: Parameters[0]) => Sentry.init({ ...options, transport: () => ({ send: async () => ({}), flush: async () => true }), - beforeSend: (event) => { events.push(event as unknown as Record); return event; }, + beforeSend: async (event, hint) => { + const enriched = await options?.beforeSend?.(event, hint) ?? event; + events.push(enriched as unknown as Record); + return enriched; + }, }), })); const gate = await importFreshSentry(); @@ -354,7 +358,7 @@ describe("browser error diagnostics with the real SDK", () => { }, }); expect(events[1]).not.toHaveProperty("contexts.react"); - expect(events[1]).not.toHaveProperty("contexts.browser_state"); + expect(events[1]).toHaveProperty("contexts.browser_state.translation_marker", false); expect(events[1]).not.toHaveProperty("tags.react_error_boundary"); for (const event of events) { expect(event).not.toHaveProperty("request"); @@ -424,12 +428,12 @@ describe("buildBrowserSentryInitOptions", () => { expect(options.tracesSampleRate).toBe(0); }); - it("holds no beforeSend hook and no custom filter function", async () => { + it("adds diagnostics without a transaction filter", async () => { const { buildBrowserSentryInitOptions } = await importFreshSentry(); const options = buildBrowserSentryInitOptions(DSN); - expect(options.beforeSend).toBeUndefined(); + expect(options.beforeSend).toBeTypeOf("function"); expect(options.beforeSendTransaction).toBeUndefined(); }); @@ -467,10 +471,8 @@ describe("captured event shape against the real @sentry/browser SDK", () => { /** * Initialize the real SDK with this module's exact options, plus a * transport stub so no event leaves the test process, plus `beforeSend` - * so the test can inspect the resolved event before it would have been - * sent. `beforeSend` here is test-only introspection — the shipped module - * adds no `beforeSend` of its own (see the "holds no beforeSend hook" - * test above). + * so the test can inspect the resolved event after the production diagnostic + * hook runs, before it would have been sent. */ async function initRealSentryForTest( onEvent: (event: Record) => void, @@ -478,17 +480,42 @@ describe("captured event shape against the real @sentry/browser SDK", () => { ) { const { buildBrowserSentryInitOptions } = await importFreshSentry(); const Sentry = await import("@sentry/browser"); + const options = buildBrowserSentryInitOptions(DSN, environment); Sentry.init({ - ...buildBrowserSentryInitOptions(DSN, environment), + ...options, transport: () => ({ send: async () => ({}), flush: async () => true }), - beforeSend: (event) => { - onEvent(event as unknown as Record); - return event; + beforeSend: async (event, hint) => { + const enriched = await options.beforeSend?.(event, hint) ?? event; + onEvent(enriched as unknown as Record); + return enriched; }, }); return Sentry; } + it("enriches an undefined global rejection without page data or suppression", async () => { + let captured: Record | null = null; + const Sentry = await initRealSentryForTest((event) => { captured = event; }, "staging"); + const previousHandler = window.onunhandledrejection; + try { + expect(previousHandler).toBeTypeOf("function"); + document.documentElement.classList.add("translated-ltr", "private-class"); + window.onunhandledrejection?.call(window, { reason: undefined } as PromiseRejectionEvent); + await Sentry.flush(2000); + expect(captured).toMatchObject({ + tags: { browser_rejection_kind: "undefined", browser_build_mode: "development" }, + contexts: { browser_state: { translation_marker: true } }, + exception: { values: [{ type: "UnhandledRejection" }] }, + }); + expect(captured).not.toHaveProperty("request"); + expect((captured as unknown as Record).breadcrumbs).toBeUndefined(); + expect(JSON.stringify(captured)).not.toContain("private-class"); + } finally { + document.documentElement.classList.remove("translated-ltr", "private-class"); + await Sentry.close(); + } + }); + it.each([ ["staging", "staging"], ["production", "production"], diff --git a/ui/src/lib/sentry.ts b/ui/src/lib/sentry.ts index 7d714793dca..852e2b855be 100644 --- a/ui/src/lib/sentry.ts +++ b/ui/src/lib/sentry.ts @@ -38,7 +38,7 @@ // (`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"; +import { buildBrowserErrorContext, readBrowserErrorState, type BrowserErrorDetails } from "./browser-error-context"; let queue: Promise = Promise.resolve(); @@ -175,6 +175,33 @@ export function buildBrowserSentryInitOptions( : undefined, tracesSampleRate: 0, sendDefaultPii: false, + beforeSend: (event, hint) => { + // Global handlers do not pass through our React boundaries. Give their + // reports the same bounded document state, without URLs or breadcrumbs. + // Preserve a boundary's earlier snapshot across the asynchronous queue. + try { + event.contexts = { + ...event.contexts, + browser_state: event.contexts?.browser_state ?? readBrowserErrorState(), + }; + event.tags = { + ...event.tags, + browser_build_mode: import.meta.env.DEV ? "development" : "production", + }; + if (event.exception?.values?.some((value) => + value.mechanism?.type === "onunhandledrejection" + || value.mechanism?.type === "auto.browser.global_handlers.onunhandledrejection", + )) { + // The SDK supplies the rejected value directly. Classify without + // reading object properties, coercing strings, or copying its value. + const reason = hint.originalException; + event.tags.browser_rejection_kind = reason === null ? "null" : typeof reason; + } + } catch { + // Diagnostics must not replace or discard the original error. + } + return event; + }, integrations: (defaults) => defaults.filter( (integration) => integration.name !== "HttpContext" && integration.name !== "Breadcrumbs",