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

+3-3
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

+1-1
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

-8
This file was deleted.

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

+21-3
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

-17
This file was deleted.

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

+30-3
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

-10
This file was deleted.

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

+18-3
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

-10
This file was deleted.

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

+18-3
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({

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

-10
This file was deleted.

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

+20-3
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,29 @@ 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 objects', 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 objects', 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 {
22+
error: 'stuff is broken',
23+
somekey: 'ok'
24+
};`,
25+
});
26+
27+
const eventData = await errorEventPromise;
1128

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

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

-10
This file was deleted.

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

+18-3
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 strings', 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 strings', 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 'stringError';
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/startSpan/error-sync/subject.js

-9
This file was deleted.

dev-packages/browser-integration-tests/suites/public-api/startSpan/error-sync/test.ts

+41-14
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,48 @@ import { expect } from '@playwright/test';
22
import type { Event } from '@sentry/types';
33

44
import { sentryTest } from '../../../../utils/fixtures';
5-
import { getMultipleSentryEnvelopeRequests, shouldSkipTracingTest } from '../../../../utils/helpers';
5+
import {
6+
getMultipleSentryEnvelopeRequests,
7+
runScriptInSandbox,
8+
shouldSkipTracingTest,
9+
} from '../../../../utils/helpers';
610

7-
sentryTest('should capture an error within a sync startSpan callback', async ({ getLocalTestPath, page }) => {
8-
if (shouldSkipTracingTest()) {
9-
sentryTest.skip();
10-
}
11+
sentryTest(
12+
'should capture an error within a sync startSpan callback',
13+
async ({ getLocalTestPath, page, browserName }) => {
14+
if (browserName === 'webkit') {
15+
// This test fails on Webkit as erros thrown from `runScriptInSandbox` are Script Errors and skipped by Sentry
16+
sentryTest.skip();
17+
}
1118

12-
const url = await getLocalTestPath({ testDir: __dirname });
13-
const gotoPromise = page.goto(url);
14-
const envelopePromise = getMultipleSentryEnvelopeRequests<Event>(page, 2);
19+
if (shouldSkipTracingTest()) {
20+
sentryTest.skip();
21+
}
1522

16-
const [, events] = await Promise.all([gotoPromise, envelopePromise]);
17-
const txn = events.find(event => event.type === 'transaction');
18-
const err = events.find(event => !event.type);
23+
const url = await getLocalTestPath({ testDir: __dirname });
1924

20-
expect(txn).toMatchObject({ transaction: 'parent_span' });
21-
expect(err?.exception?.values?.[0]?.value).toBe('Sync Error');
22-
});
25+
await page.goto(url);
26+
27+
const errorEventsPromise = getMultipleSentryEnvelopeRequests<Event>(page, 2);
28+
29+
await runScriptInSandbox(page, {
30+
content: `
31+
function run() {
32+
Sentry.startSpan({ name: 'parent_span' }, () => {
33+
throw new Error('Sync Error');
34+
});
35+
}
36+
37+
setTimeout(run);
38+
`,
39+
});
40+
41+
const events = await errorEventsPromise;
42+
43+
const txn = events.find(event => event.type === 'transaction');
44+
const err = events.find(event => !event.type);
45+
46+
expect(txn).toMatchObject({ transaction: 'parent_span' });
47+
expect(err?.exception?.values?.[0]?.value).toBe('Sync Error');
48+
},
49+
);

dev-packages/browser-integration-tests/suites/stacktraces/regular_fn_identifiers/test.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ sentryTest(
3131
{ function: '?' },
3232
{ function: 'qux' },
3333
{ function: 'qux/<' },
34-
{ function: 'qux/</<' },
34+
// The function name below was 'qux/</<' on the Firefox versions < 124
35+
{ function: 'qux/<' },
3536
{ function: 'foo' },
3637
{ function: 'bar' },
3738
{ function: 'baz' },

dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/error/subject.js

-3
This file was deleted.

0 commit comments

Comments
 (0)