From b64b2aeb0d5adbafa9d3ba6eb0619814bf7e547a Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 18 Apr 2024 13:16:14 -0400 Subject: [PATCH] test: Port onerror tests to playwright (#11666) This ports `packages/browser/test/integration/suites/onerror.js` to playwright. Because I couldn't throw top level errors without generating script errors, I elected to simulate `window.onerror` being called. Co-authored-by: Lukas Stracke --- .../instrumentation/onError/init.js | 7 + .../onError/non-string-arg/subject.js | 8 + .../onError/non-string-arg/test.ts | 27 ++++ .../onError/rethrown/subject.js | 17 ++ .../instrumentation/onError/rethrown/test.ts | 42 +++++ .../onError/syntax-errors/subject.js | 10 ++ .../onError/syntax-errors/test.ts | 24 +++ .../onError/thrown-errors/subject.js | 10 ++ .../onError/thrown-errors/test.ts | 24 +++ .../onError/thrown-objects/subject.js | 10 ++ .../onError/thrown-objects/test.ts | 24 +++ .../onError/thrown-strings/subject.js | 10 ++ .../onError/thrown-strings/test.ts | 26 ++++ .../test/integration/subjects/console-logs.js | 13 -- .../test/integration/subjects/throw-error.js | 4 - .../test/integration/subjects/throw-object.js | 7 - .../test/integration/subjects/throw-string.js | 5 - .../test/integration/suites/onerror.js | 146 ------------------ .../browser/test/integration/suites/shell.js | 1 - 19 files changed, 239 insertions(+), 176 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/init.js create mode 100644 dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-objects/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-objects/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/test.ts delete mode 100644 packages/browser/test/integration/subjects/console-logs.js delete mode 100644 packages/browser/test/integration/subjects/throw-error.js delete mode 100644 packages/browser/test/integration/subjects/throw-object.js delete mode 100644 packages/browser/test/integration/subjects/throw-string.js delete mode 100644 packages/browser/test/integration/suites/onerror.js 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 new file mode 100644 index 000000000000..d8c94f36fdd0 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/init.js @@ -0,0 +1,7 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', +}); 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 new file mode 100644 index 000000000000..ad794db023df --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/subject.js @@ -0,0 +1,8 @@ +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 new file mode 100644 index 000000000000..ed2399a43790 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/test.ts @@ -0,0 +1,27 @@ +import { expect } from '@playwright/test'; +import type { Event } from '@sentry/types'; + +import { sentryTest } from '../../../../../utils/fixtures'; +import { getFirstSentryEnvelopeRequest } 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); + + expect(eventData.exception?.values).toHaveLength(1); + expect(eventData.exception?.values?.[0]).toMatchObject({ + type: 'Error', + value: 'Object captured as exception with keys: otherKey, type', + mechanism: { + type: 'onerror', + handled: false, + }, + stacktrace: { + frames: expect.any(Array), + }, + }); + }, +); 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 new file mode 100644 index 000000000000..44a6e2d739c5 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/subject.js @@ -0,0 +1,17 @@ +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 new file mode 100644 index 000000000000..9c6209d6ea61 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts @@ -0,0 +1,42 @@ +import { expect } from '@playwright/test'; +import type { Event } from '@sentry/types'; + +import { sentryTest } from '../../../../../utils/fixtures'; +import { getMultipleSentryEnvelopeRequests } 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 }); + + expect(events[0].exception?.values).toHaveLength(1); + expect(events[0].exception?.values?.[0]).toMatchObject({ + type: 'ReferenceError', + // this exact error message varies between browsers, but they should all reference 'foo' + value: expect.stringContaining('foo'), + mechanism: { + type: 'generic', + handled: true, + }, + stacktrace: { + 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 new file mode 100644 index 000000000000..ece68fe4890e --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/subject.js @@ -0,0 +1,10 @@ +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 new file mode 100644 index 000000000000..4d55130e7190 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/test.ts @@ -0,0 +1,24 @@ +import { expect } from '@playwright/test'; +import type { Event } from '@sentry/types'; + +import { sentryTest } from '../../../../../utils/fixtures'; +import { getFirstSentryEnvelopeRequest } from '../../../../../utils/helpers'; + +sentryTest('should catch syntax errors', async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const eventData = await getFirstSentryEnvelopeRequest(page, url); + + expect(eventData.exception?.values).toHaveLength(1); + expect(eventData.exception?.values?.[0]).toMatchObject({ + type: 'SyntaxError', + value: "Unexpected token '{'", + mechanism: { + type: 'onerror', + handled: false, + }, + stacktrace: { + frames: expect.any(Array), + }, + }); +}); 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 new file mode 100644 index 000000000000..03dd4333efc5 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/subject.js @@ -0,0 +1,10 @@ +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 new file mode 100644 index 000000000000..d9b574fadfc5 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/test.ts @@ -0,0 +1,24 @@ +import { expect } from '@playwright/test'; +import type { Event } from '@sentry/types'; + +import { sentryTest } from '../../../../../utils/fixtures'; +import { getFirstSentryEnvelopeRequest } from '../../../../../utils/helpers'; + +sentryTest('should catch thrown errors', async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const eventData = await getFirstSentryEnvelopeRequest(page, url); + + expect(eventData.exception?.values).toHaveLength(1); + expect(eventData.exception?.values?.[0]).toMatchObject({ + type: 'Error', + value: 'realError', + mechanism: { + type: 'onerror', + handled: false, + }, + stacktrace: { + frames: expect.any(Array), + }, + }); +}); 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 new file mode 100644 index 000000000000..1fb30ff3e221 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-objects/subject.js @@ -0,0 +1,10 @@ +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 new file mode 100644 index 000000000000..326d7daa41fc --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-objects/test.ts @@ -0,0 +1,24 @@ +import { expect } from '@playwright/test'; +import type { Event } from '@sentry/types'; + +import { sentryTest } from '../../../../../utils/fixtures'; +import { getFirstSentryEnvelopeRequest } from '../../../../../utils/helpers'; + +sentryTest('should catch thrown objects', async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const eventData = await getFirstSentryEnvelopeRequest(page, url); + + expect(eventData.exception?.values).toHaveLength(1); + expect(eventData.exception?.values?.[0]).toMatchObject({ + type: 'Error', + value: 'Object captured as exception with keys: error, somekey', + mechanism: { + type: 'onerror', + handled: false, + }, + stacktrace: { + frames: expect.any(Array), + }, + }); +}); 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 new file mode 100644 index 000000000000..9704f713714f --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/subject.js @@ -0,0 +1,10 @@ +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 new file mode 100644 index 000000000000..a52d70f30095 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-strings/test.ts @@ -0,0 +1,26 @@ +import { expect } from '@playwright/test'; +import type { Event } from '@sentry/types'; + +import { sentryTest } from '../../../../../utils/fixtures'; +import { getFirstSentryEnvelopeRequest } from '../../../../../utils/helpers'; + +sentryTest('should catch thrown strings', async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const eventData = await getFirstSentryEnvelopeRequest(page, url); + + expect(eventData.exception?.values).toHaveLength(1); + expect(eventData.exception?.values?.[0]).toMatchObject({ + type: 'Error', + value: 'stringError', + mechanism: { + type: 'onerror', + handled: false, + }, + stacktrace: { + frames: expect.any(Array), + }, + }); + + expect(eventData.exception?.values?.[0].stacktrace?.frames).toHaveLength(1); +}); diff --git a/packages/browser/test/integration/subjects/console-logs.js b/packages/browser/test/integration/subjects/console-logs.js deleted file mode 100644 index d32f1a5e1080..000000000000 --- a/packages/browser/test/integration/subjects/console-logs.js +++ /dev/null @@ -1,13 +0,0 @@ -console.log('One'); -console.warn('Two', { a: 1 }); -console.error('Error 2', { b: { c: [] } }); - -// Passed assertions _should not_ be captured -console.assert(1 + 1 === 2, 'math works'); -// Failed assertions _should_ be captured -console.assert(1 + 1 === 3, 'math broke'); - -function a() { - throw new Error('Error thrown 3'); -} -a(); diff --git a/packages/browser/test/integration/subjects/throw-error.js b/packages/browser/test/integration/subjects/throw-error.js deleted file mode 100644 index 4ba4f3415da1..000000000000 --- a/packages/browser/test/integration/subjects/throw-error.js +++ /dev/null @@ -1,4 +0,0 @@ -function throwRealError() { - throw new Error('realError'); -} -throwRealError(); diff --git a/packages/browser/test/integration/subjects/throw-object.js b/packages/browser/test/integration/subjects/throw-object.js deleted file mode 100644 index e69f1f37d5d6..000000000000 --- a/packages/browser/test/integration/subjects/throw-object.js +++ /dev/null @@ -1,7 +0,0 @@ -function throwObjectError() { - // never do this; just making sure Raven.js handles this case - // gracefully - throw { error: 'stuff is broken', somekey: 'ok' }; -} - -throwObjectError(); diff --git a/packages/browser/test/integration/subjects/throw-string.js b/packages/browser/test/integration/subjects/throw-string.js deleted file mode 100644 index 7429303cab1a..000000000000 --- a/packages/browser/test/integration/subjects/throw-string.js +++ /dev/null @@ -1,5 +0,0 @@ -function throwStringError() { - throw 'stringError'; -} - -throwStringError(); diff --git a/packages/browser/test/integration/suites/onerror.js b/packages/browser/test/integration/suites/onerror.js deleted file mode 100644 index 1352d51e0484..000000000000 --- a/packages/browser/test/integration/suites/onerror.js +++ /dev/null @@ -1,146 +0,0 @@ -describe('window.onerror', function () { - it('should catch syntax errors', function () { - return runInSandbox(sandbox, function () { - eval('foo{};'); - }).then(function (summary) { - // ¯\_(ツ)_/¯ - if (summary.window.isBelowIE11()) { - assert.equal(summary.events[0].exception.values[0].type, 'Error'); - } else { - assert.match(summary.events[0].exception.values[0].type, /SyntaxError/); - } - assert.equal(summary.events[0].exception.values[0].stacktrace.frames.length, 1); // just one frame - }); - }); - - it('should catch thrown strings', function () { - return runInSandbox(sandbox, { manual: true }, function () { - // intentionally loading this error via a script file to make - // sure it is 1) not caught by instrumentation 2) doesn't trigger - // "Script error" - var script = document.createElement('script'); - script.src = '/base/subjects/throw-string.js'; - script.onload = function () { - window.finalizeManualTest(); - }; - document.head.appendChild(script); - }).then(function (summary) { - assert.match(summary.events[0].exception.values[0].value, /stringError$/); - assert.equal(summary.events[0].exception.values[0].stacktrace.frames.length, 1); // always 1 because thrown strings can't provide > 1 frame - - // some browsers extract proper url, line, and column for thrown strings - // but not all - falls back to frame url - assert.match( - summary.events[0].exception.values[0].stacktrace.frames[0].filename, - /(\/subjects\/throw-string.js|\/base\/variants\/)/, - ); - assert.match( - summary.events[0].exception.values[0].stacktrace.frames[0]['function'], - /throwStringError|\?|global code/i, - ); - }); - }); - - it('should catch thrown objects', function () { - return runInSandbox(sandbox, { manual: true }, function () { - // intentionally loading this error via a script file to make - // sure it is 1) not caught by instrumentation 2) doesn't trigger - // "Script error" - var script = document.createElement('script'); - script.src = '/base/subjects/throw-object.js'; - script.onload = function () { - window.finalizeManualTest(); - }; - document.head.appendChild(script); - }).then(function (summary) { - assert.equal(summary.events[0].exception.values[0].type, 'Error'); - - // ¯\_(ツ)_/¯ - if (summary.window.isBelowIE11()) { - assert.equal(summary.events[0].exception.values[0].value, '[object Object]'); - } else { - assert.equal( - summary.events[0].exception.values[0].value, - 'Object captured as exception with keys: error, somekey', - ); - } - assert.equal(summary.events[0].exception.values[0].stacktrace.frames.length, 1); // always 1 because thrown objects can't provide > 1 frame - - // some browsers extract proper url, line, and column for thrown objects - // but not all - falls back to frame url - assert.match( - summary.events[0].exception.values[0].stacktrace.frames[0].filename, - /(\/subjects\/throw-object.js|\/base\/variants\/)/, - ); - assert.match( - summary.events[0].exception.values[0].stacktrace.frames[0]['function'], - /throwStringError|\?|global code/i, - ); - }); - }); - - it('should catch thrown errors', function () { - return runInSandbox(sandbox, { manual: true }, function () { - // intentionally loading this error via a script file to make - // sure it is 1) not caught by instrumentation 2) doesn't trigger - // "Script error" - var script = document.createElement('script'); - script.src = '/base/subjects/throw-error.js'; - script.onload = function () { - window.finalizeManualTest(); - }; - document.head.appendChild(script); - }).then(function (summary) { - // ¯\_(ツ)_/¯ - if (summary.window.isBelowIE11()) { - assert.equal(summary.events[0].exception.values[0].type, 'Error'); - } else { - assert.match(summary.events[0].exception.values[0].type, /^Error/); - } - assert.match(summary.events[0].exception.values[0].value, /realError$/); - // 1 or 2 depending on platform - assert.isAtLeast(summary.events[0].exception.values[0].stacktrace.frames.length, 1); - assert.isAtMost(summary.events[0].exception.values[0].stacktrace.frames.length, 2); - assert.match(summary.events[0].exception.values[0].stacktrace.frames[0].filename, /\/subjects\/throw-error\.js/); - assert.match( - summary.events[0].exception.values[0].stacktrace.frames[0]['function'], - /\?|global code|throwRealError/i, - ); - }); - }); - - it('should onerror calls with non-string first argument gracefully', function () { - return runInSandbox(sandbox, function () { - window.onerror({ - type: 'error', - otherKey: 'hi', - }); - }).then(function (summary) { - assert.equal(summary.events[0].exception.values[0].type, 'Error'); - assert.equal( - summary.events[0].exception.values[0].value, - 'Object captured as exception with keys: otherKey, type', - ); - assert.deepEqual(summary.events[0].extra.__serialized__, { - type: 'error', - otherKey: 'hi', - }); - }); - }); - - it('should NOT catch an exception already caught [but rethrown] via Sentry.captureException', function () { - return runInSandbox(sandbox, function () { - try { - foo(); - } catch (e) { - Sentry.captureException(e); - throw e; // intentionally re-throw - } - }).then(function (summary) { - // IE10 uses different type (Error instead of ReferenceError) for rethrown errors... - if (!summary.window.isBelowIE11()) { - assert.equal(summary.events.length, 1); - } - }); - }); -}); diff --git a/packages/browser/test/integration/suites/shell.js b/packages/browser/test/integration/suites/shell.js index 7df81ae2b53e..bec3ab7a8768 100644 --- a/packages/browser/test/integration/suites/shell.js +++ b/packages/browser/test/integration/suites/shell.js @@ -22,7 +22,6 @@ function runVariant(variant) { /** * The test runner will replace each of these placeholders with the contents of the corresponding file. */ - {{ suites/onerror.js }} // biome-ignore format: No trailing commas {{ suites/onunhandledrejection.js }} // biome-ignore format: No trailing commas {{ suites/builtins.js }} // biome-ignore format: No trailing commas {{ suites/loader.js }} // biome-ignore format: No trailing commas