diff --git a/runner/packages/runtime/src/container.ts b/runner/packages/runtime/src/container.ts index 4a3270798..412d79c40 100644 --- a/runner/packages/runtime/src/container.ts +++ b/runner/packages/runtime/src/container.ts @@ -17,7 +17,7 @@ import type { } from "./types.js"; import { mintSessionId } from "./session.js"; import { applyHandsontableCss, applyHandsontableVersion } from "./version.js"; -import { MONITOR_EVENT_CEILING, truncateMessage } from "./monitor.js"; +import { MONITOR_EVENT_CEILING, normalizeMonitorMessage, truncateMessage } from "./monitor.js"; import { failureDetail, STDERR_MARKERS } from "./failure-log.js"; /** @@ -299,9 +299,19 @@ export class ContainerRuntime implements DemoRuntime { private flushTimer: ReturnType | null = null; private readonly progressCbs = new Set<(log: string) => void>(); private readonly stderrCbs = new Set<(line: string) => void>(); - /** Dev-server stderr lines already relayed, so a message the server repeats every - * keystroke is filed once. Capped by `MONITOR_EVENT_CEILING` below; the set only - * ever holds what fit under it. */ + /** Dev-server stderr lines already relayed, keyed on `normalizeMonitorMessage` — + * the same fingerprint `sentry.ts` groups the Sentry issue by — so a message the + * server repeats every keystroke, or with only its clock changed (a build + * envelope's timestamp), is filed once. A coarser key than the raw line, on + * purpose: the parent already fingerprints on the normalized message, so two raw + * lines that normalize identically were always going to land in one Sentry + * issue — relaying both just spent a `MONITOR_EVENT_CEILING` slot on a second + * sample of a fault already reported. The honest trade: only the first variant + * of a class now ever leaves the page, so no issue is lost, but sample diversity + * *within* an issue narrows — `Cannot find module 'foo'` and `'bar'` used to + * both reach Sentry as two events under one fingerprint; now only the first + * does. Capped by `MONITOR_EVENT_CEILING` below; the set only ever holds what + * fit under it. */ private readonly stderrSeen = new Set(); private stderrRelayed = 0; private previewUrl = ""; @@ -827,6 +837,11 @@ export class ContainerRuntime implements DemoRuntime { * same lines arrive over and over; `stderrSeen` is what makes this a report per * fault rather than one per minute. The ceiling is the same one the in-page * reporter uses, for the same reason — the kill switch is a deploy away. + * + * `stderrSeen` is keyed on `normalizeMonitorMessage(message)`, not the raw line + * — see the field comment. The relayed payload stays the raw, truncated line: the + * diagnostic reaching Sentry is still the verbatim compiler output, only the + * dedupe key is coarsened to match what `sentry.ts:263` fingerprints on. */ private relayStderr(log: string): void { if (this.disposed || this.stderrCbs.size === 0) return; @@ -835,8 +850,9 @@ export class ContainerRuntime implements DemoRuntime { const line = raw.trim(); if (!line || !STDERR_MARKERS.test(line)) continue; const message = truncateMessage(line); - if (this.stderrSeen.has(message)) continue; - this.stderrSeen.add(message); + const key = normalizeMonitorMessage(message); + if (this.stderrSeen.has(key)) continue; + this.stderrSeen.add(key); this.stderrRelayed += 1; for (const cb of this.stderrCbs) cb(message); } diff --git a/runner/packages/runtime/src/monitor.ts b/runner/packages/runtime/src/monitor.ts index d7a25d7aa..3ecdb9254 100644 --- a/runner/packages/runtime/src/monitor.ts +++ b/runner/packages/runtime/src/monitor.ts @@ -262,14 +262,26 @@ export function createMonitorBudget(ceiling: number = MONITOR_EVENT_CEILING): { /** * Collapse the volatile parts of a message so one broken demo files one issue - * rather than hundreds. Numbers, quoted strings and URLs are what differ between - * two reports of the same fault (a row index, a version, a session id). + * rather than hundreds. Numbers, quoted strings, URLs and timestamps are what + * differ between two reports of the same fault (a row index, a version, a + * session id, the clock a dev-server envelope was printed at). + * + * A timestamp is matched whole, ahead of the number rule, because it is one + * volatile token rather than a run of numbers. The number rule's word + * boundaries are load-bearing in the other direction: they are what keeps + * `TS1005` out of `TS`, so a diagnostic code stays a fingerprint and two + * different compiler errors stay two issues — confirmed for the live + * `TS1005` group (Sentry DEMOS-3K), and true even when two codes share + * identical prose (constructed, not observed: `TS2554`/`TS2555` both read + * "Expected N arguments, but got M"). Relaxing the boundaries to catch the + * `…-25T18:…` they miss would collapse the codes too. * * Used for the Sentry fingerprint, not for the message the issue displays. */ export function normalizeMonitorMessage(message: string): string { return message .replace(/https?:\/\/\S+/g, "") + .replace(/\d{4}-\d{2}-\d{2}[T ]\d{2}:\d{2}:\d{2}(?:\.\d+)?(?:Z|[+-]\d{2}:?\d{2})?/g, "") .replace(/["'`][^"'`]*["'`]/g, "") .replace(/\b\d+(\.\d+)*\b/g, "") .replace(/\s+/g, " ") diff --git a/runner/pipeline/monitor-inject.test.mjs b/runner/pipeline/monitor-inject.test.mjs index bea90aef6..9c6d227f0 100644 --- a/runner/pipeline/monitor-inject.test.mjs +++ b/runner/pipeline/monitor-inject.test.mjs @@ -653,6 +653,47 @@ test("normalizeMonitorMessage collapses what differs between two reports of one assert.equal(a, b, "same fault must fingerprint the same"); }); +// Sentry DEMOS-4G, DEMOS-4V. `normalizeMonitorMessage`'s number rule +// (`/\b\d+(\.\d+)*\b/g`) has a trailing `\b` that fails to match where a digit run +// abuts a letter, so an ISO timestamp's milliseconds and `Z` survived, and every +// Tier-2 build failure minted a fresh Sentry fingerprint — 28 issues, one event +// each, in `handsoncode/demos`. + +/** DEMOS-4G and DEMOS-4V: the same Angular build failure, two of the 28 issues it opened. */ +const BUNDLE_A = "Application bundle generation failed. [0.431 seconds] - 2026-08-25T18:06:09.937Z"; +const BUNDLE_B = "Application bundle generation failed. [1.595 seconds] - 2026-08-27T14:20:04.952Z"; + +const TS1005 = "✘ [ERROR] TS1005: ',' expected. [plugin angular-compiler]"; // DEMOS-3K +const TS2554 = "✘ [ERROR] TS2554: Expected 2 arguments, but got 1. [plugin angular-compiler]"; +const TS2555 = "✘ [ERROR] TS2555: Expected 2 arguments, but got 1. [plugin angular-compiler]"; + +test("one build failure is one fingerprint, whatever the clock said", () => { + assert.equal(normalizeMonitorMessage(BUNDLE_A), normalizeMonitorMessage(BUNDLE_B)); +}); + +test("a compiler diagnostic code survives normalisation", () => { + // THE discriminator. Collapsing the timestamp by relaxing the number rule's word + // boundaries (/\d+(\.\d+)*/g) also passes the inequality test below, because the + // prose differs — and silently turns every TS code into TS. This assertion is + // what fails under that fix. + assert.match(normalizeMonitorMessage(TS1005), /\bTS1005\b/); +}); + +test("two codes with identical prose stay two issues", () => { + // Constructed, not observed: no two live issues share prose today. TS2554/TS2555 + // both read "Expected N arguments, but got M", so the code is the only + // discriminator — exactly the case a boundary-dropping fix fails to cover. + assert.notEqual(normalizeMonitorMessage(TS2554), normalizeMonitorMessage(TS2555)); +}); + +test("the other timestamp shapes a dev server prints collapse too", () => { + // The [T ] and offset arms of the pattern, which the Angular fixture does not reach. + assert.equal( + normalizeMonitorMessage("done 2026-08-25 18:06:09 ok"), + normalizeMonitorMessage("done 2026-08-27T14:20:04+02:00 ok"), + ); +}); + // ---- wired through SandpackRuntime ----------------------------------------- // // The injector being correct is not the same as the runtime using it correctly. What diff --git a/runner/pipeline/monitor-stderr-relay.test.mjs b/runner/pipeline/monitor-stderr-relay.test.mjs new file mode 100644 index 000000000..55899b724 --- /dev/null +++ b/runner/pipeline/monitor-stderr-relay.test.mjs @@ -0,0 +1,175 @@ +// Sentry DEMOS-4G / DEMOS-4V: 28 `handsoncode/demos` issues, one event each, all the +// same Angular build failure — "Application bundle generation failed. [ seconds] - +// ". The envelope line carries a live timestamp, so `relayStderr`'s dedupe +// (`stderrSeen`, keyed on the raw line) never collapses it: every failed edit relays a +// fresh copy and spends a `MONITOR_EVENT_CEILING` slot on a line with no new +// diagnostic. After ~20 such relays a visitor's genuine runtime `DemoError`s are +// silently never reported (`sentry.ts`'s budget is shared across monitor kinds). +// +// The fix keys `stderrSeen` on `normalizeMonitorMessage(message)` — the same +// fingerprint `sentry.ts:263` groups the Sentry issue by — instead of the raw line, so +// the relay and the parent's grouping agree. This file drives that through the real +// keepalive path (`relayStderr` is private; there is nothing else that reaches it). + +import test from "node:test"; +import assert from "node:assert/strict"; +import { ContainerRuntime } from "../packages/runtime/dist/container.js"; + +const settle = () => new Promise((resolve) => setImmediate(resolve)); + +const ENTRY = { + framework: "angular", + displayName: "Angular", + tier: 2, + engine: "container", + sandpackTemplate: null, + sandpackEnvironment: null, + container: "angular", + htWrappers: [], + entry: "/src/app/app.component.ts", + htmlEntry: null, + devCommand: "dev", + buildCommand: "build", + outputDir: "dist", + outputGlob: null, + staticExport: true, + spaMode: false, + port: 4200, + installCommand: "install", + htCoreRange: null, + minCoreMajor: null, + fileCount: 2, + assets: [], + skipped: [], + files: {}, +}; + +const PREVIEW_URL = "https://4200-angular-abc.preview.test/"; + +/** A pointed, monitored runtime with `onStderr` wired before `poll()` (`relayStderr` + * returns early while `stderrCbs` is empty), and a `fetch` stub the keepalive drives + * through the status route. No `document` stub: Node has no global `document`, so + * the `typeof document !== "undefined"` guard short-circuits and the keepalive always + * proceeds — matching `pointed()` in `container-preview-readiness.test.mjs`, which + * doesn't stub it either. */ +async function keptAlive() { + const fetchBefore = globalThis.fetch; + const windowBefore = globalThis.window; + globalThis.window = { + addEventListener() {}, + removeEventListener() {}, + }; + + let currentLog = ""; + const fetches = []; + globalThis.fetch = (url) => { + // Captured per call, not read lazily: an in-flight tick must never observe a + // log that was set after it started, or a later `serve()` could race an + // earlier one's response. + const body = { ready: true, log: currentLog }; + fetches.push(url); + return Promise.resolve({ + ok: true, + status: 200, + json: () => Promise.resolve(body), + clone() { + return this; + }, + }); + }; + + const iframe = { + src: "", + addEventListener() {}, + removeEventListener() {}, + }; + + const runtime = new ContainerRuntime(ENTRY, { + iframe, + apiBase: "https://api.test", + renderGraceMs: 1, + keepaliveMs: 5, + monitor: true, + }); + + const relayed = []; + runtime.onStderr((line) => relayed.push(line)); + + runtime.sessionId = "s1"; + runtime.port = 4200; + runtime.previewUrl = PREVIEW_URL; + + runtime.poll(); + await settle(); + await settle(); + assert.equal(iframe.src, PREVIEW_URL, "the port answered, so the frame is pointed and the keepalive starts"); + + /** Set the status route's log tail and wait for a keepalive tick to consume it + * through `clone().json()` and the relay drain — gated on the stub's own call + * log, not on elapsed time, so a tick that never lands fails loudly instead of + * the assert racing a timer that hasn't fired yet. */ + const serve = async (log) => { + currentLog = log; + const before = fetches.length; + while (fetches.length === before) await settle(); + await settle(); + await settle(); + }; + + return { + runtime, + relayed, + serve, + restore() { + runtime.dispose(); + globalThis.fetch = fetchBefore; + globalThis.window = windowBefore; + }, + }; +} + +const log = (seconds, iso) => + [ + "✘ [ERROR] TS1005: ',' expected. [plugin angular-compiler]", + "", + " src/app/app.component.ts:12:34:", // no marker word — filtered, as today + `Application bundle generation failed. [${seconds} seconds] - ${iso}`, + ].join("\n"); + +const BUILD_1 = log("0.431", "2026-08-25T18:06:09.937Z"); +const BUILD_2 = log("1.595", "2026-08-27T14:20:04.952Z"); + +test("re-running one broken build does not spend a second relay slot", async () => { + // Two keepalive ticks, the same fault, only the clock different. Today this + // relays three events: the TS line once (it dedupes on its own raw text) and the + // envelope twice (its timestamp makes every raw line unique). + const h = await keptAlive(); + try { + await h.serve(BUILD_1); + await h.serve(BUILD_2); + assert.equal(h.relayed.length, 2, JSON.stringify(h.relayed)); + assert.ok(h.relayed.some((m) => m.includes("TS1005")), "the diagnostic line is what must survive"); + // Names the collapsed population directly, rather than inferring it from a + // total, so a tick that never landed fails loudly instead of arithmetically. + assert.equal( + h.relayed.filter((m) => m.includes("Application bundle generation failed")).length, + 1, + "the envelope is one report per fault, not one per rebuild", + ); + } finally { + h.restore(); + } +}); + +test("a genuinely different fault still gets through", async () => { + // Guards the dedupe against over-collapsing: the key must not be so coarse that + // a new compiler error is mistaken for the old one. + const h = await keptAlive(); + try { + await h.serve(BUILD_1); + await h.serve(BUILD_1.replace("TS1005: ','", "TS2304: 'foo'")); + assert.ok(h.relayed.some((m) => m.includes("TS2304")), "a new diagnostic must still be reported"); + } finally { + h.restore(); + } +});