From 0a26b1c4fbb1a05d0d5ac8613cbffbdd193c4337 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Mon, 22 Apr 2024 09:36:55 +0100 Subject: [PATCH 1/8] ref(test): Trigger top-level errors inside sandbox. --- .../browser-integration-tests/README.md | 6 ++-- .../onError/non-string-arg/subject.js | 8 ----- .../onError/non-string-arg/test.ts | 15 ++++++-- .../onError/rethrown/subject.js | 17 --------- .../instrumentation/onError/rethrown/test.ts | 31 ++++++++-------- .../onError/syntax-errors/subject.js | 10 ------ .../onError/syntax-errors/test.ts | 12 +++++-- .../onError/thrown-errors/subject.js | 10 ------ .../onError/thrown-errors/test.ts | 12 +++++-- .../onError/thrown-objects/subject.js | 10 ------ .../onError/thrown-objects/test.ts | 14 ++++++-- .../onError/thrown-strings/subject.js | 10 ------ .../onError/thrown-strings/test.ts | 12 +++++-- .../startSpan/error-sync/subject.js | 9 ----- .../public-api/startSpan/error-sync/test.ts | 24 ++++++++++--- .../error/subject.js | 3 -- .../browserTracingIntegration/error/test.ts | 12 ++++++- .../utils/helpers.ts | 35 +++++++------------ 18 files changed, 117 insertions(+), 133 deletions(-) delete mode 100644 dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/subject.js delete mode 100644 dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/subject.js delete mode 100644 dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/subject.js delete mode 100644 dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/subject.js delete mode 100644 dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-objects/subject.js delete mode 100644 dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/subject.js delete mode 100644 dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/subject.js delete mode 100644 dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/subject.js diff --git a/dev-packages/browser-integration-tests/README.md b/dev-packages/browser-integration-tests/README.md index b24bdfb86ee3..0d943e8de5f0 100644 --- a/dev-packages/browser-integration-tests/README.md +++ b/dev-packages/browser-integration-tests/README.md @@ -14,7 +14,7 @@ or `init.js` is not defined in a case folder. `subject.js` contains the logic that sets up the environment to be tested. It also can be defined locally and as a group fallback. Unlike `template.hbs` and `init.js`, it's not required to be defined for a group, as there may be cases that -does not require a subject, instead the logic is injected using `injectScriptAndGetEvents` from `utils/helpers.ts`. +does not require a subject. `test.ts` is required for each test case, which contains the assertions (and if required the script injection logic). For every case, any set of `init.js`, `template.hbs` and `subject.js` can be defined locally, and each one of them will @@ -22,8 +22,8 @@ have precedence over the default definitions of the test group. To test page multi-page navigations, you can specify additional `page-*.html` (e.g. `page-0.html`, `page-1.html`) files. These will also be compiled and initialized with the same `init.js` and `subject.js` files that are applied to -`template.hbs/html`. Note: `page-*.html` file lookup **doesn not** fall back to the parent directories, meaning that -page files have to be directly in the `test.ts` directory. +`template.hbs/html`. Note: `page-*.html` file lookup **does not** fall back to the parent directories, meaning that page +files have to be directly in the `test.ts` directory. ``` suites/ diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/subject.js b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/subject.js deleted file mode 100644 index ad794db023df..000000000000 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/subject.js +++ /dev/null @@ -1,8 +0,0 @@ -function run() { - window.onerror({ - type: 'error', - otherKey: 'hi', - }); -} - -run(); diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/test.ts index ed2399a43790..9013c744333d 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/test.ts @@ -2,14 +2,25 @@ import { expect } from '@playwright/test'; import type { Event } from '@sentry/types'; import { sentryTest } from '../../../../../utils/fixtures'; -import { getFirstSentryEnvelopeRequest } from '../../../../../utils/helpers'; +import { getFirstSentryEnvelopeRequest, runScriptInSandbox } from '../../../../../utils/helpers'; sentryTest( 'should catch onerror calls with non-string first argument gracefully', async ({ getLocalTestPath, page }) => { const url = await getLocalTestPath({ testDir: __dirname }); - const eventData = await getFirstSentryEnvelopeRequest(page, url); + await page.goto(url); + + runScriptInSandbox(page, { + content: ` + throw { + type: 'Error', + otherKey: 'otherValue', + }; + `, + }); + + const eventData = await getFirstSentryEnvelopeRequest(page); expect(eventData.exception?.values).toHaveLength(1); expect(eventData.exception?.values?.[0]).toMatchObject({ diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/subject.js b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/subject.js deleted file mode 100644 index 44a6e2d739c5..000000000000 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/subject.js +++ /dev/null @@ -1,17 +0,0 @@ -function run() { - try { - try { - foo(); - } catch (e) { - Sentry.captureException(e); - throw e; // intentionally re-throw - } - } catch (e) { - // simulate window.onerror without generating a Script error - window.onerror('error', 'file.js', 1, 1, e); - } -} - -run(); - -Sentry.captureException(new Error('error 2')); diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts index 9c6209d6ea61..d81e954c55ec 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts @@ -2,14 +2,27 @@ import { expect } from '@playwright/test'; import type { Event } from '@sentry/types'; import { sentryTest } from '../../../../../utils/fixtures'; -import { getMultipleSentryEnvelopeRequests } from '../../../../../utils/helpers'; +import { getMultipleSentryEnvelopeRequests, runScriptInSandbox } from '../../../../../utils/helpers'; sentryTest( 'should NOT catch an exception already caught [but rethrown] via Sentry.captureException', async ({ getLocalTestPath, page }) => { const url = await getLocalTestPath({ testDir: __dirname }); - const events = await getMultipleSentryEnvelopeRequests(page, 2, { url }); + await page.goto(url); + + runScriptInSandbox(page, { + content: ` + try { + foo(); + } catch (e) { + Sentry.captureException(e); + throw e; + } + `, + }); + + const events = await getMultipleSentryEnvelopeRequests(page, 1); expect(events[0].exception?.values).toHaveLength(1); expect(events[0].exception?.values?.[0]).toMatchObject({ @@ -24,19 +37,5 @@ sentryTest( frames: expect.any(Array), }, }); - - // This is not a refernece error, but another generic error - expect(events[1].exception?.values).toHaveLength(1); - expect(events[1].exception?.values?.[0]).toMatchObject({ - type: 'Error', - value: 'error 2', - mechanism: { - type: 'generic', - handled: true, - }, - stacktrace: { - frames: expect.any(Array), - }, - }); }, ); diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/subject.js b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/subject.js deleted file mode 100644 index ece68fe4890e..000000000000 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/subject.js +++ /dev/null @@ -1,10 +0,0 @@ -function run() { - try { - eval('foo{};'); - } catch (e) { - // simulate window.onerror without generating a Script error - window.onerror('error', 'file.js', 1, 1, e); - } -} - -run(); diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/test.ts index 4d55130e7190..d163e4071c8e 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/test.ts @@ -2,12 +2,20 @@ import { expect } from '@playwright/test'; import type { Event } from '@sentry/types'; import { sentryTest } from '../../../../../utils/fixtures'; -import { getFirstSentryEnvelopeRequest } from '../../../../../utils/helpers'; +import { getFirstSentryEnvelopeRequest, runScriptInSandbox } from '../../../../../utils/helpers'; sentryTest('should catch syntax errors', async ({ getLocalTestPath, page }) => { const url = await getLocalTestPath({ testDir: __dirname }); - const eventData = await getFirstSentryEnvelopeRequest(page, url); + await page.goto(url); + + runScriptInSandbox(page, { + content: ` + foo{}; // SyntaxError + `, + }); + + const eventData = await getFirstSentryEnvelopeRequest(page); expect(eventData.exception?.values).toHaveLength(1); expect(eventData.exception?.values?.[0]).toMatchObject({ diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/subject.js b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/subject.js deleted file mode 100644 index 03dd4333efc5..000000000000 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/subject.js +++ /dev/null @@ -1,10 +0,0 @@ -function run() { - try { - throw new Error('realError'); - } catch (e) { - // simulate window.onerror without generating a Script error - window.onerror('error', 'file.js', 1, 1, e); - } -} - -run(); diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/test.ts index d9b574fadfc5..2a1f9638b4a3 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/test.ts @@ -2,12 +2,20 @@ import { expect } from '@playwright/test'; import type { Event } from '@sentry/types'; import { sentryTest } from '../../../../../utils/fixtures'; -import { getFirstSentryEnvelopeRequest } from '../../../../../utils/helpers'; +import { getFirstSentryEnvelopeRequest, runScriptInSandbox } from '../../../../../utils/helpers'; sentryTest('should catch thrown errors', async ({ getLocalTestPath, page }) => { const url = await getLocalTestPath({ testDir: __dirname }); - const eventData = await getFirstSentryEnvelopeRequest(page, url); + await page.goto(url); + + runScriptInSandbox(page, { + content: ` + throw new Error('realError'); + `, + }); + + const eventData = await getFirstSentryEnvelopeRequest(page); expect(eventData.exception?.values).toHaveLength(1); expect(eventData.exception?.values?.[0]).toMatchObject({ diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-objects/subject.js b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-objects/subject.js deleted file mode 100644 index 1fb30ff3e221..000000000000 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-objects/subject.js +++ /dev/null @@ -1,10 +0,0 @@ -function run() { - try { - throw { error: 'stuff is broken', somekey: 'ok' }; - } catch (e) { - // simulate window.onerror without generating a Script error - window.onerror('error', 'file.js', 1, 1, e); - } -} - -run(); diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-objects/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-objects/test.ts index 326d7daa41fc..86d409f850fd 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-objects/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-objects/test.ts @@ -2,12 +2,22 @@ import { expect } from '@playwright/test'; import type { Event } from '@sentry/types'; import { sentryTest } from '../../../../../utils/fixtures'; -import { getFirstSentryEnvelopeRequest } from '../../../../../utils/helpers'; +import { getFirstSentryEnvelopeRequest, runScriptInSandbox } from '../../../../../utils/helpers'; sentryTest('should catch thrown objects', async ({ getLocalTestPath, page }) => { const url = await getLocalTestPath({ testDir: __dirname }); - const eventData = await getFirstSentryEnvelopeRequest(page, url); + await page.goto(url); + + runScriptInSandbox(page, { + content: ` + throw { + error: 'stuff is broken', + somekey: 'ok' + };`, + }); + + const eventData = await getFirstSentryEnvelopeRequest(page); expect(eventData.exception?.values).toHaveLength(1); expect(eventData.exception?.values?.[0]).toMatchObject({ diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/subject.js b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/subject.js deleted file mode 100644 index 9704f713714f..000000000000 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/subject.js +++ /dev/null @@ -1,10 +0,0 @@ -function run() { - try { - throw 'stringError'; - } catch (e) { - // simulate window.onerror without generating a Script error - window.onerror('error', 'file.js', 1, 1, e); - } -} - -run(); diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/test.ts index a52d70f30095..7e557e2046d0 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/test.ts @@ -2,12 +2,20 @@ import { expect } from '@playwright/test'; import type { Event } from '@sentry/types'; import { sentryTest } from '../../../../../utils/fixtures'; -import { getFirstSentryEnvelopeRequest } from '../../../../../utils/helpers'; +import { getFirstSentryEnvelopeRequest, runScriptInSandbox } from '../../../../../utils/helpers'; sentryTest('should catch thrown strings', async ({ getLocalTestPath, page }) => { const url = await getLocalTestPath({ testDir: __dirname }); - const eventData = await getFirstSentryEnvelopeRequest(page, url); + await page.goto(url); + + runScriptInSandbox(page, { + content: ` + throw 'stringError'; + `, + }); + + const eventData = await getFirstSentryEnvelopeRequest(page); expect(eventData.exception?.values).toHaveLength(1); expect(eventData.exception?.values?.[0]).toMatchObject({ diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/subject.js b/dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/subject.js deleted file mode 100644 index 55d9bf76d224..000000000000 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/subject.js +++ /dev/null @@ -1,9 +0,0 @@ -function run() { - Sentry.startSpan({ name: 'parent_span' }, () => { - throw new Error('Sync Error'); - }); -} - -// using `setTimeout` here because otherwise the thrown error will be -// thrown as a generic "Script Error." instead of the actual error". -setTimeout(run); diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/test.ts index 2c1d92ebbe00..5bf693f9d543 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/test.ts @@ -2,7 +2,11 @@ import { expect } from '@playwright/test'; import type { Event } from '@sentry/types'; import { sentryTest } from '../../../../utils/fixtures'; -import { getMultipleSentryEnvelopeRequests, shouldSkipTracingTest } from '../../../../utils/helpers'; +import { + getMultipleSentryEnvelopeRequests, + runScriptInSandbox, + shouldSkipTracingTest, +} from '../../../../utils/helpers'; sentryTest('should capture an error within a sync startSpan callback', async ({ getLocalTestPath, page }) => { if (shouldSkipTracingTest()) { @@ -10,10 +14,22 @@ sentryTest('should capture an error within a sync startSpan callback', async ({ } const url = await getLocalTestPath({ testDir: __dirname }); - const gotoPromise = page.goto(url); - const envelopePromise = getMultipleSentryEnvelopeRequests(page, 2); + await page.goto(url); + + runScriptInSandbox(page, { + content: ` + function run() { + Sentry.startSpan({ name: 'parent_span' }, () => { + throw new Error('Sync Error'); + }); + } + + setTimeout(run); + `, + }); + + const events = await getMultipleSentryEnvelopeRequests(page, 2); - const [, events] = await Promise.all([gotoPromise, envelopePromise]); const txn = events.find(event => event.type === 'transaction'); const err = events.find(event => !event.type); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/subject.js b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/subject.js deleted file mode 100644 index 2bb2bba64d9e..000000000000 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/subject.js +++ /dev/null @@ -1,3 +0,0 @@ -setTimeout(() => { - throw new Error('Error during pageload'); -}, 100); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/test.ts index df684872e7c0..22031cf7976f 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/test.ts @@ -1,7 +1,11 @@ import { expect } from '@playwright/test'; import type { Event } from '@sentry/types'; import { sentryTest } from '../../../../utils/fixtures'; -import { getMultipleSentryEnvelopeRequests, shouldSkipTracingTest } from '../../../../utils/helpers'; +import { + getMultipleSentryEnvelopeRequests, + runScriptInSandbox, + shouldSkipTracingTest, +} from '../../../../utils/helpers'; sentryTest( 'should put the pageload transaction name onto an error event caught during pageload', @@ -14,6 +18,12 @@ sentryTest( await page.goto(url); + runScriptInSandbox(page, { + content: ` + throw new Error('Error during pageload'); + `, + }); + const [e1, e2] = await getMultipleSentryEnvelopeRequests(page, 2); const pageloadTxnEvent = e1.type === 'transaction' ? e1 : e2; diff --git a/dev-packages/browser-integration-tests/utils/helpers.ts b/dev-packages/browser-integration-tests/utils/helpers.ts index b4fe7d2607ff..501af83dab3e 100644 --- a/dev-packages/browser-integration-tests/utils/helpers.ts +++ b/dev-packages/browser-integration-tests/utils/helpers.ts @@ -148,11 +148,17 @@ export const countEnvelopes = async ( * Run script at the given path inside the test environment. * * @param {Page} page - * @param {string} path + * @param {{ path?: string; content?: string }} impl * @return {*} {Promise} */ -async function runScriptInSandbox(page: Page, path: string): Promise { - await page.addScriptTag({ path }); +async function runScriptInSandbox( + page: Page, + impl: { + path?: string; + content?: string; + }, +): Promise { + await page.addScriptTag({ path: impl.path, content: impl.content }); } /** @@ -340,26 +346,11 @@ async function getFirstSentryEnvelopeRequest( } /** - * Manually inject a script into the page of given URL. - * This function is useful to create more complex test subjects that can't be achieved by pre-built pages. - * The given script should be vanilla browser JavaScript + * Trigger an error in the page context. + * This function is useful to test error handling in the page. * * @param {Page} page - * @param {string} url - * @param {string} scriptPath - * @return {*} {Promise>} + * @param {unknown} error */ -async function injectScriptAndGetEvents(page: Page, url: string, scriptPath: string): Promise> { - await page.goto(url); - await runScriptInSandbox(page, scriptPath); - return getSentryEvents(page); -} - -export { - runScriptInSandbox, - getMultipleSentryEnvelopeRequests, - getFirstSentryEnvelopeRequest, - getSentryEvents, - injectScriptAndGetEvents, -}; +export { runScriptInSandbox, getMultipleSentryEnvelopeRequests, getFirstSentryEnvelopeRequest, getSentryEvents }; From 6b7e9189436d2857273fbb0b24d7acdf6220af38 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Mon, 22 Apr 2024 10:55:24 +0100 Subject: [PATCH 2/8] Update playwright to latest. --- .../browser-integration-tests/package.json | 2 +- yarn.lock | 26 ++++++++++++++----- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/dev-packages/browser-integration-tests/package.json b/dev-packages/browser-integration-tests/package.json index 82893df63610..dd383ed7f5d7 100644 --- a/dev-packages/browser-integration-tests/package.json +++ b/dev-packages/browser-integration-tests/package.json @@ -40,7 +40,7 @@ }, "dependencies": { "@babel/preset-typescript": "^7.16.7", - "@playwright/test": "^1.40.1", + "@playwright/test": "^1.43.1", "@sentry-internal/rrweb": "2.11.0", "@sentry/browser": "8.0.0-beta.4", "axios": "1.6.7", diff --git a/yarn.lock b/yarn.lock index 50ffbc6d803c..236f8b18b110 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5803,12 +5803,12 @@ resolved "https://registry.yarnpkg.com/@pkgjs/parseargs/-/parseargs-0.11.0.tgz#a77ea742fab25775145434eb1d2328cf5013ac33" integrity sha512-+1VkjdD0QBLPodGrJUeqarH8VAIvQODIbwh9XpP5Syisf7YoQgsJKPNFoqqLQlu+VQ/tVSshMR6loPMn8U+dPg== -"@playwright/test@^1.40.1": - version "1.40.1" - resolved "https://registry.yarnpkg.com/@playwright/test/-/test-1.40.1.tgz#9e66322d97b1d74b9f8718bacab15080f24cde65" - integrity sha512-EaaawMTOeEItCRvfmkI9v6rBkF1svM8wjl/YPRrg2N2Wmp+4qJYkWtJsbew1szfKKDm6fPLy4YAanBhIlf9dWw== +"@playwright/test@^1.43.1": + version "1.43.1" + resolved "https://registry.yarnpkg.com/@playwright/test/-/test-1.43.1.tgz#16728a59eb8ce0f60472f98d8886d6cab0fa3e42" + integrity sha512-HgtQzFgNEEo4TE22K/X7sYTYNqEMMTZmFS8kTq6m8hXj+m1D8TgwgIbumHddJa9h4yl4GkKb8/bgAl2+g7eDgA== dependencies: - playwright "1.40.1" + playwright "1.43.1" "@polka/url@^1.0.0-next.20": version "1.0.0-next.21" @@ -23830,7 +23830,21 @@ playwright-core@1.40.1, playwright-core@^1.29.1: resolved "https://registry.yarnpkg.com/playwright-core/-/playwright-core-1.40.1.tgz#442d15e86866a87d90d07af528e0afabe4c75c05" integrity sha512-+hkOycxPiV534c4HhpfX6yrlawqVUzITRKwHAmYfmsVreltEl6fAZJ3DPfLMOODw0H3s1Itd6MDCWmP1fl/QvQ== -playwright@1.40.1, playwright@^1.31.1: +playwright-core@1.43.1: + version "1.43.1" + resolved "https://registry.yarnpkg.com/playwright-core/-/playwright-core-1.43.1.tgz#0eafef9994c69c02a1a3825a4343e56c99c03b02" + integrity sha512-EI36Mto2Vrx6VF7rm708qSnesVQKbxEWvPrfA1IPY6HgczBplDx7ENtx+K2n4kJ41sLLkuGfmb0ZLSSXlDhqPg== + +playwright@1.43.1: + version "1.43.1" + resolved "https://registry.yarnpkg.com/playwright/-/playwright-1.43.1.tgz#8ad08984ac66c9ef3d0db035be54dd7ec9f1c7d9" + integrity sha512-V7SoH0ai2kNt1Md9E3Gwas5B9m8KR2GVvwZnAI6Pg0m3sh7UvgiYhRrhsziCmqMJNouPckiOhk8T+9bSAK0VIA== + dependencies: + playwright-core "1.43.1" + optionalDependencies: + fsevents "2.3.2" + +playwright@^1.31.1: version "1.40.1" resolved "https://registry.yarnpkg.com/playwright/-/playwright-1.40.1.tgz#a11bf8dca15be5a194851dbbf3df235b9f53d7ae" integrity sha512-2eHI7IioIpQ0bS1Ovg/HszsN/XKNwEG1kbzSDDmADpclKc7CyqkHw7Mg2JCz/bbCxg25QUPcjksoMW7JcIFQmw== From 8a397c1627bb465c8672760521d8fef55eb24d95 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Mon, 22 Apr 2024 12:08:27 +0100 Subject: [PATCH 3/8] Switch to Promise.all --- .../onError/non-string-arg/test.ts | 11 ++++---- .../instrumentation/onError/rethrown/test.ts | 25 ++++++++++--------- .../onError/syntax-errors/test.ts | 11 ++++---- .../onError/thrown-errors/test.ts | 13 +++++----- .../onError/thrown-objects/test.ts | 19 +++++++------- .../onError/thrown-strings/test.ts | 11 ++++---- .../public-api/startSpan/error-sync/test.ts | 11 ++++---- .../browserTracingIntegration/error/test.ts | 15 +++++------ .../utils/helpers.ts | 12 +++++++-- 9 files changed, 72 insertions(+), 56 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/test.ts index 9013c744333d..74c6a0419aee 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/test.ts @@ -11,16 +11,17 @@ sentryTest( await page.goto(url); - runScriptInSandbox(page, { - content: ` + const [, eventData] = await Promise.all([ + runScriptInSandbox(page, { + content: ` throw { type: 'Error', otherKey: 'otherValue', }; `, - }); - - const eventData = await getFirstSentryEnvelopeRequest(page); + }), + getFirstSentryEnvelopeRequest(page), + ]); expect(eventData.exception?.values).toHaveLength(1); expect(eventData.exception?.values?.[0]).toMatchObject({ diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts index d81e954c55ec..95c19190e87a 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts @@ -11,18 +11,19 @@ sentryTest( await page.goto(url); - runScriptInSandbox(page, { - content: ` - try { - foo(); - } catch (e) { - Sentry.captureException(e); - throw e; - } - `, - }); - - const events = await getMultipleSentryEnvelopeRequests(page, 1); + const [, events] = await Promise.all([ + runScriptInSandbox(page, { + content: ` + try { + foo(); + } catch (e) { + Sentry.captureException(e); + throw e; + } + `, + }), + getMultipleSentryEnvelopeRequests(page, 1), + ]); expect(events[0].exception?.values).toHaveLength(1); expect(events[0].exception?.values?.[0]).toMatchObject({ diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/test.ts index d163e4071c8e..fc9eb8971aaa 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/test.ts @@ -9,13 +9,14 @@ sentryTest('should catch syntax errors', async ({ getLocalTestPath, page }) => { await page.goto(url); - runScriptInSandbox(page, { - content: ` + const [, eventData] = await Promise.all([ + runScriptInSandbox(page, { + content: ` foo{}; // SyntaxError `, - }); - - const eventData = await getFirstSentryEnvelopeRequest(page); + }), + getFirstSentryEnvelopeRequest(page), + ]); expect(eventData.exception?.values).toHaveLength(1); expect(eventData.exception?.values?.[0]).toMatchObject({ diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/test.ts index 2a1f9638b4a3..5f7c70cec2b3 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/test.ts @@ -9,13 +9,14 @@ sentryTest('should catch thrown errors', async ({ getLocalTestPath, page }) => { await page.goto(url); - runScriptInSandbox(page, { - content: ` + const [, eventData] = await Promise.all([ + runScriptInSandbox(page, { + content: ` throw new Error('realError'); - `, - }); - - const eventData = await getFirstSentryEnvelopeRequest(page); + `, + }), + getFirstSentryEnvelopeRequest(page), + ]); expect(eventData.exception?.values).toHaveLength(1); expect(eventData.exception?.values?.[0]).toMatchObject({ diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-objects/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-objects/test.ts index 86d409f850fd..01a4585a9915 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-objects/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-objects/test.ts @@ -9,15 +9,16 @@ sentryTest('should catch thrown objects', async ({ getLocalTestPath, page }) => await page.goto(url); - runScriptInSandbox(page, { - content: ` - throw { - error: 'stuff is broken', - somekey: 'ok' - };`, - }); - - const eventData = await getFirstSentryEnvelopeRequest(page); + const [, eventData] = await Promise.all([ + runScriptInSandbox(page, { + content: ` + throw { + error: 'stuff is broken', + somekey: 'ok' + };`, + }), + getFirstSentryEnvelopeRequest(page), + ]); expect(eventData.exception?.values).toHaveLength(1); expect(eventData.exception?.values?.[0]).toMatchObject({ diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/test.ts index 7e557e2046d0..eb57a09c0faf 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/test.ts @@ -9,13 +9,14 @@ sentryTest('should catch thrown strings', async ({ getLocalTestPath, page }) => await page.goto(url); - runScriptInSandbox(page, { - content: ` + const [, eventData] = await Promise.all([ + runScriptInSandbox(page, { + content: ` throw 'stringError'; `, - }); - - const eventData = await getFirstSentryEnvelopeRequest(page); + }), + getFirstSentryEnvelopeRequest(page), + ]); expect(eventData.exception?.values).toHaveLength(1); expect(eventData.exception?.values?.[0]).toMatchObject({ diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/test.ts index 5bf693f9d543..dfac45b95610 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/test.ts @@ -16,8 +16,9 @@ sentryTest('should capture an error within a sync startSpan callback', async ({ const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); - runScriptInSandbox(page, { - content: ` + const [, events] = await Promise.all([ + runScriptInSandbox(page, { + content: ` function run() { Sentry.startSpan({ name: 'parent_span' }, () => { throw new Error('Sync Error'); @@ -26,9 +27,9 @@ sentryTest('should capture an error within a sync startSpan callback', async ({ setTimeout(run); `, - }); - - const events = await getMultipleSentryEnvelopeRequests(page, 2); + }), + getMultipleSentryEnvelopeRequests(page, 2), + ]); const txn = events.find(event => event.type === 'transaction'); const err = events.find(event => !event.type); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/test.ts index 22031cf7976f..c8b75713f98f 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/test.ts @@ -18,13 +18,14 @@ sentryTest( await page.goto(url); - runScriptInSandbox(page, { - content: ` - throw new Error('Error during pageload'); - `, - }); - - const [e1, e2] = await getMultipleSentryEnvelopeRequests(page, 2); + const [, [e1, e2]] = await Promise.all([ + runScriptInSandbox(page, { + content: ` + throw new Error('Error during pageload'); + `, + }), + getMultipleSentryEnvelopeRequests(page, 2), + ]); const pageloadTxnEvent = e1.type === 'transaction' ? e1 : e2; const errorEvent = e1.type === 'transaction' ? e2 : e1; diff --git a/dev-packages/browser-integration-tests/utils/helpers.ts b/dev-packages/browser-integration-tests/utils/helpers.ts index 501af83dab3e..fc990eb817b3 100644 --- a/dev-packages/browser-integration-tests/utils/helpers.ts +++ b/dev-packages/browser-integration-tests/utils/helpers.ts @@ -145,7 +145,7 @@ export const countEnvelopes = async ( }; /** - * Run script at the given path inside the test environment. + * Run script inside the test environment. * * @param {Page} page * @param {{ path?: string; content?: string }} impl @@ -158,7 +158,15 @@ async function runScriptInSandbox( content?: string; }, ): Promise { - await page.addScriptTag({ path: impl.path, content: impl.content }); + try { + await page.addScriptTag({ path: impl.path, content: impl.content }); + } catch (e) { + // no-op + } +} + +/** + * } /** From 9f8c64d1fb158b2ce12209f405f8923f70798a73 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Tue, 23 Apr 2024 12:03:56 +0100 Subject: [PATCH 4/8] Change event waiting pattern. --- .../public-api/instrumentation/onError/init.js | 1 + .../instrumentation/onError/non-string-arg/test.ts | 13 +++++++------ .../instrumentation/onError/rethrown/test.ts | 13 +++++++------ .../instrumentation/onError/syntax-errors/test.ts | 13 +++++++------ .../instrumentation/onError/thrown-errors/test.ts | 13 +++++++------ .../instrumentation/onError/thrown-objects/test.ts | 13 +++++++------ .../instrumentation/onError/thrown-strings/test.ts | 13 +++++++------ .../suites/public-api/startSpan/error-sync/test.ts | 14 ++++++++------ .../browserTracingIntegration/error/test.ts | 13 +++++++------ .../browser-integration-tests/utils/helpers.ts | 12 ------------ 10 files changed, 58 insertions(+), 60 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/init.js b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/init.js index d8c94f36fdd0..573e4fdcb621 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/init.js +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/init.js @@ -4,4 +4,5 @@ window.Sentry = Sentry; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', + debug: true, }); diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/test.ts index 74c6a0419aee..92a18febd3ac 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/test.ts @@ -11,17 +11,18 @@ sentryTest( await page.goto(url); - const [, eventData] = await Promise.all([ - runScriptInSandbox(page, { - content: ` + const errorEventPromise = getFirstSentryEnvelopeRequest(page); + + runScriptInSandbox(page, { + content: ` throw { type: 'Error', otherKey: 'otherValue', }; `, - }), - getFirstSentryEnvelopeRequest(page), - ]); + }); + + const eventData = await errorEventPromise; expect(eventData.exception?.values).toHaveLength(1); expect(eventData.exception?.values?.[0]).toMatchObject({ diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts index 95c19190e87a..a796b9c0d208 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts @@ -11,9 +11,10 @@ sentryTest( await page.goto(url); - const [, events] = await Promise.all([ - runScriptInSandbox(page, { - content: ` + const errorEventsPromise = getMultipleSentryEnvelopeRequests(page, 1); + + runScriptInSandbox(page, { + content: ` try { foo(); } catch (e) { @@ -21,9 +22,9 @@ sentryTest( throw e; } `, - }), - getMultipleSentryEnvelopeRequests(page, 1), - ]); + }); + + const events = await errorEventsPromise; expect(events[0].exception?.values).toHaveLength(1); expect(events[0].exception?.values?.[0]).toMatchObject({ diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/test.ts index fc9eb8971aaa..313a9eeebfcb 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/test.ts @@ -9,14 +9,15 @@ sentryTest('should catch syntax errors', async ({ getLocalTestPath, page }) => { await page.goto(url); - const [, eventData] = await Promise.all([ - runScriptInSandbox(page, { - content: ` + const errorEventPromise = getFirstSentryEnvelopeRequest(page); + + runScriptInSandbox(page, { + content: ` foo{}; // SyntaxError `, - }), - getFirstSentryEnvelopeRequest(page), - ]); + }); + + const eventData = await errorEventPromise; expect(eventData.exception?.values).toHaveLength(1); expect(eventData.exception?.values?.[0]).toMatchObject({ diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/test.ts index 5f7c70cec2b3..3da379c9b6c0 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/test.ts @@ -9,14 +9,15 @@ sentryTest('should catch thrown errors', async ({ getLocalTestPath, page }) => { await page.goto(url); - const [, eventData] = await Promise.all([ - runScriptInSandbox(page, { - content: ` + const errorEventPromise = getFirstSentryEnvelopeRequest(page); + + runScriptInSandbox(page, { + content: ` throw new Error('realError'); `, - }), - getFirstSentryEnvelopeRequest(page), - ]); + }); + + const eventData = await errorEventPromise; expect(eventData.exception?.values).toHaveLength(1); expect(eventData.exception?.values?.[0]).toMatchObject({ diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-objects/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-objects/test.ts index 01a4585a9915..fdb52a6dac20 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-objects/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-objects/test.ts @@ -9,16 +9,17 @@ sentryTest('should catch thrown objects', async ({ getLocalTestPath, page }) => await page.goto(url); - const [, eventData] = await Promise.all([ - runScriptInSandbox(page, { - content: ` + const errorEventPromise = getFirstSentryEnvelopeRequest(page); + + await runScriptInSandbox(page, { + content: ` throw { error: 'stuff is broken', somekey: 'ok' };`, - }), - getFirstSentryEnvelopeRequest(page), - ]); + }); + + const eventData = await errorEventPromise; expect(eventData.exception?.values).toHaveLength(1); expect(eventData.exception?.values?.[0]).toMatchObject({ diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/test.ts index eb57a09c0faf..7e27a25df23d 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/test.ts @@ -9,14 +9,15 @@ sentryTest('should catch thrown strings', async ({ getLocalTestPath, page }) => await page.goto(url); - const [, eventData] = await Promise.all([ - runScriptInSandbox(page, { - content: ` + const errorEventPromise = getFirstSentryEnvelopeRequest(page); + + runScriptInSandbox(page, { + content: ` throw 'stringError'; `, - }), - getFirstSentryEnvelopeRequest(page), - ]); + }); + + const eventData = await errorEventPromise; expect(eventData.exception?.values).toHaveLength(1); expect(eventData.exception?.values?.[0]).toMatchObject({ diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/test.ts index dfac45b95610..8518b2bb8df1 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/test.ts @@ -14,11 +14,13 @@ sentryTest('should capture an error within a sync startSpan callback', async ({ } const url = await getLocalTestPath({ testDir: __dirname }); + await page.goto(url); - const [, events] = await Promise.all([ - runScriptInSandbox(page, { - content: ` + const errorEventsPromise = getMultipleSentryEnvelopeRequests(page, 2); + + runScriptInSandbox(page, { + content: ` function run() { Sentry.startSpan({ name: 'parent_span' }, () => { throw new Error('Sync Error'); @@ -27,9 +29,9 @@ sentryTest('should capture an error within a sync startSpan callback', async ({ setTimeout(run); `, - }), - getMultipleSentryEnvelopeRequests(page, 2), - ]); + }); + + const events = await errorEventsPromise; const txn = events.find(event => event.type === 'transaction'); const err = events.find(event => !event.type); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/test.ts index c8b75713f98f..7e6eabd920a9 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/test.ts @@ -18,14 +18,15 @@ sentryTest( await page.goto(url); - const [, [e1, e2]] = await Promise.all([ - runScriptInSandbox(page, { - content: ` + const errorEventsPromise = getMultipleSentryEnvelopeRequests(page, 2); + + runScriptInSandbox(page, { + content: ` throw new Error('Error during pageload'); `, - }), - getMultipleSentryEnvelopeRequests(page, 2), - ]); + }); + + const [e1, e2] = await errorEventsPromise; const pageloadTxnEvent = e1.type === 'transaction' ? e1 : e2; const errorEvent = e1.type === 'transaction' ? e2 : e1; diff --git a/dev-packages/browser-integration-tests/utils/helpers.ts b/dev-packages/browser-integration-tests/utils/helpers.ts index fc990eb817b3..678f441c5eb9 100644 --- a/dev-packages/browser-integration-tests/utils/helpers.ts +++ b/dev-packages/browser-integration-tests/utils/helpers.ts @@ -165,10 +165,6 @@ async function runScriptInSandbox( } } -/** - * -} - /** * Get Sentry events at the given URL, or the current page. * @@ -353,12 +349,4 @@ async function getFirstSentryEnvelopeRequest( return (await getMultipleSentryEnvelopeRequests(page, 1, { url }, requestParser))[0]; } -/** - * Trigger an error in the page context. - * This function is useful to test error handling in the page. - * - * @param {Page} page - * @param {unknown} error - */ - export { runScriptInSandbox, getMultipleSentryEnvelopeRequests, getFirstSentryEnvelopeRequest, getSentryEvents }; From c8a819f40abb6d26bb63fd9fc7baa00b65f517eb Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Tue, 23 Apr 2024 12:34:18 +0100 Subject: [PATCH 5/8] Simulate `window.onerror` in rethrow tests. --- .../instrumentation/onError/rethrown/test.ts | 37 +++++++++++++++---- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts index a796b9c0d208..a6d96dbc935f 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts @@ -11,17 +11,24 @@ sentryTest( await page.goto(url); - const errorEventsPromise = getMultipleSentryEnvelopeRequests(page, 1); + const errorEventsPromise = getMultipleSentryEnvelopeRequests(page, 2); runScriptInSandbox(page, { content: ` - try { - foo(); - } catch (e) { - Sentry.captureException(e); - throw e; - } - `, + try { + try { + foo(); + } catch (e) { + Sentry.captureException(e); + throw e; // intentionally re-throw + } + } catch (e) { + // simulate window.onerror without generating a Script error + window.onerror('error', 'file.js', 1, 1, e); + } + + Sentry.captureException(new Error('error 2')); + `, }); const events = await errorEventsPromise; @@ -39,5 +46,19 @@ sentryTest( frames: expect.any(Array), }, }); + + // This is not a refernece error, but another generic error + expect(events[1].exception?.values).toHaveLength(1); + expect(events[1].exception?.values?.[0]).toMatchObject({ + type: 'Error', + value: 'error 2', + mechanism: { + type: 'generic', + handled: true, + }, + stacktrace: { + frames: expect.any(Array), + }, + }); }, ); From 9144ea48d1b3f5bd586dfddfd6b0aefb9c2de8fd Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Tue, 23 Apr 2024 15:46:43 +0100 Subject: [PATCH 6/8] Skip `runScriptInSandbox` tests on webkit. --- .../instrumentation/onError/init.js | 1 - .../onError/non-string-arg/test.ts | 7 +++- .../instrumentation/onError/rethrown/test.ts | 7 +++- .../onError/syntax-errors/test.ts | 7 +++- .../onError/thrown-errors/test.ts | 7 +++- .../onError/thrown-objects/test.ts | 7 +++- .../onError/thrown-strings/test.ts | 7 +++- .../public-api/startSpan/error-sync/test.ts | 40 +++++++++++-------- .../browserTracingIntegration/error/test.ts | 7 +++- .../utils/helpers.ts | 3 ++ 10 files changed, 69 insertions(+), 24 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/init.js b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/init.js index 573e4fdcb621..d8c94f36fdd0 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/init.js +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/init.js @@ -4,5 +4,4 @@ window.Sentry = Sentry; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', - debug: true, }); diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/test.ts index 92a18febd3ac..ee7e5193d477 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/test.ts @@ -6,7 +6,12 @@ import { getFirstSentryEnvelopeRequest, runScriptInSandbox } from '../../../../. sentryTest( 'should catch onerror calls with non-string first argument gracefully', - async ({ getLocalTestPath, page }) => { + async ({ getLocalTestPath, page, browserName }) => { + if (browserName === 'webkit') { + // This test fails on Webkit as erros thrown from `runScriptInSandbox` are Script Errors and skipped by Sentry + sentryTest.skip(); + } + const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts index a6d96dbc935f..8058aa77d6c9 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts @@ -6,7 +6,12 @@ import { getMultipleSentryEnvelopeRequests, runScriptInSandbox } from '../../../ sentryTest( 'should NOT catch an exception already caught [but rethrown] via Sentry.captureException', - async ({ getLocalTestPath, page }) => { + async ({ getLocalTestPath, page, browserName }) => { + if (browserName === 'webkit') { + // This test fails on Webkit as erros thrown from `runScriptInSandbox` are Script Errors and skipped by Sentry + sentryTest.skip(); + } + const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/test.ts index 313a9eeebfcb..d218deecc27f 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/test.ts @@ -4,7 +4,12 @@ import type { Event } from '@sentry/types'; import { sentryTest } from '../../../../../utils/fixtures'; import { getFirstSentryEnvelopeRequest, runScriptInSandbox } from '../../../../../utils/helpers'; -sentryTest('should catch syntax errors', async ({ getLocalTestPath, page }) => { +sentryTest('should catch syntax errors', async ({ getLocalTestPath, page, browserName }) => { + if (browserName === 'webkit') { + // This test fails on Webkit as erros thrown from `runScriptInSandbox` are Script Errors and skipped by Sentry + sentryTest.skip(); + } + const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/test.ts index 3da379c9b6c0..413de04d3587 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/test.ts @@ -4,7 +4,12 @@ import type { Event } from '@sentry/types'; import { sentryTest } from '../../../../../utils/fixtures'; import { getFirstSentryEnvelopeRequest, runScriptInSandbox } from '../../../../../utils/helpers'; -sentryTest('should catch thrown errors', async ({ getLocalTestPath, page }) => { +sentryTest('should catch thrown errors', async ({ getLocalTestPath, page, browserName }) => { + if (browserName === 'webkit') { + // This test fails on Webkit as erros thrown from `runScriptInSandbox` are Script Errors and skipped by Sentry + sentryTest.skip(); + } + const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-objects/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-objects/test.ts index fdb52a6dac20..4ed03991ff58 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-objects/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-objects/test.ts @@ -4,7 +4,12 @@ import type { Event } from '@sentry/types'; import { sentryTest } from '../../../../../utils/fixtures'; import { getFirstSentryEnvelopeRequest, runScriptInSandbox } from '../../../../../utils/helpers'; -sentryTest('should catch thrown objects', async ({ getLocalTestPath, page }) => { +sentryTest('should catch thrown objects', async ({ getLocalTestPath, page, browserName }) => { + if (browserName === 'webkit') { + // This test fails on Webkit as erros thrown from `runScriptInSandbox` are Script Errors and skipped by Sentry + sentryTest.skip(); + } + const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/test.ts index 7e27a25df23d..a391cfa02b05 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/test.ts @@ -4,7 +4,12 @@ import type { Event } from '@sentry/types'; import { sentryTest } from '../../../../../utils/fixtures'; import { getFirstSentryEnvelopeRequest, runScriptInSandbox } from '../../../../../utils/helpers'; -sentryTest('should catch thrown strings', async ({ getLocalTestPath, page }) => { +sentryTest('should catch thrown strings', async ({ getLocalTestPath, page, browserName }) => { + if (browserName === 'webkit') { + // This test fails on Webkit as erros thrown from `runScriptInSandbox` are Script Errors and skipped by Sentry + sentryTest.skip(); + } + const url = await getLocalTestPath({ testDir: __dirname }); await page.goto(url); diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/test.ts index 8518b2bb8df1..0d4a1576f12e 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/test.ts @@ -8,19 +8,26 @@ import { shouldSkipTracingTest, } from '../../../../utils/helpers'; -sentryTest('should capture an error within a sync startSpan callback', async ({ getLocalTestPath, page }) => { - if (shouldSkipTracingTest()) { - sentryTest.skip(); - } +sentryTest( + 'should capture an error within a sync startSpan callback', + async ({ getLocalTestPath, page, browserName }) => { + if (browserName === 'webkit') { + // This test fails on Webkit as erros thrown from `runScriptInSandbox` are Script Errors and skipped by Sentry + sentryTest.skip(); + } - const url = await getLocalTestPath({ testDir: __dirname }); + if (shouldSkipTracingTest()) { + sentryTest.skip(); + } - await page.goto(url); + const url = await getLocalTestPath({ testDir: __dirname }); - const errorEventsPromise = getMultipleSentryEnvelopeRequests(page, 2); + await page.goto(url); - runScriptInSandbox(page, { - content: ` + const errorEventsPromise = getMultipleSentryEnvelopeRequests(page, 2); + + runScriptInSandbox(page, { + content: ` function run() { Sentry.startSpan({ name: 'parent_span' }, () => { throw new Error('Sync Error'); @@ -29,13 +36,14 @@ sentryTest('should capture an error within a sync startSpan callback', async ({ setTimeout(run); `, - }); + }); - const events = await errorEventsPromise; + const events = await errorEventsPromise; - const txn = events.find(event => event.type === 'transaction'); - const err = events.find(event => !event.type); + const txn = events.find(event => event.type === 'transaction'); + const err = events.find(event => !event.type); - expect(txn).toMatchObject({ transaction: 'parent_span' }); - expect(err?.exception?.values?.[0]?.value).toBe('Sync Error'); -}); + expect(txn).toMatchObject({ transaction: 'parent_span' }); + expect(err?.exception?.values?.[0]?.value).toBe('Sync Error'); + }, +); diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/test.ts index 7e6eabd920a9..3f44cc8f069c 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/test.ts @@ -9,7 +9,12 @@ import { sentryTest( 'should put the pageload transaction name onto an error event caught during pageload', - async ({ getLocalTestPath, page }) => { + async ({ getLocalTestPath, page, browserName }) => { + if (browserName === 'webkit') { + // This test fails on Webkit as erros thrown from `runScriptInSandbox` are Script Errors and skipped by Sentry + sentryTest.skip(); + } + if (shouldSkipTracingTest()) { sentryTest.skip(); } diff --git a/dev-packages/browser-integration-tests/utils/helpers.ts b/dev-packages/browser-integration-tests/utils/helpers.ts index 678f441c5eb9..0e888a708f00 100644 --- a/dev-packages/browser-integration-tests/utils/helpers.ts +++ b/dev-packages/browser-integration-tests/utils/helpers.ts @@ -146,6 +146,9 @@ export const countEnvelopes = async ( /** * Run script inside the test environment. + * This is useful for throwing errors in the test environment. + * + * Errors thrown from this function are not guaranteed to be captured by Sentry, especially in Webkit. * * @param {Page} page * @param {{ path?: string; content?: string }} impl From 38f142105f4cf79e807dd0162340f989e1e847d2 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 24 Apr 2024 09:41:31 +0100 Subject: [PATCH 7/8] Await `runScriptInSandbox`. --- .../public-api/instrumentation/onError/non-string-arg/test.ts | 2 +- .../suites/public-api/instrumentation/onError/rethrown/test.ts | 2 +- .../public-api/instrumentation/onError/syntax-errors/test.ts | 2 +- .../public-api/instrumentation/onError/thrown-errors/test.ts | 2 +- .../public-api/instrumentation/onError/thrown-strings/test.ts | 2 +- .../suites/public-api/startSpan/error-sync/test.ts | 2 +- .../suites/tracing/browserTracingIntegration/error/test.ts | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/test.ts index ee7e5193d477..681d5db8bf02 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/test.ts @@ -18,7 +18,7 @@ sentryTest( const errorEventPromise = getFirstSentryEnvelopeRequest(page); - runScriptInSandbox(page, { + await runScriptInSandbox(page, { content: ` throw { type: 'Error', diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts index 8058aa77d6c9..01b319e759b2 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts @@ -18,7 +18,7 @@ sentryTest( const errorEventsPromise = getMultipleSentryEnvelopeRequests(page, 2); - runScriptInSandbox(page, { + await runScriptInSandbox(page, { content: ` try { try { diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/test.ts index d218deecc27f..2b6cc09be8a2 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/test.ts @@ -16,7 +16,7 @@ sentryTest('should catch syntax errors', async ({ getLocalTestPath, page, browse const errorEventPromise = getFirstSentryEnvelopeRequest(page); - runScriptInSandbox(page, { + await runScriptInSandbox(page, { content: ` foo{}; // SyntaxError `, diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/test.ts index 413de04d3587..17dd6c650b43 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/test.ts @@ -16,7 +16,7 @@ sentryTest('should catch thrown errors', async ({ getLocalTestPath, page, browse const errorEventPromise = getFirstSentryEnvelopeRequest(page); - runScriptInSandbox(page, { + await runScriptInSandbox(page, { content: ` throw new Error('realError'); `, diff --git a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/test.ts b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/test.ts index a391cfa02b05..326cf414f0f8 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/test.ts @@ -16,7 +16,7 @@ sentryTest('should catch thrown strings', async ({ getLocalTestPath, page, brows const errorEventPromise = getFirstSentryEnvelopeRequest(page); - runScriptInSandbox(page, { + await runScriptInSandbox(page, { content: ` throw 'stringError'; `, diff --git a/dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/test.ts b/dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/test.ts index 0d4a1576f12e..bb7b3b43c516 100644 --- a/dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/test.ts +++ b/dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/test.ts @@ -26,7 +26,7 @@ sentryTest( const errorEventsPromise = getMultipleSentryEnvelopeRequests(page, 2); - runScriptInSandbox(page, { + await runScriptInSandbox(page, { content: ` function run() { Sentry.startSpan({ name: 'parent_span' }, () => { diff --git a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/test.ts b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/test.ts index 3f44cc8f069c..581f0fd206dc 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/test.ts @@ -25,7 +25,7 @@ sentryTest( const errorEventsPromise = getMultipleSentryEnvelopeRequests(page, 2); - runScriptInSandbox(page, { + await runScriptInSandbox(page, { content: ` throw new Error('Error during pageload'); `, From 955e50a6775e7ed5ed809576d2f6acd69c49b621 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Fri, 26 Apr 2024 16:20:18 +0100 Subject: [PATCH 8/8] Update tests to match the new Firefox behaviour. --- .../suites/stacktraces/regular_fn_identifiers/test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dev-packages/browser-integration-tests/suites/stacktraces/regular_fn_identifiers/test.ts b/dev-packages/browser-integration-tests/suites/stacktraces/regular_fn_identifiers/test.ts index 66f8286a4b5e..6ba6e15e6bd2 100644 --- a/dev-packages/browser-integration-tests/suites/stacktraces/regular_fn_identifiers/test.ts +++ b/dev-packages/browser-integration-tests/suites/stacktraces/regular_fn_identifiers/test.ts @@ -31,7 +31,8 @@ sentryTest( { function: '?' }, { function: 'qux' }, { function: 'qux/<' }, - { function: 'qux/