8000 feat(integrations): Add `ContextLines` integration for html-embedded … · kiner-tang/sentry-javascript@436fdeb · GitHub
[go: up one dir, main page]
Skip to content

Commit 436fdeb

Browse files
Lms24Lms24
authored
feat(integrations): Add ContextLines integration for html-embedded JS stack frames (getsentry#8699)
Adds a "best-effort" `ContextLines` integration as an optional integration for the browser SDKs to pick up source code of and around stack frames pointing to code that's directly embedded in the current page's html. See PR and linked issue about limitations.
1 parent 7947e34 commit 436fdeb

File tree

11 files changed

+438
-0
lines changed

11 files changed

+438
-0
lines changed

‎packages/browser-integration-tests/suites/integrations/ContextLines/init.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import * as Sentry from '@sentry/browser';
2+
import { ContextLines } from '@sentry/integrations';
3+
4+
window.Sentry = Sentry;
5+
6+
Sentry.init({
7+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
8+
integrations: [new ContextLines()],
9+
});

‎packages/browser-integration-tests/suites/integrations/ContextLines/inline/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="inline-error-btn" onclick="throw new Error('Error with context lines')">Click me</button>
8+
</body>
9+
<footer>
10+
Some text...
11+
</foot>
12+
</html>

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
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 inline JS',
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+
pre_context: ['<!DOCTYPE html>', '<html>', '<head>', ' <meta charset="utf-8">', ' </head>', ' <body>'],
37+
context_line:
38+
' <button id="inline-error-btn" onclick="throw new Error(\'Error with context lines\')">Click me</button>',
39+
post_context: [
40+
expect.stringContaining('<script'), // this line varies in the test based on tarball/cdn bundle (+variants)
41+
' <footer>',
42+
' Some text...',
43+
' ',
44+
'',
45+
'</footer></body>',
46+
'</html>',
47+
],
48+
},
49+
],
50+
},
51+
});
52+
},
53+
);

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
document.getElementById('script-error-btn').addEventListener('click', () => {
2+
throw new Error('Error without context lines');
3+
});

‎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: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
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: [
38+
' <script>',
39+
' function throwTestError() {',
40+
" throw new Error('Error with context lines')",
41+
' }',
42+
' </script>',
43+
' </head>',
44+
' <body>',
45+
],
46+
context_line: ' <button id="inline-error-btn" onclick="throwTestError()">Click me</button>',
47+
post_context: [
48+
expect.stringContaining('<script'), // this line varies in the test based on tarball/cdn bundle (+variants)
49+
' <footer>',
50+
' Some text...',
51+
' ',
52+
'',
53+
'</footer></body>',
54+
'</html>',
55+
],
56+
},
57+
{
58+
lineno: 7,
59+
pre_context: [
60+
'<!DOCTYPE html>',
61+
'<html>',
62+
'<head>',
63+
' <meta charset="utf-8">',
64+
' <script>',
65+
' function throwTestError() {',
66+
],
67+
context_line: " throw new Error('Error with context lines')",
68+
post_context: [
69+
' }',
70+
' </script>',
71+
' </head>',
72+
' <body>',
73+
' <button id="inline-error-btn" onclick="throwTestError()">Click me</button>',
74+
expect.stringContaining('<script'), // this line varies in the test based on tarball/cdn bundle (+variants)
75+
' <footer>',
76+
],
77+
},
78+
],
79+
},
80+
});
81+
},
82+
);

‎packages/integrations/src/contextlines.ts

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
import type { Event, EventProcessor, Hub, Integration, StackFrame } from '@sentry/types';
2+
import { addContextToFrame, GLOBAL_OBJ, stripUrlQueryAndFragment } from '@sentry/utils';
3+
4+
const WINDOW = GLOBAL_OBJ as typeof GLOBAL_OBJ & Window;
5+
6+
const DEFAULT_LINES_OF_CONTEXT = 7;
7+
8+
interface ContextLinesOptions {
9+
/**
10+
* Sets the number of context lines for each frame when loading a file.
11+
* Defaults to 7.
12+
*
13+
* Set to 0 to disable loading and inclusion of source files.
14+
**/
15+
frameContextLines?: number;
16+
}
17+
18+
/**
19+
* Collects source context lines around the lines of stackframes pointing to JS embedded in
20+
* the current page's HTML.
21+
*
22+
* This integration DOES NOT work for stack frames pointing to JS files that are loaded by the browser.
23+
* For frames pointing to files, context lines are added during ingestion and symbolication
24+
* by attempting to download the JS files to the Sentry backend.
25+
*
26+
* Use this integration if you have inline JS code in HTML pages that can't be accessed
27+
* by our backend (e.g. due to a login-protected page).
28+
*/
29+
export class ContextLines implements Integration {
30+
/**
31+
* @inheritDoc
32+
*/
33+
public static id: string = 'ContextLines';
34+
35+
/**
36+
* @inheritDoc
37+
*/
38+
public name: string;
39+
40+
public constructor(private readonly _options: ContextLinesOptions = {}) {
41+
this.name = ContextLines.id;
42+
}
43+
44+
/**
45+
* @inheritDoc
46+
*/
47+
public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
48+
addGlobalEventProcessor(event => {
49+
const self = getCurrentHub().getIntegration(ContextLines);
50+
if (!self) {
51+
return event;
52+
}
53+
return this.addSourceContext(event);
54+
});
55+
}
56+
57+
/** Processes an event and adds context lines */
58+
public addSourceContext(event: Event): Event {
59+
const doc = WINDOW.document;
60+
const htmlFilename = WINDOW.location && stripUrlQueryAndFragment(WINDOW.location.href);
61+
if (!doc || !htmlFilename) {
62+
return event;
63+
}
64+
65+
const exceptions = event.exception && event.exception.values;
66+
if (!exceptions || !exceptions.length) {
67+
return event;
68+
}
69+
70+
const html = doc.documentElement.innerHTML;
71+
if (!html) {
72+
return event;
73+
}
74+
75+
const htmlLines = ['<!DOCTYPE html>', '<html>', ...html.split('\n'), '</html>'];
76+
77+
exceptions.forEach(exception => {
78+
const stacktrace = exception.stacktrace;
79+
if (stacktrace && stacktrace.frames) {
80+
stacktrace.frames = stacktrace.frames.map(frame =>
81+
applySourceContextToFrame(
82+
frame,
83+
htmlLines,
84+
htmlFilename,
85+
this._options.frameContextLines != null ? this._options.frameContextLines : DEFAULT_LINES_OF_CONTEXT,
86+
),
87+
);
88+
}
89+
});
90+
91+
return event;
92+
}
93+
}
94+
95+
/**
96+
* Only exported for testing
97+
*/
98+
export function applySourceContextToFrame(
99+
frame: StackFrame,
100+
htmlLines: string[],
101+
htmlFilename: string,
102+
linesOfContext: number,
103+
): StackFrame {
104+
if (frame.filename !== htmlFilename || !frame.lineno || !htmlLines.length) {
105+
return frame;
106+
}
107+
108+
addContextToFrame(htmlLines, frame, linesOfContext);
109+
110+
return frame;
111+
}

‎packages/integrations/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,4 @@ export { RewriteFrames } from './rewriteframes';
99
export { SessionTiming } from './sessiontiming';
1010
export { Transaction } from './transaction';
1111
export { HttpClient } from './httpclient';
12+
export { ContextLines } from './contextlines';

0 commit comments

Comments
 (0)
0