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..9b918e8d1170 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/thirdPartyErrorsFilter/test.ts @@ -0,0 +1,82 @@ +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'; + +const bundle = process.env.PW_BUNDLE || ''; +// 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(); +} + +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 1a0628359f5b..7d742d5c76ea 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 ?? 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..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 @@ -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', @@ -51,14 +64,25 @@ const eventWithOnlyFirstPartyFrames: Event = { colno: 1, filename: __filename, function: 'function', - lineno: 1, }, { - colno: 2, filename: __filename, 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 +110,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 +149,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 +160,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 +171,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 +184,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 +195,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 +206,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 +219,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 +230,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 +241,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 +254,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 +265,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 +276,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'],