From 87ed83045794a0da268095f764b686d2ae17f903 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Artur=20M=C4=99dryga=C5=82?= Date: Wed, 9 Sep 2026 15:16:17 +0200 Subject: [PATCH] fix(runner): promote React 18 boundary logs so they carry a component stack (DEV-2875) Sentry DEMOS-4P arrives stackless and its issues shard per component. React 18 passes ONE joined string to console.error -- react-dom@18.3.1 cjs/react-dom.development.js:18689-18704 builds componentNameMessage + "\n" + componentStack + "\n\n" + errorBoundaryMessage -- so the component stack lives inside the message. `send` truncates messages at MONITOR_MESSAGE_MAX (500), which is why recognition has to happen in-page: parent-side, the stack would already be gone. `reactBoundaryReport` runs after `errorArgReport`, so React 19 keeps winning (it passes the Error as an argument and is already re-homed). Promotion requires four things, and the fourth is the safety property: one string argument, React's prefix at index 0, "error boundary" in the tail, and at least one `at `-form frame. No frames means no promotion, so React prose alone stays on the console channel. `at `-form only -- `sentry.ts` synthesises `DemoError: \n` for Sentry's Chrome parser, and the legacy `in X (at file:line)` form parses to zero frames, which is worse than staying put. The real defect is not the duplication. Today the companion's fingerprint contains the component stack, so it mints a new issue per component, file and line; the constant message makes it one permanent bucket. The pair itself cannot be collapsed client-side -- React logs the companion after the browser already saw the error, and it carries no error message to correlate on. The trace id is the pivot, and it already works. Two events per fault, never three: the branch is exclusive. The component name is elided to in-page rather than in `normalizeMonitorMessage`, which feeds only the fingerprint -- a rule there would fix grouping and leave the title flapping per visitor-chosen component name, which is the DEV-2854 defect. Two corrections to the plan, both caught by the tests: - It claimed the stack is "capped at MONITOR_STACK_MAX". `truncate` appends a 3-char ellipsis, so the real bound is STACK_MAX + 3 (2003). Asserted exactly, because a loose `< 3000` would not notice the cap being removed. - Its stack-cap test wanted the stack to both exceed the cap and retain the last frame. Those cannot both hold -- truncation drops around frame 36 of 40. The test now asserts the bound and the truncation marker. Co-Authored-By: Claude Opus 5 --- runner/packages/runtime/src/monitor.ts | 95 ++++++++++++++-- runner/packages/runtime/src/sandpack.ts | 15 +-- runner/pipeline/monitor-inject.test.mjs | 141 ++++++++++++++++++++++++ 3 files changed, 235 insertions(+), 16 deletions(-) diff --git a/runner/packages/runtime/src/monitor.ts b/runner/packages/runtime/src/monitor.ts index 5f8f8223c..e7b26e496 100644 --- a/runner/packages/runtime/src/monitor.ts +++ b/runner/packages/runtime/src/monitor.ts @@ -73,6 +73,35 @@ export const MONITOR_COMPILE_MESSAGE_MAX = 2000; /** URL cap. A path this long is already unreadable; the rest is only volume. */ export const MONITOR_URL_MAX = 500; +/** + * The exact prefix React 18 dev uses when it logs an error-boundary component + * stack to `console.error` — `react-dom@18.3.1/cjs/react-dom.development.js:18689-18704` + * (fetched verbatim) builds `componentNameMessage + "\n" + componentStack + "\n\n" + + * errorBoundaryMessage` and calls `console['error'](combinedMessage)` as a single + * string argument. Recognising it (DEV-2875, DEMOS-4P) is what lets that call be + * promoted from `console-error` to `error`, carrying its component stack as a real + * `stack` rather than losing it inside a message `send` truncates at + * `MONITOR_MESSAGE_MAX`. + * + * React 19 needs none of this: it calls `console.error("%o\n\n%s\n\n%s\n", error, …)`, + * so the Error object is an argument and `errorArgReport` already re-homes it with the + * real message and stack. + */ +export const MONITOR_REACT_BOUNDARY_PREFIX = "The above error occurred in "; + +/** + * What a React 18 error-boundary component name is replaced with in a promoted + * event's message. + * + * `` and its siblings are docs-authored content, not ours — 1794 + * occurrences under `apps/authoring/public/docs-examples/` — and this message becomes + * both the Sentry issue title and (via `normalizeMonitorMessage`) the fingerprint. An + * unredacted component name would fingerprint one bucket per authored example instead + * of one bucket for the defect, which is the DEV-2854 flapping-title failure this + * ticket exists to fix. Matches house style: `` / `` / `` / ``. + */ +export const MONITOR_COMPONENT_PLACEHOLDER = ""; + /** * What a Tier-2 preview host is replaced with. * @@ -374,6 +403,8 @@ export const REPORTER_SOURCE = `(function () { } catch (e) { return; } var TYPE = ${JSON.stringify(MONITOR_MESSAGE_TYPE)}; + var REACT_PREFIX = ${JSON.stringify(MONITOR_REACT_BOUNDARY_PREFIX)}; + var COMPONENT = ${JSON.stringify(MONITOR_COMPONENT_PLACEHOLDER)}; var CEILING = ${MONITOR_EVENT_CEILING}; var WARN_CEILING = ${MONITOR_BREADCRUMB_CEILING}; var MAX = ${MONITOR_MESSAGE_MAX}; @@ -497,7 +528,10 @@ export const REPORTER_SOURCE = `(function () { // \`instanceof\` can throw on an exotic proxy, and a cross-realm error (one raised in // an iframe the demo itself created) answers false. Both degrade the same way: the // value is not treated as an Error, so the console event is relayed as a plain - // \`console-error\` — the pre-DEV-2552 behaviour, never a crash. + // \`console-error\` — the pre-DEV-2552 behaviour, never a crash. \`reactBoundaryReport\` + // (DEV-2875) is the one other escape hatch off that default: React 18's + // error-boundary log has no Error argument for this function to find at all, so it + // is recognised by string shape instead, after this check has already declined. function isErrorLike(a) { try { return !!a && a instanceof Error; @@ -545,6 +579,41 @@ export const REPORTER_SOURCE = `(function () { return null; } + // DEV-2875. Second re-homing path, tried after \`errorArgReport\` so React 19 (an + // Error argument, covered above) keeps winning. React 18's boundary log has no Error + // arg at all — one joined string, component stack included — so it must be + // recognised by shape, in-page, before \`send\` truncates the message and takes the + // stack with it. Four load-bearing conditions: (1) one string arg — React 19's \`%o\` + // form is excluded by arity; (2) REACT_PREFIX at index 0; (3) "error boundary" + // present; (4) >=1 \`at \`-form frame below line 1 — the condition that makes + // promotion conditional on actually carrying a stack, so frame-less prose stays on + // \`console-error\`. \`at \`-form only: Sentry's parser reads that shape, the legacy + // \`in X (at file:line)\` form parses to zero frames. \`else if (frames.length) break\` + // skips React's one leading blank line without swallowing trailing prose. Component + // name elided (first match only) — it is visitor-authored and would otherwise + // fingerprint one issue per example (DEV-2854). try/catch as in \`errorArgReport\`: + // runs at the call site, outside \`send\`'s own catch. + function reactBoundaryReport(args) { + try { + if (args.length !== 1) return null; + var s = args[0]; + if (typeof s !== "string") return null; + if (s.indexOf(REACT_PREFIX) !== 0) return null; + if (s.indexOf("error boundary") === -1) return null; + var lines = s.split("\\n"); + var frames = []; + for (var i = 1; i < lines.length; i++) { + if (/^\\s+at\\s+\\S/.test(lines[i])) frames.push(lines[i]); + else if (frames.length) break; + } + if (!frames.length) return null; + return { + message: lines[0].replace(/<[^<>]*>/, COMPONENT), + stack: frames.join("\\n") + }; + } catch (e) { return null; } + } + function argsToMessage(args) { var parts = []; for (var i = 0; i < args.length; i++) { @@ -586,13 +655,20 @@ export const REPORTER_SOURCE = `(function () { var origError = console.error; var origWarn = console.warn; console.error = function () { - // See \`errorArgReport\`: an Error here belongs to the error channel, and is sent - // under that kind so \`send\`'s dedupe collapses it with the window listener's - // copy of the same throw. The passthrough is outside the branch — what the - // reporter does with an event must never change the demo's own console output. + // See \`errorArgReport\`: an Error here belongs to the error channel, so its dedupe + // key matches the window listener's copy of the same throw. It MUST keep winning + // first — that covers React 19's Error-argument form. \`reactBoundaryReport\` is + // the second and last escape hatch (DEV-2875): React 18's boundary log has no + // Error argument, so it only gets a look once the first has declined. Passthrough + // is outside both branches — the reporter must never change the demo's own + // console output. var report = errorArgReport(arguments); if (report) send("error", report.message, report.stack); - else send("console-error", argsToMessage(arguments), ""); + else { + var boundary = reactBoundaryReport(arguments); + if (boundary) send("error", boundary.message, boundary.stack); + else send("console-error", argsToMessage(arguments), ""); + } if (origError) origError.apply(console, arguments); }; console.warn = function () { @@ -688,10 +764,11 @@ export const REPORTER_SOURCE = `(function () { * * One physical line is not free, and the cost lands somewhere non-obvious: babel's code * frame prints the two lines above the fault verbatim, so a syntax error on authored - * line 1 or 2 renders all 12.6 KB of this into the compile message ahead of the line + * line 1 or 2 renders all ~14.9 KB of this into the compile message ahead of the line * that is actually wrong, and `MONITOR_COMPILE_MESSAGE_MAX` then cuts the diagnostic off - * (measured: 289 characters of usable message with the reporter inlined, 12,872 with it - * on one line). `boundCompileMessage` in sandpack.ts therefore replaces this exact + * (measured after DEV-2875 grew the reporter: 294 characters of usable message with the + * reporter inlined, 15,486 with it on one line — was 289 / 12,872 before that ticket). + * `boundCompileMessage` in sandpack.ts therefore replaces this exact * constant with a marker before the cap runs — `stripInjectedReporter`, which is * coupled to this constant on purpose. Do not change the shape of this line without * checking that strip still fires. diff --git a/runner/packages/runtime/src/sandpack.ts b/runner/packages/runtime/src/sandpack.ts index 5e62fa5e6..c86507881 100644 --- a/runner/packages/runtime/src/sandpack.ts +++ b/runner/packages/runtime/src/sandpack.ts @@ -206,13 +206,14 @@ const INJECTED_REPORTER_MARKER = ""; /** * Strip the monitor's own injected line out of a compile message (DEV-2557). * - * `REPORTER_MODULE_LINE` is one 12.6 KB physical line at the top of the module entry, - * and babel's code frame prints the two lines *above* the fault verbatim — so a syntax - * error on authored line 1 or 2 renders that whole blob into the message before the - * offending line is reached. Measured on the vue starter's entry with an unterminated - * string on line 1: 289 characters of usable message with a caret when the reporter was - * still inlined, 12,872 with it on one line, and `MONITOR_COMPILE_MESSAGE_MAX` then cuts - * at 2,000 — so the diagnostic line and its caret were gone. That is DEV-2550's + * `REPORTER_MODULE_LINE` is one ~14.9 KB physical line at the top of the module entry + * (DEV-2875 grew it from ~12.3 KB), and babel's code frame prints the two lines *above* + * the fault verbatim — so a syntax error on authored line 1 or 2 renders that whole blob + * into the message before the offending line is reached. Measured on the vue starter's + * entry with an unterminated string on line 1: 294 characters of usable message with a + * caret when the reporter was still inlined, 15,486 with it on one line, and + * `MONITOR_COMPILE_MESSAGE_MAX` then cuts at 2,000 — so the diagnostic line and its + * caret were gone. That is DEV-2550's * buried-diagnostic failure (DEMOS-15) coming back on the same channel, from our own * bytes rather than from a source map. * diff --git a/runner/pipeline/monitor-inject.test.mjs b/runner/pipeline/monitor-inject.test.mjs index 1fc7c0f9a..c587f1032 100644 --- a/runner/pipeline/monitor-inject.test.mjs +++ b/runner/pipeline/monitor-inject.test.mjs @@ -481,6 +481,147 @@ test("DEV-2552: console.warn is not re-homed — it has no error-channel twin", ); }); +// ---- React 18 boundary logs get their component stack (DEV-2875, DEMOS-4P) --- +// +// react-dom@18.3.1 cjs/react-dom.development.js:18689-18704 builds ONE joined +// string — componentNameMessage + "\n" + componentStack + "\n\n" + +// errorBoundaryMessage — and passes it as a single argument. So the component +// stack lives INSIDE the message, which `send` truncates at MONITOR_MESSAGE_MAX. +// That is why recognition has to happen in-page rather than parent-side, and it +// is what test 2 below pins. + +const REACT18_STACK = + "\n at ExampleComponent (https://" + PREVIEW_HOST + "/src/App.js:20:11)" + + "\n at div" + + "\n at App (https://" + PREVIEW_HOST + "/src/App.js:8:3)"; +const REACT18_TAIL = + "Consider adding an error boundary to your tree to customize error handling behavior.\n" + + "Visit https://reactjs.org/link/error-boundaries to learn more about error boundaries."; +const REACT18_BOUNDARY_LOG = + "The above error occurred in the component:\n" + REACT18_STACK + "\n\n" + REACT18_TAIL; +const PROMOTED = "The above error occurred in the component:"; + +test("DEV-2875: a React 18 boundary log is promoted with its component stack", () => { + const h = runReporter(); + h.console.error(REACT18_BOUNDARY_LOG); + + assert.equal(h.sent.length, 1); + assert.equal(h.sent[0].kind, "error", "must reach captureException, which is the only path a stack survives"); + assert.equal(h.sent[0].message, PROMOTED); + assert.ok( + !h.sent[0].message.includes("ExampleComponent"), + "the component name is visitor-authored and becomes both title and fingerprint, so it is elided", + ); + assert.match(h.sent[0].stack, /at ExampleComponent/); + assert.equal(h.passthrough.length, 1, "the demo's own console output still happens"); +}); + +test("DEV-2875: the stack lands under the stack cap, not cut by the message cap", () => { + // The whole justification for recognising in-page. The joined block is one + // string, so parent-side the stack would already have been truncated at + // MONITOR_MESSAGE_MAX (500). Split in-page, it rides in `stack` under + // MONITOR_STACK_MAX (2000) instead. + const frames = []; + for (let i = 0; i < 40; i++) frames.push(`\n at Component${i} (https://${PREVIEW_HOST}/src/F${i}.js:${i}:7)`); + const long = + "The above error occurred in the component:" + frames.join("") + "\n\n" + REACT18_TAIL; + const h = runReporter(); + h.console.error(long); + + assert.ok(long.length > MONITOR_MESSAGE_MAX, "fixture must actually exceed the message cap"); + assert.equal(h.sent[0].message, "The above error occurred in the component:"); + assert.ok(h.sent[0].stack.length > MONITOR_MESSAGE_MAX, "the stack outlived the message cap"); + // `truncate` appends a 3-char ellipsis, so the bound is STACK_MAX + 3 rather + // than STACK_MAX. Asserted exactly: a loose `< 3000` would not notice the cap + // being removed, which is the thing worth pinning. + assert.ok(h.sent[0].stack.length <= MONITOR_STACK_MAX + 3, `unbounded stack: ${h.sent[0].stack.length}`); + assert.ok(h.sent[0].stack.endsWith("..."), "a stack past the cap is marked as truncated"); +}); + +test("DEV-2875: promoted component-stack frames are host-redacted", () => { + // First time frames travel in `stack` from the console channel, so `send`'s + // redact-before-truncate has to cover this path too. + const h = runReporter(); + h.console.error(REACT18_BOUNDARY_LOG); + + assert.match(h.sent[0].stack, //); + assert.ok(!h.sent[0].stack.includes("tok9xQ"), "the preview host token must not travel"); +}); + +test("DEV-2875: one fault reports twice, never three times", () => { + // React 18 dev surfaces the throw to window.onerror via the guarded-invoke + // fake-event trick — hence its own wording, "The ABOVE error". So the twin is + // common, not absent (DEMOS-76/77 share a trace). The pair cannot be collapsed + // client-side: React logs the companion after the browser already saw the + // error, and the companion carries no error message to correlate on. + const h = runReporter(); + h.fire("error", { error: new Error("boom"), message: "boom" }); + h.console.error(REACT18_BOUNDARY_LOG); + + assert.equal(h.sent.length, 2); + assert.deepEqual(h.sent.map((pl) => pl.kind), ["error", "error"]); + assert.deepEqual(h.sent.map((pl) => pl.message), ["boom", PROMOTED]); +}); + +test("DEV-2875 guard: an ordinary console.error is untouched", () => { + // Passes either way — this is the over-match boundary the whole change turns on. + const h = runReporter(); + h.console.error("something"); + + assert.deepEqual( + h.sent.map((pl) => [pl.kind, pl.message]), + [["console-error", "something"]], + ); +}); + +test("DEV-2875 guard: React prose with no frame block is not promoted", () => { + // Passes either way. The strongest anti-over-match rule: promotion is + // conditional on carrying the component stack that justifies it, so prose + // alone stays on the console channel. + const h = runReporter(); + h.console.error("The above error occurred in the component:\n\n" + REACT18_TAIL); + + assert.equal(h.sent[0].kind, "console-error"); +}); + +test("DEV-2875 guard: React 19's Error-argument form still wins via errorArgReport", () => { + // Passes either way. React 19 needs no fix — it passes the Error as an + // argument, so the first escape hatch re-homes it with the real message and + // stack. Recorded here so the ordering is not casually swapped. + const h = runReporter(); + h.console.error( + "%o\n\n%s\n\n%s\n", + new Error("boom"), + "The above error occurred in the component.", + "React will try to recreate this component tree", + ); + + assert.equal(h.sent[0].kind, "error"); + assert.equal(h.sent[0].message, "boom"); + assert.match(h.sent[0].stack, /Error: boom/); +}); + +test("DEV-2875 guard: the arity rule — a second argument declines promotion", () => { + // Passes either way. React 18's shape is exactly one string argument. + const h = runReporter(); + h.console.error(REACT18_BOUNDARY_LOG, "extra"); + + assert.equal(h.sent[0].kind, "console-error"); +}); + +test("DEV-2875 guard: repeats cost one slot, and the message is fingerprint-stable", () => { + // Passes either way. Today's key contains the whole block including the + // component stack, so a boundary looping over different components mints a new + // key each time; the constant message bounds that. + const h = runReporter(); + h.console.error(REACT18_BOUNDARY_LOG); + h.console.error(REACT18_BOUNDARY_LOG); + h.console.error(REACT18_BOUNDARY_LOG); + + assert.equal(h.sent.length, 1); + assert.equal(normalizeMonitorMessage(PROMOTED), PROMOTED, "no normalizer rule may reshape the constant title"); +}); + // ---- network events are same-origin only (DEV-2539, DEMOS-Z) ---------------- // // The reporter runs inside the preview document, so `location.host` *is* the preview