Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 120 additions & 0 deletions src/integrations/datadog/shouldKeepEvent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,126 @@ describe('shouldKeepEvent', () => {
});
});

// Regression tests for #1696, on the stacks RUM recorded for GTM's LinkedIn Insight tag. The guard
// frame is `installBrowserTranslationDomGuard`'s `insertBefore`, which `vite.config.ts` builds into
// the Datadog chunk; each browser names it differently.
describe('vendor tags injected through the DOM guard', () => {
const gtm = 'https://www.googletagmanager.com/gtm.js?id=GTM-5QQX432';
const guardChunk = 'https://fabric.harper.fast/assets/vendor-datadog-CLUcJXOo.js:3:3191';
const lintrk = (guardFrame: string, inlineFrame = ' at <anonymous> @ <anonymous>:1:8') =>
errorEvent({
type: 'TypeError',
source: 'source',
message: 'window.lintrk is not a function',
stack: [
'TypeError: window.lintrk is not a function',
inlineFrame,
guardFrame,
` at <anonymous> @ ${gtm}:546:207`,
` at WS @ ${gtm}:552:412`,
` at Qd.VS @ ${gtm}:552:96`,
].join('\n'),
});

it.each([
` at Node.insertBefore @ ${guardChunk}`,
` at QI.Node.insertBefore @ ${guardChunk}`,
` at <anonymous> @ ${guardChunk}`,
])('discards a tag that throws while GTM inserts it (%s)', (guardFrame) => {
expect(shouldKeepEvent(lintrk(guardFrame))).toBe(false);
});

// Firefox names the inline script after the page, in a form that carries no parseable URL.
it('discards the Firefox shape', () => {
expect(
shouldKeepEvent(
lintrk(
` at QI/Node.prototype.insertBefore @ ${guardChunk}`,
' at <anonymous> @ https://fabric.harper.fast/#/sign-in line 829 > injectedScript:1:8',
),
),
).toBe(false);
});

// The neutral frame is what the build placement buys: in the app entry it is a Studio frame.
it('keeps the same stack while the guard is built into the app entry', () => {
expect(
shouldKeepEvent(
lintrk(' at QI.Node.insertBefore @ https://fabric.harper.fast/assets/index-BmfggGwq.js:829:50787'),
),
).toBe(true);
});

it('keeps a Studio error that passes through the guard', () => {
expect(
shouldKeepEvent(
errorEvent({
type: 'HierarchyRequestError',
source: 'source',
message: "Failed to execute 'insertBefore' on 'Node': The new child element contains the parent.",
stack: [
"HierarchyRequestError: Failed to execute 'insertBefore' on 'Node': The new child element contains the parent.",
` at QI.Node.insertBefore @ ${guardChunk}`,
' at Ua @ https://fabric.harper.fast/assets/vendor-react-B2kMp9xQ.js:8:41022',
' at Gs @ https://fabric.harper.fast/assets/vendor-react-B2kMp9xQ.js:8:47135',
].join('\n'),
}),
),
).toBe(true);
expect(
shouldKeepEvent(
errorEvent({
message: 'Boom',
stack: [
'TypeError: Boom',
' at <anonymous> @ <anonymous>:1:8',
` at QI.Node.insertBefore @ ${guardChunk}`,
' at useGTM @ https://fabric.harper.fast/assets/index-BmfggGwq.js:5:1234',
].join('\n'),
}),
),
).toBe(true);
});

it('keeps a stack whose only located frame is the guard', () => {
expect(
shouldKeepEvent(
errorEvent({
message: 'Boom',
stack: ['TypeError: Boom', ` at QI.Node.insertBefore @ ${guardChunk}`].join('\n'),
}),
),
).toBe(true);
});

it('discards a stack of nothing but GTM frames', () => {
expect(
shouldKeepEvent(
errorEvent({
message: 'Failed to fetch',
stack: [
'TypeError: Failed to fetch',
` at <anonymous> @ ${guardChunk}`,
` at wp.sendRequest @ ${gtm}:463:308`,
` at Qu @ ${gtm}:470:87`,
].join('\n'),
}),
),
).toBe(false);
});

it('keeps a frame on a host that merely starts with the GTM host', () => {
expect(
shouldKeepEvent(
errorEvent({
message: 'Boom',
stack: ['TypeError: Boom', ' at x @ https://www.googletagmanager.com.example.net/gtm.js:1:1'].join('\n'),
}),
),
).toBe(true);
});
});

