From 7730d9c62c2320243dbbe781f6cca9e50940d675 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 18 Sep 2025 11:03:23 +0200 Subject: [PATCH 1/5] fix(core): Ensure builtin frames do not affect `thirdPartyErrorFilterIntegration` --- .../integrations/third-party-errors-filter.ts | 5 +- .../third-party-errors-filter.test.ts | 63 +++++++++++++++---- 2 files changed, 54 insertions(+), 14 deletions(-) diff --git a/packages/core/src/integrations/third-party-errors-filter.ts b/packages/core/src/integrations/third-party-errors-filter.ts index 1a0628359f5b..59cb6abf08f1 100644 --- a/packages/core/src/integrations/third-party-errors-filter.ts +++ b/packages/core/src/integrations/third-party-errors-filter.ts @@ -108,8 +108,9 @@ function getBundleKeysForAllFramesWithFilenames(event: Event): string[][] | unde return ( frames - // Exclude frames without a filename since these are likely native code or built-ins - .filter(frame => !!frame.filename) + // Exclude frames without a filename or without lineno and colno, + // since these are likely native code or built-ins + .filter(frame => !!frame.filename && (frame.lineno != null || frame.colno != null)) .map(frame => { if (frame.module_metadata) { return Object.keys(frame.module_metadata) diff --git a/packages/core/test/lib/integrations/third-party-errors-filter.test.ts b/packages/core/test/lib/integrations/third-party-errors-filter.test.ts index d68dbaeb5b56..f5df3ddd7e95 100644 --- a/packages/core/test/lib/integrations/third-party-errors-filter.test.ts +++ b/packages/core/test/lib/integrations/third-party-errors-filter.test.ts @@ -32,6 +32,19 @@ const eventWithThirdAndFirstPartyFrames: Event = { function: 'function', lineno: 2, }, + // The following frames are native/built-in frames which should be ignored by the integration + { + function: 'Array.forEach', + filename: '', + abs_path: '', + in_app: true, + }, + { + function: 'async Promise.all', + filename: 'index 1', + abs_path: 'index 1', + in_app: true, + }, ], }, type: 'SyntaxError', @@ -59,6 +72,19 @@ const eventWithOnlyFirstPartyFrames: Event = { function: 'function', lineno: 2, }, + // The following frames are native/built-in frames which should be ignored by the integration + { + function: 'Array.forEach', + filename: '', + abs_path: '', + in_app: true, + }, + { + function: 'async Promise.all', + filename: 'index 1', + abs_path: 'index 1', + in_app: true, + }, ], }, type: 'SyntaxError', @@ -86,6 +112,19 @@ const eventWithOnlyThirdPartyFrames: Event = { function: 'function', lineno: 2, }, + // The following frames are native/built-in frames which should be ignored by the integration + { + function: 'Array.forEach', + filename: '', + abs_path: '', + in_app: true, + }, + { + function: 'async Promise.all', + filename: 'index 1', + abs_path: 'index 1', + in_app: true, + }, ], }, type: 'SyntaxError', @@ -112,7 +151,7 @@ describe('ThirdPartyErrorFilter', () => { }); describe('drop-error-if-contains-third-party-frames', () => { - it('should keep event if there are exclusively first-party frames', async () => { + it('keeps event if there are exclusively first-party frames', async () => { const integration = thirdPartyErrorFilterIntegration({ behaviour: 'drop-error-if-contains-third-party-frames', filterKeys: ['some-key'], @@ -123,7 +162,7 @@ describe('ThirdPartyErrorFilter', () => { expect(result).toBeDefined(); }); - it('should drop event if there is at least one third-party frame', async () => { + it('drops event if there is at least one third-party frame', async () => { const integration = thirdPartyErrorFilterIntegration({ behaviour: 'drop-error-if-contains-third-party-frames', filterKeys: ['some-key'], @@ -134,7 +173,7 @@ describe('ThirdPartyErrorFilter', () => { expect(result).toBe(null); }); - it('should drop event if all frames are third-party frames', async () => { + it('drops event if all frames are third-party frames', async () => { const integration = thirdPartyErrorFilterIntegration({ behaviour: 'drop-error-if-contains-third-party-frames', filterKeys: ['some-key'], @@ -147,7 +186,7 @@ describe('ThirdPartyErrorFilter', () => { }); describe('drop-error-if-exclusively-contains-third-party-frames', () => { - it('should keep event if there are exclusively first-party frames', async () => { + it('keeps event if there are exclusively first-party frames', async () => { const integration = thirdPartyErrorFilterIntegration({ behaviour: 'drop-error-if-exclusively-contains-third-party-frames', filterKeys: ['some-key'], @@ -158,7 +197,7 @@ describe('ThirdPartyErrorFilter', () => { expect(result).toBeDefined(); }); - it('should keep event if there is at least one first-party frame', async () => { + it('keeps event if there is at least one first-party frame', async () => { const integration = thirdPartyErrorFilterIntegration({ behaviour: 'drop-error-if-exclusively-contains-third-party-frames', filterKeys: ['some-key'], @@ -169,7 +208,7 @@ describe('ThirdPartyErrorFilter', () => { expect(result).toBeDefined(); }); - it('should drop event if all frames are third-party frames', async () => { + it('drops event if all frames are third-party frames', async () => { const integration = thirdPartyErrorFilterIntegration({ behaviour: 'drop-error-if-exclusively-contains-third-party-frames', filterKeys: ['some-key'], @@ -182,7 +221,7 @@ describe('ThirdPartyErrorFilter', () => { }); describe('apply-tag-if-contains-third-party-frames', () => { - it('should not tag event if exclusively contains first-party frames', async () => { + it("doesn't tag event if exclusively contains first-party frames", async () => { const integration = thirdPartyErrorFilterIntegration({ behaviour: 'apply-tag-if-contains-third-party-frames', filterKeys: ['some-key'], @@ -193,7 +232,7 @@ describe('ThirdPartyErrorFilter', () => { expect(result?.tags?.third_party_code).toBeUndefined(); }); - it('should tag event if contains at least one third-party frame', async () => { + it('tags event if contains at least one third-party frame', async () => { const integration = thirdPartyErrorFilterIntegration({ behaviour: 'apply-tag-if-contains-third-party-frames', filterKeys: ['some-key'], @@ -204,7 +243,7 @@ describe('ThirdPartyErrorFilter', () => { expect(result?.tags).toMatchObject({ third_party_code: true }); }); - it('should tag event if contains exclusively third-party frames', async () => { + it('tags event if contains exclusively third-party frames', async () => { const integration = thirdPartyErrorFilterIntegration({ behaviour: 'apply-tag-if-contains-third-party-frames', filterKeys: ['some-key'], @@ -217,7 +256,7 @@ describe('ThirdPartyErrorFilter', () => { }); describe('apply-tag-if-exclusively-contains-third-party-frames', () => { - it('should not tag event if exclusively contains first-party frames', async () => { + it("doesn't tag event if exclusively contains first-party frames", async () => { const integration = thirdPartyErrorFilterIntegration({ behaviour: 'apply-tag-if-exclusively-contains-third-party-frames', filterKeys: ['some-key'], @@ -228,7 +267,7 @@ describe('ThirdPartyErrorFilter', () => { expect(result?.tags?.third_party_code).toBeUndefined(); }); - it('should not tag event if contains at least one first-party frame', async () => { + it("doesn't tag event if contains at least one first-party frame", async () => { const integration = thirdPartyErrorFilterIntegration({ behaviour: 'apply-tag-if-exclusively-contains-third-party-frames', filterKeys: ['some-key'], @@ -239,7 +278,7 @@ describe('ThirdPartyErrorFilter', () => { expect(result?.tags?.third_party_code).toBeUndefined(); }); - it('should tag event if contains exclusively third-party frames', async () => { + it('tags event if contains exclusively third-party frames', async () => { const integration = thirdPartyErrorFilterIntegration({ behaviour: 'apply-tag-if-exclusively-contains-third-party-frames', filterKeys: ['some-key'], From 6640038b64523f68a0ec0af0aa64098173ca14f5 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 18 Sep 2025 12:29:26 +0200 Subject: [PATCH 2/5] add integration tests --- .../thirdPartyErrorsFilter/init.js | 34 +++++++++ .../thirdPartyErrorsFilter/subject.js | 28 +++++++ .../thirdPartyErrorsFilter/template.html | 10 +++ .../thirdPartyErrorsFilter/test.ts | 75 +++++++++++++++++++ .../thirdPartyScript.js | 3 + .../integrations/third-party-errors-filter.ts | 2 +- .../third-party-errors-filter.test.ts | 2 - 7 files changed, 151 insertions(+), 3 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/init.js create mode 100644 dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/template.html create mode 100644 dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/thirdPartyScript.js diff --git a/dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/init.js b/dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/init.js new file mode 100644 index 000000000000..9b8269790d41 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/init.js @@ -0,0 +1,34 @@ +import * as Sentry from '@sentry/browser'; +// eslint-disable-next-line import/no-duplicates +import { thirdPartyErrorFilterIntegration } from '@sentry/browser'; +// eslint-disable-next-line import/no-duplicates +import { captureConsoleIntegration } from '@sentry/browser'; + +// This is the code the bundler plugin would inject to mark the init bundle as a first party module: +var _sentryModuleMetadataGlobal = + typeof window !== 'undefined' + ? window + : typeof global !== 'undefined' + ? global + : typeof self !== 'undefined' + ? self + : {}; + +_sentryModuleMetadataGlobal._sentryModuleMetadata = _sentryModuleMetadataGlobal._sentryModuleMetadata || {}; + +_sentryModuleMetadataGlobal._sentryModuleMetadata[new Error().stack] = Object.assign( + {}, + _sentryModuleMetadataGlobal._sentryModuleMetadata[new Error().stack], + { + '_sentryBundlerPluginAppKey:my-app': true, + }, +); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [ + thirdPartyErrorFilterIntegration({ behaviour: 'apply-tag-if-contains-third-party-frames', filterKeys: ['my-app'] }), + captureConsoleIntegration({ levels: ['error'], handled: false }), + ], + attachStacktrace: true, +}); diff --git a/dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/subject.js b/dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/subject.js new file mode 100644 index 000000000000..0a70d1f25c42 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/subject.js @@ -0,0 +1,28 @@ +// This is the code the bundler plugin would inject to mark the subject bundle as a first party module: +var _sentryModuleMetadataGlobal = + typeof window !== 'undefined' + ? window + : typeof global !== 'undefined' + ? global + : typeof self !== 'undefined' + ? self + : {}; + +_sentryModuleMetadataGlobal._sentryModuleMetadata = _sentryModuleMetadataGlobal._sentryModuleMetadata || {}; + +_sentryModuleMetadataGlobal._sentryModuleMetadata[new Error().stack] = Object.assign( + {}, + _sentryModuleMetadataGlobal._sentryModuleMetadata[new Error().stack], + { + '_sentryBundlerPluginAppKey:my-app': true, + }, +); + +const errorBtn = document.getElementById('errBtn'); +errorBtn.addEventListener('click', async () => { + Promise.allSettled([Promise.reject('I am a first party Error')]).then(values => + values.forEach(value => { + if (value.status === 'rejected') console.error(value.reason); + }), + ); +}); diff --git a/dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/template.html b/dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/template.html new file mode 100644 index 000000000000..25a91142be08 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/template.html @@ -0,0 +1,10 @@ + + + + + + + + + + diff --git a/dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/test.ts b/dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/test.ts new file mode 100644 index 000000000000..229813eed9a0 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/test.ts @@ -0,0 +1,75 @@ +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { expect } from '@playwright/test'; +import { sentryTest } from '../../../utils/fixtures'; +import { envelopeRequestParser, waitForErrorRequest } from '../../../utils/helpers'; + +sentryTest('tags event if contains at least one third-party frame', async ({ getLocalTestUrl, page }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + + const errorEventPromise = waitForErrorRequest(page, e => { + return e.exception?.values?.[0]?.value === 'I am a third party Error'; + }); + + await page.route('**/thirdPartyScript.js', route => + route.fulfill({ + status: 200, + body: readFileSync(join(__dirname, 'thirdPartyScript.js')), + }), + ); + + await page.goto(url); + + const errorEvent = envelopeRequestParser(await errorEventPromise); + expect(errorEvent.tags?.third_party_code).toBe(true); +}); + +/** + * This test seems a bit more complicated than necessary but this is intentional: + * When using `captureConsoleIntegration` in combination with `thirdPartyErrorFilterIntegration` + * and `attachStacktrace: true`, the stack trace includes native code stack frames which previously broke + * the third party error filtering logic. + * + * see https://github.com/getsentry/sentry-javascript/issues/17674 + */ +sentryTest( + "doesn't tag event if doesn't contain third-party frames", + async ({ getLocalTestUrl, page, browserName }) => { + const url = await getLocalTestUrl({ testDir: __dirname }); + + const errorEventPromise = waitForErrorRequest(page, e => { + return e.exception?.values?.[0]?.value === 'I am a first party Error'; + }); + + await page.route('**/thirdPartyScript.js', route => + route.fulfill({ + status: 200, + body: readFileSync(join(__dirname, 'thirdPartyScript.js')), + }), + ); + + await page.goto(url); + + await page.click('#errBtn'); + + const errorEvent = envelopeRequestParser(await errorEventPromise); + + expect(errorEvent.tags?.third_party_code).toBeUndefined(); + + // ensure the stack trace includes native code stack frames which previously broke + // the third party error filtering logic + if (browserName === 'chromium') { + expect(errorEvent.exception?.values?.[0]?.stacktrace?.frames).toContainEqual({ + filename: '', + function: 'Array.forEach', + in_app: true, + }); + } else if (browserName === 'webkit') { + expect(errorEvent.exception?.values?.[0]?.stacktrace?.frames).toContainEqual({ + filename: '[native code]', + function: 'forEach', + in_app: true, + }); + } + }, +); diff --git a/dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/thirdPartyScript.js b/dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/thirdPartyScript.js new file mode 100644 index 000000000000..e6a2e35dba01 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/thirdPartyScript.js @@ -0,0 +1,3 @@ +setTimeout(() => { + throw new Error('I am a third party Error'); +}, 100); diff --git a/packages/core/src/integrations/third-party-errors-filter.ts b/packages/core/src/integrations/third-party-errors-filter.ts index 59cb6abf08f1..15f8ab688c69 100644 --- a/packages/core/src/integrations/third-party-errors-filter.ts +++ b/packages/core/src/integrations/third-party-errors-filter.ts @@ -110,7 +110,7 @@ function getBundleKeysForAllFramesWithFilenames(event: Event): string[][] | unde frames // Exclude frames without a filename or without lineno and colno, // since these are likely native code or built-ins - .filter(frame => !!frame.filename && (frame.lineno != null || frame.colno != null)) + .filter(frame => !!frame.filename && (frame.lineno || frame.colno) != null) .map(frame => { if (frame.module_metadata) { return Object.keys(frame.module_metadata) diff --git a/packages/core/test/lib/integrations/third-party-errors-filter.test.ts b/packages/core/test/lib/integrations/third-party-errors-filter.test.ts index f5df3ddd7e95..2b5445a4544e 100644 --- a/packages/core/test/lib/integrations/third-party-errors-filter.test.ts +++ b/packages/core/test/lib/integrations/third-party-errors-filter.test.ts @@ -64,10 +64,8 @@ const eventWithOnlyFirstPartyFrames: Event = { colno: 1, filename: __filename, function: 'function', - lineno: 1, }, { - colno: 2, filename: __filename, function: 'function', lineno: 2, From e0ba83c348b3afa555cb0b448a0fb2fe2be16256 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 18 Sep 2025 12:39:41 +0200 Subject: [PATCH 3/5] whoops --- packages/core/src/integrations/third-party-errors-filter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/integrations/third-party-errors-filter.ts b/packages/core/src/integrations/third-party-errors-filter.ts index 15f8ab688c69..7d742d5c76ea 100644 --- a/packages/core/src/integrations/third-party-errors-filter.ts +++ b/packages/core/src/integrations/third-party-errors-filter.ts @@ -110,7 +110,7 @@ function getBundleKeysForAllFramesWithFilenames(event: Event): string[][] | unde frames // Exclude frames without a filename or without lineno and colno, // since these are likely native code or built-ins - .filter(frame => !!frame.filename && (frame.lineno || frame.colno) != null) + .filter(frame => !!frame.filename && (frame.lineno ?? frame.colno) != null) .map(frame => { if (frame.module_metadata) { return Object.keys(frame.module_metadata) From 20e9de8476b576660433ebf47f69933f4ecc3565 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 18 Sep 2025 14:31:59 +0200 Subject: [PATCH 4/5] skip bundle tests --- .../suites/integrations/thirdPartyErrorsFilter/test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/test.ts b/dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/test.ts index 229813eed9a0..1d2729681029 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/test.ts +++ b/dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/test.ts @@ -4,6 +4,12 @@ import { expect } from '@playwright/test'; import { sentryTest } from '../../../utils/fixtures'; import { envelopeRequestParser, waitForErrorRequest } from '../../../utils/helpers'; +const bundle = process.env.PW_BUNDLE || ''; +// We only want to run this in non-CDN bundle mode +if (bundle.startsWith('bundle')) { + sentryTest.skip(); +} + sentryTest('tags event if contains at least one third-party frame', async ({ getLocalTestUrl, page }) => { const url = await getLocalTestUrl({ testDir: __dirname }); From f198f839b2633bc42d463a36a159365fc51d3788 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 18 Sep 2025 14:32:36 +0200 Subject: [PATCH 5/5] . --- .../suites/integrations/thirdPartyErrorsFilter/test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/test.ts b/dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/test.ts index 1d2729681029..9b918e8d1170 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/test.ts +++ b/dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/test.ts @@ -5,7 +5,8 @@ import { sentryTest } from '../../../utils/fixtures'; import { envelopeRequestParser, waitForErrorRequest } from '../../../utils/helpers'; const bundle = process.env.PW_BUNDLE || ''; -// We only want to run this in non-CDN bundle mode +// We only want to run this in non-CDN bundle mode because +// thirdPartyErrorFilterIntegration is only available in the NPM package if (bundle.startsWith('bundle')) { sentryTest.skip(); }