Skip to content

Commit cda367c

Browse files
authored
ref(test): Trigger top-level errors inside sandbox. (#11712)
Resolves: #11678 Related: #11666 This PR updates `runScriptInSandbox` utility, which was implemented (probably for a similar reason) but not used. Ports relevant tests to this pattern.
1 parent 43e370f commit cda367c

File tree

21 files changed

+230
-151
lines changed

21 files changed

+230
-151
lines changed

dev-packages/browser-integration-tests/README.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,16 @@ or `init.js` is not defined in a case folder.
1414

1515
`subject.js` contains the logic that sets up the environment to be tested. It also can be defined locally and as a group
1616
fallback. Unlike `template.hbs` and `init.js`, it's not required to be defined for a group, as there may be cases that
17-
does not require a subject, instead the logic is injected using `injectScriptAndGetEvents` from `utils/helpers.ts`.
17+
does not require a subject.
1818

1919
`test.ts` is required for each test case, which contains the assertions (and if required the script injection logic).
2020
For every case, any set of `init.js`, `template.hbs` and `subject.js` can be defined locally, and each one of them will
2121
have precedence over the default definitions of the test group.
2222

2323
To test page multi-page navigations, you can specify additional `page-*.html` (e.g. `page-0.html`, `page-1.html`) files.
2424
These will also be compiled and initialized with the same `init.js` and `subject.js` files that are applied to
25-
`template.hbs/html`. Note: `page-*.html` file lookup **doesn not** fall back to the parent directories, meaning that
26-
page files have to be directly in the `test.ts` directory.
25+
`template.hbs/html`. Note: `page-*.html` file lookup **does not** fall back to the parent directories, meaning that page
26+
files have to be directly in the `test.ts` directory.
2727

2828
```
2929
suites/

dev-packages/browser-integration-tests/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
},
4141
"dependencies": {
4242
"@babel/preset-typescript": "^7.16.7",
43-
"@playwright/test": "^1.40.1",
43+
"@playwright/test": "^1.43.1",
4444
"@sentry-internal/rrweb": "2.11.0",
4545
"@sentry/browser": "8.0.0-beta.4",
4646
"axios": "1.6.7",

dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/subject.js

Lines changed: 0 additions & 8 deletions
This file was deleted.

dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/non-string-arg/test.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,32 @@ import { expect } from '@playwright/test';
22
import type { Event } from '@sentry/types';
33

44
import { sentryTest } from '../../../../../utils/fixtures';
5-
import { getFirstSentryEnvelopeRequest } from '../../../../../utils/helpers';
5+
import { getFirstSentryEnvelopeRequest, runScriptInSandbox } from '../../../../../utils/helpers';
66

77
sentryTest(
88
'should catch onerror calls with non-string first argument gracefully',
9-
async ({ getLocalTestPath, page }) => {
9+
async ({ getLocalTestPath, page, browserName }) => {
10+
if (browserName === 'webkit') {
11+
// This test fails on Webkit as erros thrown from `runScriptInSandbox` are Script Errors and skipped by Sentry
12+
sentryTest.skip();
13+
}
14+
1015
const url = await getLocalTestPath({ testDir: __dirname });
1116

12-
const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
17+
await page.goto(url);
18+
19+
const errorEventPromise = getFirstSentryEnvelopeRequest<Event>(page);
20+
21+
await runScriptInSandbox(page, {
22+
content: `
23+
throw {
24+
type: 'Error',
25+
otherKey: 'otherValue',
26+
};
27+
`,
28+
});
29+
30+
const eventData = await errorEventPromise;
1331

1432
expect(eventData.exception?.values).toHaveLength(1);
1533
expect(eventData.exception?.values?.[0]).toMatchObject({

dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/subject.js

Lines changed: 0 additions & 17 deletions
This file was deleted.

dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/rethrown/test.ts

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,41 @@ import { expect } from '@playwright/test';
22
import type { Event } from '@sentry/types';
33

44
import { sentryTest } from '../../../../../utils/fixtures';
5-
import { getMultipleSentryEnvelopeRequests } from '../../../../../utils/helpers';
5+
import { getMultipleSentryEnvelopeRequests, runScriptInSandbox } from '../../../../../utils/helpers';
66

77
sentryTest(
88
'should NOT catch an exception already caught [but rethrown] via Sentry.captureException',
9-
async ({ getLocalTestPath, page }) => {
9+
async ({ getLocalTestPath, page, browserName }) => {
10+
if (browserName === 'webkit') {
11+
// This test fails on Webkit as erros thrown from `runScriptInSandbox` are Script Errors and skipped by Sentry
12+
sentryTest.skip();
13+
}
14+
1015
const url = await getLocalTestPath({ testDir: __dirname });
1116

12-
const events = await getMultipleSentryEnvelopeRequests<Event>(page, 2, { url });
17+
await page.goto(url);
18+
19+
const errorEventsPromise = getMultipleSentryEnvelopeRequests<Event>(page, 2);
20+
21+
await runScriptInSandbox(page, {
22+
content: `
23+
try {
24+
try {
25+
foo();
26+
} catch (e) {
27+
Sentry.captureException(e);
28+
throw e; // intentionally re-throw
29+
}
30+
} catch (e) {
31+
// simulate window.onerror without generating a Script error
32+
window.onerror('error', 'file.js', 1, 1, e);
33+
}
34+
35+
Sentry.captureException(new Error('error 2'));
36+
`,
37+
});
38+
39+
const events = await errorEventsPromise;
1340

1441
expect(events[0].exception?.values).toHaveLength(1);
1542
expect(events[0].exception?.values?.[0]).toMatchObject({

dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/subject.js

Lines changed: 0 additions & 10 deletions
This file was deleted.

dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/syntax-errors/test.ts

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,27 @@ import { expect } from '@playwright/test';
22
import type { Event } from '@sentry/types';
33

44
import { sentryTest } from '../../../../../utils/fixtures';
5-
import { getFirstSentryEnvelopeRequest } from '../../../../../utils/helpers';
5+
import { getFirstSentryEnvelopeRequest, runScriptInSandbox } from '../../../../../utils/helpers';
6+
7+
sentryTest('should catch syntax errors', async ({ getLocalTestPath, page, browserName }) => {
8+
if (browserName === 'webkit') {
9+
// This test fails on Webkit as erros thrown from `runScriptInSandbox` are Script Errors and skipped by Sentry
10+
sentryTest.skip();
11+
}
612

7-
sentryTest('should catch syntax errors', async ({ getLocalTestPath, page }) => {
813
const url = await getLocalTestPath({ testDir: __dirname });
914

10-
const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
15+
await page.goto(url);
16+
17+
const errorEventPromise = getFirstSentryEnvelopeRequest<Event>(page);
18+
19+
await runScriptInSandbox(page, {
20+
content: `
21+
foo{}; // SyntaxError
22+
`,
23+
});
24+
25+
const eventData = await errorEventPromise;
1126

1227
expect(eventData.exception?.values).toHaveLength(1);
1328
expect(eventData.exception?.values?.[0]).toMatchObject({

dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/subject.js

Lines changed: 0 additions & 10 deletions
This file was deleted.

dev-packages/browser-integration-tests/suites/public-api/instrumentation/onError/thrown-errors/test.ts

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,27 @@ import { expect } from '@playwright/test';
22
import type { Event } from '@sentry/types';
33

44
import { sentryTest } from '../../../../../utils/fixtures';
5-
import { getFirstSentryEnvelopeRequest } from '../../../../../utils/helpers';
5+
import { getFirstSentryEnvelopeRequest, runScriptInSandbox } from '../../../../../utils/helpers';
6+
7+
sentryTest('should catch thrown errors', async ({ getLocalTestPath, page, browserName }) => {
8+
if (browserName === 'webkit') {
9+
// This test fails on Webkit as erros thrown from `runScriptInSandbox` are Script Errors and skipped by Sentry
10+
sentryTest.skip();
11+
}
612

7-
sentryTest('should catch thrown errors', async ({ getLocalTestPath, page }) => {
813
const url = await getLocalTestPath({ testDir: __dirname });
914

10-
const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
15+
await page.goto(url);
16+
17+
const errorEventPromise = getFirstSentryEnvelopeRequest<Event>(page);
18+
19+
await runScriptInSandbox(page, {
20+
content: `
21+
throw new Error('realError');
22+
`,
23+
});
24+
25+
const eventData = await errorEventPromise;
1126

1227
expect(eventData.exception?.values).toHaveLength(1);
1328
expect(eventData.exception?.values?.[0]).toMatchObject({

0 commit comments

Comments
 (0)