Skip to content

Commit de565e8

Browse files
Lms24Lms24
committed
fix guard check, add more tests
1 parent cbfaa35 commit de565e8

File tree

8 files changed

+115
-28
lines changed

8 files changed

+115
-28
lines changed

‎packages/browser-integration-tests/suites/integrations/ContextLines/template.html renamed to ‎packages/browser-integration-tests/suites/integrations/ContextLines/inline/template.html

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
</head>
66
<body>
77
<button id="inline-error-btn" onclick="throw new Error('Error with context lines')">Click me</button>
8-
<button id="script-error-btn">Click me too</button>
98
</body>
109
<footer>
1110
Some text...

‎packages/browser-integration-tests/suites/integrations/ContextLines/test.ts renamed to ‎packages/browser-integration-tests/suites/integrations/ContextLines/inline/test.ts

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { expect } from '@playwright/test';
22

3-
import { sentryTest } from '../../../utils/fixtures';
4-
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../utils/helpers';
3+
import { sentryTest } from '../../../../utils/fixtures';
4+
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';
55

66
sentryTest(
77
'should add source context lines around stack frames from errors in Html inline JS',
@@ -37,34 +37,13 @@ sentryTest(
3737
context_line:
3838
' <button id="inline-error-btn" onclick="throw new Error(\'Error with context lines\')">Click me</button>',
3939
post_context: [
40-
' <button id="script-error-btn">Click me too</button>',
4140
expect.stringContaining('subject.bundle.js'), // this line varies in the test based on tarball/cdn bundle (+variants)
4241
' <footer>',
42+
' Some text...',
4343
],
4444
},
4545
],
4646
},
4747
});
4848
},
4949
);
50-
51-
sentryTest('should not add source context lines to errors from script files', async ({ getLocalTestPath, page }) => {
52-
const url = await getLocalTestPath({ testDir: __dirname });
53-
54-
const eventReqPromise = waitForErrorRequestOnUrl(page, url);
55-
56-
const clickPromise = page.click('#script-error-btn');
57-
58-
const [req] = await Promise.all([eventReqPromise, clickPromise]);
59-
60-
const eventData = envelopeRequestParser(req);
61-
62-
const exception = eventData.exception?.values?.[0];
63-
const frames = exception?.stacktrace?.frames;
64-
expect(frames).toHaveLength(1);
65-
frames?.forEach(f => {
66-
expect(f).not.toHaveProperty('pre_context');
67-
expect(f).not.toHaveProperty('context_line');
68-
expect(f).not.toHaveProperty('post_context');
69-
});
70-
});

‎packages/browser-integration-tests/suites/integrations/ContextLines/subject.js renamed to ‎packages/browser-integration-tests/suites/integrations/ContextLines/noAddedLines/subject.js

File renamed without changes.

‎packages/browser-integration-tests/suites/integrations/ContextLines/noAddedLines/template.html

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8" />
5+
</head>
6+
<body>
7+
<button id="script-error-btn">Click me</button>
8+
</body>
9+
<footer>
10+
Some text...
11+
</foot>
12+
</html>

‎packages/browser-integration-tests/suites/integrations/ContextLines/noAddedLines/test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { expect } from '@playwright/test';
2+
3+
import { sentryTest } from '../../../../utils/fixtures';
4+
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';
5+
6+
sentryTest('should not add source context lines to errors from script files', async ({ getLocalTestPath, page }) => {
7+
const url = await getLocalTestPath({ testDir: __dirname });
8+
9+
const eventReqPromise = waitForErrorRequestOnUrl(page, url);
10+
11+
const clickPromise = page.click('#script-error-btn');
12+
13+
const [req] = await Promise.all([eventReqPromise, clickPromise]);
14+
15+
const eventData = envelopeRequestParser(req);
16+
17+
const exception = eventData.exception?.values?.[0];
18+
const frames = exception?.stacktrace?.frames;
19+
expect(frames).toHaveLength(1);
20+
frames?.forEach(f => {
21+
expect(f).not.toHaveProperty('pre_context');
22+
expect(f).not.toHaveProperty('context_line');
23+
expect(f).not.toHaveProperty('post_context');
24+
});
25+
});

‎packages/browser-integration-tests/suites/integrations/ContextLines/scriptTag/template.html

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<meta charset="utf-8" />
5+
<script>
6+
function throwTestError() {
7+
throw new Error('Error with context lines')
8+
}
9+
</script>
10+
</head>
11+
<body>
12+
<button id="inline-error-btn" onclick="throwTestError()">Click me</button>
13+
</body>
14+
<footer>
15+
Some text...
16+
</foot>
17+
</html>

‎packages/browser-integration-tests/suites/integrations/ContextLines/scriptTag/test.ts

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import { expect } from '@playwright/test';
2+
3+
import { sentryTest } from '../../../../utils/fixtures';
4+
import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../../utils/helpers';
5+
6+
sentryTest(
7+
'should add source context lines around stack frames from errors in Html script tags',
8+
async ({ getLocalTestPath, page, browserName }) => {
9+
if (browserName === 'webkit') {
10+
// The error we're throwing in this test is thrown as "Script error." in Webkit.
11+
// We filter "Script error." out by default in `InboundFilters`.
12+
// I don't think there's much value to disable InboundFilters defaults for this test,
13+
// given that most of our users won't do that either.
14+
// Let's skip it instead for Webkit.
15+
sentryTest.skip();
16+
}
17+
18+
const url = await getLocalTestPath({ testDir: __dirname });
19+
20+
const eventReqPromise = waitForErrorRequestOnUrl(page, url);
21+
22+
const clickPromise = page.click('#inline-error-btn');
23+
24+
const [req] = await Promise.all([eventReqPromise, clickPromise]);
25+
26+
const eventData = envelopeRequestParser(req);
27+
28+
expect(eventData.exception?.values).toHaveLength(1);
29+
30+
const exception = eventData.exception?.values?.[0];
31+
32+
expect(exception).toMatchObject({
33+
stacktrace: {
34+
frames: [
35+
{
36+
lineno: 12,
37+
pre_context: [' </script>', ' </head>', ' <body>'],
38+
context_line: ' <button id="inline-error-btn" onclick="throwTestError()">Click me</button>',
39+
post_context: [
40+
expect.stringContaining('subject.bundle.js'), // this line varies in the test based on tarball/cdn bundle (+variants)
41+
' <footer>',
42+
' Some text...',
43+
],
44+
},
45+
{
46+
lineno: 7,
47+
pre_context: [' <meta charset="utf-8">', ' <script>', ' function throwTestError() {'],
48+
context_line: " throw new Error('Error with context lines')",
49+
post_context: [' }', ' </script>', ' </head>'],
50+
},
51+
],
52+
},
53+
});
54+
},
55+
);

‎packages/integrations/src/contextlines.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,12 @@ export class ContextLines implements Integration {
5858
}
5959

6060
const html = doc.documentElement.innerHTML;
61-
62-
const htmlLines = ['<!DOCTYPE html>', '<html>', ...html.split('\n'), '</html>'];
63-
if (!htmlLines.length) {
61+
if (!html) {
6462
return event;
6563
}
6664

65+
const htmlLines = ['<!DOCTYPE html>', '<html>', ...html.split('\n'), '</html>'];
66+
6767
exceptions.forEach(exception => {
6868
const stacktrace = exception.stacktrace;
6969
if (stacktrace && stacktrace.frames) {

0 commit comments

Comments
 (0)