From 0c92d58e2b6004ff9b8c1cef6bb9ee4e0d15c844 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Sat, 10 Oct 2026 22:31:43 -0400 Subject: [PATCH 1/2] refactor(datadog): remove shouldKeepEvent's instance-endpoint gate, which no RUM error can reach The two branches gated on `error.resource.url` matching `/(HDBInstance|Cluster)//operation` never fire: the browser RUM SDK (7.15.0) records a failed request as a `resource` event, never as an error. `ErrorSource.NETWORK` is declared but nothing emits it, and `processError` builds no `error.resource`. Production agrees: 0 of 4,407 error events in the last 30 days carry `@error.resource.url` or `@error.source:network`, against 11,568 status-0 resources. Replaying 1,063 of those events through the old and new predicate gives the same verdict for all of them. The header now states what actually reaches the filter, so the next reader does not reason from a resource-error path that does not exist (as the #1680 review did). Console-relayed 5xx and `Network Error` AxiosErrors are still kept; whether to drop them is #1669. Closes #1690 Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 6 +- src/integrations/datadog/beforeSend.test.ts | 5 +- .../datadog/shouldKeepEvent.test.ts | 91 +++++-------------- src/integrations/datadog/shouldKeepEvent.ts | 49 +++------- 4 files changed, 38 insertions(+), 113 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index ac6c2c8fc..2d9d0a7ef 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -689,9 +689,9 @@ For a hook whose job is redaction, that is the one failure mode that matters, an instead of testing it for truthiness. Same reason the URL redaction runs **before** `shouldKeepEvent` and the error-text redaction -**after** it. The filter attributes errors on the raw stack and raw `error.resource.url`, so those -can't be rewritten ahead of it — but it reads no URL field, and it can itself throw on a malformed -error field, which would ship the event with its URLs untouched. +**after** it. The filter attributes errors on the raw stack, so that can't be rewritten ahead of +it — but it reads no URL field, and it can itself throw on a malformed error field, which would +ship the event with its URLs untouched. `error.stack` is **not** the engine's stack string. The SDK parses it and re-serializes every stack it records, so what reaches `beforeSend` is always this shape, whatever the engine produced — no V8 diff --git a/src/integrations/datadog/beforeSend.test.ts b/src/integrations/datadog/beforeSend.test.ts index 60642d99d..86a062a48 100644 --- a/src/integrations/datadog/beforeSend.test.ts +++ b/src/integrations/datadog/beforeSend.test.ts @@ -284,9 +284,8 @@ describe('beforeSend', () => { expect(beforeSend(event)).toBe(true); }); - // Each field is read twice, by the filter and by the redaction, so one case pins both guards. The - // 5xx message makes the filter coerce `resource.url`: under `?? ''` the array stringifies into an - // instance endpoint and the event is silently dropped rather than kept. + // The filter and the redaction both read `stack`, so that case pins both guards; the redaction alone + // reads the other two. it.each([ 'stack', 'handling_stack', diff --git a/src/integrations/datadog/shouldKeepEvent.test.ts b/src/integrations/datadog/shouldKeepEvent.test.ts index ba1c21e31..58657a408 100644 --- a/src/integrations/datadog/shouldKeepEvent.test.ts +++ b/src/integrations/datadog/shouldKeepEvent.test.ts @@ -160,27 +160,6 @@ describe('shouldKeepEvent', () => { ).toBe(false); }); - // The name gate above deliberately does not fire for a nameless timeout, so this is the only - // thing dropping the tracked *resource* errors, which need not carry `type: 'AxiosError'`. - it('discards timeout errors against instance/cluster resource URLs, in either spelling', () => { - expect( - shouldKeepEvent( - errorEvent({ - message: 'timeout of 60000ms exceeded', - resource: { url: 'https://api.harper.fast/HDBInstance/ins-123/operation' }, - }), - ), - ).toBe(false); - expect( - shouldKeepEvent( - errorEvent({ - message: 'timeout exceeded', - resource: { url: 'https://api.harper.fast/Cluster/clu-123/operation' }, - }), - ), - ).toBe(false); - }); - // Pins that the name gate only ever adds drops: the numeric spelling is still dropped without // a name, so a timeout Studio re-wraps into a plain `Error` (`secrets.ts`) behaves as before, // while the bare spelling — indistinguishable from prose without a name — is reported. @@ -191,22 +170,6 @@ describe('shouldKeepEvent', () => { expect(shouldKeepEvent(errorEvent({ message: 'timeout exceeded' }))).toBe(true); }); - it('discards network failures against an instance/cluster operation endpoint', () => { - expect( - shouldKeepEvent( - errorEvent({ - message: 'Network Error', - resource: { url: 'https://api.harper.fast/Cluster/clu-123/operation' }, - }), - ), - ).toBe(false); - expect( - shouldKeepEvent( - errorEvent({ source: 'network', resource: { url: 'https://api.harper.fast/HDBInstance/ins-9/operation' } }), - ), - ).toBe(false); - }); - // Regression test for issue #1386: a session expiring / signing out mid-poll leaves // an in-flight request that 401s; these reach RUM as handled AxiosErrors with no URL. it('discards 401 auth failures on the message alone but keeps 403', () => { @@ -243,28 +206,29 @@ describe('shouldKeepEvent', () => { expect(shouldKeepEvent(errorEvent({ message: 'Failed to deploy component: invalid package' }))).toBe(true); }); - it('discards 5xx errors against an instance/cluster operation endpoint', () => { - expect( - shouldKeepEvent( - errorEvent({ - message: 'AxiosError: Request failed with status code 500', - resource: { url: 'https://api.harper.fast/HDBInstance/ins-123/operation' }, - }), - ), - ).toBe(false); - }); + // #1669 owns whether these console-relayed request failures should be dropped. Until then they + // are kept, and nothing about the request can change that: the SDK never names one (#1690). + describe('request failures', () => { + const instanceOperationUrls = [ + 'https://api.harper.fast/HDBInstance/ins-123/operation', + 'https://api.harper.fast/Cluster/clu-123/operation', + ]; - it('keeps 5xx errors that are not attributable to an instance/cluster operation endpoint', () => { - // An unattributed 5xx could be a genuine backend failure worth surfacing. - expect(shouldKeepEvent(errorEvent({ message: 'Request failed with status code 500' }))).toBe(true); - expect( - shouldKeepEvent( - errorEvent({ - message: 'Request failed with status code 502', - resource: { url: 'https://api.harper.fast/Organization/org-1' }, - }), - ), - ).toBe(true); + it.each([ + 'AxiosError: Request failed with status code 500', + 'AxiosError: Request failed with status code 502', + 'AxiosError: Network Error', + ])('keeps a console-relayed %s', (message) => { + expect(shouldKeepEvent(errorEvent({ type: 'AxiosError', source: 'console', message }))).toBe(true); + }); + + it.each(instanceOperationUrls)('keeps them with an error.resource.url of %s', (url) => { + for (const message of ['Request failed with status code 500', 'Network Error', 'timeout exceeded']) { + expect(shouldKeepEvent(errorEvent({ message }))).toBe(true); + expect(shouldKeepEvent(errorEvent({ message, resource: { url } }))).toBe(true); + } + expect(shouldKeepEvent(errorEvent({ source: 'network', resource: { url } }))).toBe(true); + }); }); // Regression tests for the Reo.dev analytics errors that reached Error Tracking on @@ -712,15 +676,4 @@ describe('shouldKeepEvent', () => { ).toBe(true); }); }); - - it('keeps non-timeout network failures that are not attributable to an instance endpoint', () => { - // Without an instance/cluster URL we cannot tell a real backend failure from an - // expected one, so a bare "Network Error" stays visible (unlike timeouts). - expect(shouldKeepEvent(errorEvent({ message: 'Network Error' }))).toBe(true); - expect( - shouldKeepEvent( - errorEvent({ message: 'Network Error', resource: { url: 'https://api.harper.fast/Organization/org-1' } }), - ), - ).toBe(true); - }); }); diff --git a/src/integrations/datadog/shouldKeepEvent.ts b/src/integrations/datadog/shouldKeepEvent.ts index da7f185ca..974acbffa 100644 --- a/src/integrations/datadog/shouldKeepEvent.ts +++ b/src/integrations/datadog/shouldKeepEvent.ts @@ -114,16 +114,14 @@ function originatesInThirdPartyScript(stack: string, type: string | undefined, m * Datadog `beforeSend` predicate: returns true to keep the event, false to discard * it (false is what tells RUM to drop the event). Wired straight into `beforeSend`. * - * Reaching an instance can legitimately fail when it's down, restarting, or the - * user lacks Fabric Connect — these are expected states, not Studio bugs, and they - * otherwise flood Error Tracking. We drop connectivity-class errors (timeouts, - * network failures) so they don't create issues. - * - * These errors reach RUM two ways: as tracked resource errors (which carry an - * `error.resource.url`) and as handled AxiosErrors surfaced through `console.error` - * — most notably React Query's global error handler — which do NOT. The URL-based - * endpoint check below only ever sees the former, so the URL-less timeouts slipped - * past it and flooded Error Tracking (issue #1371). + * The browser RUM SDK makes an error event from a thrown exception (`source: 'source'`), a + * `console.error` call (`'console'`), `addError` (`'custom'` — `addReactError` in `main.tsx`) or a + * browser report (`'report'`). A failed request is a `resource` event, never an error: + * `ErrorSource.NETWORK` is declared but nothing emits it, and `processError` + * (`browser-rum-core/…/errorCollection`) builds no `error.resource`. So a handled request failure + * reaches this filter only as whatever logged it, mostly React Query's global handler, with nothing + * naming the endpoint. Attribute by message, name and stack; an unreachable instance is visible only + * as `@type:resource @resource.status_code:0`. */ export function shouldKeepEvent(event: DatadogErrorEvent) { if (event.type !== 'error') { @@ -132,11 +130,9 @@ export function shouldKeepEvent(event: DatadogErrorEvent) { // Type-checked, not `?? ''`: a malformed field would throw out of this filter, which the SDK // swallows into shipping the event unfiltered. const message = typeof event.error?.message === 'string' ? event.error.message : ''; - const source = event.error?.source; // Aborted requests are client-initiated cancellations (navigating away, a // component unmounting, React Query cancelling an in-flight query) — not failures. - // Safe to drop globally regardless of endpoint. if (/Request aborted/i.test(message)) { return false; } @@ -215,11 +211,9 @@ export function shouldKeepEvent(event: DatadogErrorEvent) { // A request timeout is a connectivity-class failure, never a Studio bug: the instance, // cluster, or backend was too slow to answer, and backend latency is tracked server-side. - // The handled AxiosErrors that flood Error Tracking (#1371) carry no resource URL, so the - // endpoint gate below cannot reach them. The bare spelling needs the name because on its - // own it would also discard a Harper-relayed `Operation timeout exceeded`; the numeric one - // stays name-free, so a timeout Studio re-wraps into a plain `Error` (`secrets.ts`) is - // dropped exactly as before. + // The bare spelling needs the name because on its own it would also discard a Harper-relayed + // `Operation timeout exceeded`; the numeric one stays name-free, so a timeout Studio re-wraps + // into a plain `Error` (`secrets.ts`) is dropped exactly as before. if ( NUMERIC_TIMEOUT_MESSAGE.test(message) || (errorName === 'AxiosError' && BARE_TIMEOUT_MESSAGE.test(message)) @@ -256,26 +250,5 @@ export function shouldKeepEvent(event: DatadogErrorEvent) { return false; } - const url = typeof event.error?.resource?.url === 'string' ? event.error.resource.url : ''; - const isInstanceEndpoint = /\/(HDBInstance|Cluster)\/[^/]+\/operation/.test(url); - - // A 5xx or a bare timeout from an instance/cluster operation endpoint is the instance itself - // failing to answer (broken, unsupported, or mid-restart) — a server-side condition tracked - // on the backend, not a Studio bug. The URL attributes it whatever the thrown value's name - // is, which is what reaches the tracked resource errors the name gate above cannot. - if ( - isInstanceEndpoint - && (/Request failed with status code 5\d\d\b/i.test(message) || BARE_TIMEOUT_MESSAGE.test(message)) - ) { - return false; - } - - // Other network failures (connection refused, DNS, offline) against an - // instance/cluster operation endpoint — expected when the instance is unreachable. - const isNetworkFailure = /Network Error/i.test(message) || source === 'network'; - if (isInstanceEndpoint && isNetworkFailure) { - return false; - } - return true; } From 99b33932b1ff4f66011c70b7ab5ed7ea5447d783 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Sat, 10 Oct 2026 22:39:01 -0400 Subject: [PATCH 2/2] fix(datadog): stop attributing GTM-injected vendor tag errors to Studio through the DOM guard frame GTM's LinkedIn Insight snippet calls `window.lintrk` before `snap.licdn.com` has loaded, and the `TypeError` was kept as a Studio error: 15 events, 4 sessions in 30 days, every one shaped `:1:8` -> `Node.insertBefore @ /assets/index-*.js` -> ~30 googletagmanager.com frames. Two frames on that stack scored as Studio's, and either one alone kept it: - `installBrowserTranslationDomGuard` (#1388) patches `Node.prototype.insertBefore` globally, so every script that inserts a node runs through it, and an inline `