// Regression tests for #1645. Stacks below are the real ones RUM recorded, truncated.
describe('browser extension content scripts', () => {
it('discards errors whose stack is nothing but extension frames', () => {
Expand Down
12 changes: 8 additions & 4 deletions src/integrations/datadog/shouldKeepEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,12 @@ export interface DatadogErrorEvent {
}

/**
* Scripts we embed but do not ship or control — currently only the Reo.dev analytics
* bundle (`src/integrations/reo/reo.ts` loads it from this CDN). Errors thrown inside
* them are vendor bugs we can neither reproduce nor fix.
* Scripts we embed but do not ship or control: the Reo.dev analytics bundle
* (`src/integrations/reo/reo.ts`) and Google Tag Manager (`src/integrations/google/gtm.ts`), whose
* container injects the marketing tags. Errors thrown inside them are vendor bugs we can neither
* reproduce nor fix.
*/
const THIRD_PARTY_SCRIPT_FRAME = /^https?:\/\/static\.reo\.dev\//;
const THIRD_PARTY_SCRIPT_FRAME = /^https?:\/\/(?:static\.reo\.dev|www\.googletagmanager\.com)\//;

/**
* A content script the user's own browser extension injected into the page. Matched on the
Expand Down Expand Up @@ -80,6 +81,9 @@ const NESTED_ORIGIN_PREFIX = /^blob:/;
* The Datadog browser SDK monkey-patches `fetch`/XHR, so its own bundle sits at the top
* of every stack it instruments — including stacks whose real origin is third-party.
* Treat it as instrumentation rather than as a Studio frame when attributing an error.
* `vite.config.ts` puts `installBrowserTranslationDomGuard` in this chunk for the same reason:
* every script that inserts a node runs through its `insertBefore`, so a vendor tag that throws
* while GTM injects it would otherwise carry a Studio frame (#1696).
*/
const INSTRUMENTATION_FRAME = /\/assets\/vendor-datadog-[^/\s]*\.js/;

Expand Down
3 changes: 2 additions & 1 deletion tsconfig.node.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"noUncheckedSideEffectImports": true
},
"include": [
"vite.config.ts"
"vite.config.ts",
"vite.config.test.ts"
]
}
31 changes: 31 additions & 0 deletions vite.config.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import path from 'node:path';
import { describe, expect, it } from 'vitest';
import viteConfig from './vite.config';

function buildOutput() {
const config = viteConfig({ mode: 'prod', command: 'build', isSsrBuild: false, isPreview: false });
if (typeof config === 'function' || config instanceof Promise) {
throw new Error('vite.config.ts no longer returns a plain config object');
}
return config.build?.rollupOptions?.output as { manualChunks?: unknown; chunkFileNames?: unknown };
}

// `shouldKeepEvent` reads a `vendor-datadog` frame as neutral instrumentation (#1696).
describe('manualChunks', () => {
it('builds the translation DOM guard into the Datadog chunk', () => {
const { manualChunks, chunkFileNames } = buildOutput();
const guard = path.resolve(__dirname, 'src/lib/installBrowserTranslationDomGuard.ts');
const chunkOf = manualChunks as (id: string) => string | undefined;

expect(chunkOf(guard)).toBe('vendor-datadog');
expect(chunkOf(path.resolve(__dirname, 'node_modules/@datadog/browser-rum/esm/index.js'))).toBe('vendor-datadog');
expect(chunkFileNames).toBe('assets/[name]-[hash].js');
});

it('leaves the rest of Studio to the automatic split', () => {
const chunkOf = buildOutput().manualChunks as (id: string) => string | undefined;

expect(chunkOf(path.resolve(__dirname, 'src/main.tsx'))).toBeUndefined();
expect(chunkOf(path.resolve(__dirname, 'src/lib/installStaleDeployReload.ts'))).toBeUndefined();
});
});
6 changes: 6 additions & 0 deletions vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,12 @@ export default defineConfig(({ mode }) => ({
// Forcing them into manual chunks does the opposite — it promotes them to
// eager — so leave them to auto-split.
manualChunks(id) {
// The translation DOM guard wraps `Node.prototype.insertBefore`, so it is on the stack of
// every script that inserts a node, vendor tags included. `shouldKeepEvent` treats frames
// from this chunk as neutral instrumentation; in the app entry they would read as Studio's.
if (id.replace(/\\/g, '/').endsWith('/src/lib/installBrowserTranslationDomGuard.ts')) {
return 'vendor-datadog';
}
Comment on lines +211 to +213

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Vite/Rollup can append query parameters to module IDs during resolution or transformation (e.g., ?worker, ?commonjs-entry, or other plugin-specific flags). If a query parameter is appended, endsWith on the raw id will fail to match, causing the translation DOM guard to be bundled into the main chunk instead of vendor-datadog. Stripping any query parameters before performing the path check ensures the bundling logic remains robust.

Suggested change
if (id.replace(/\\/g, '/').endsWith('/src/lib/installBrowserTranslationDomGuard.ts')) {
return 'vendor-datadog';
}
const cleanId = id.split('?')[0].replace(/\\/g, '/');
if (cleanId.endsWith('/src/lib/installBrowserTranslationDomGuard.ts')) {
return 'vendor-datadog';
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Declining. The id this rule has to match never has a query. The guard is imported in exactly one place, a plain import … from '@/lib/installBrowserTranslationDomGuard' at src/main.tsx:5, so its module id is the bare file path. I checked that in a real build (described under Verification in the PR body): vite build --mode prod put Node.prototype.insertBefore=function(e,t){…} in vendor-datadog-*.js, and index-*.js contained no copy of it.

A query-suffixed id like ?raw, ?url or ?worker belongs to a different module that never runs the patch, so stripping the query would only pull that copy into the Datadog chunk. vite.config.test.ts pins how the real file path is placed.

🤖 Generated by Claude Code (Claude Opus 5.5); posted via @dawsontoth.

if (!id.includes('node_modules')) { return; }

// Resolve the npm package name from the module path so rules match on
Expand Down
Loading