Skip to content

Commit 011ba71

Browse files
authored
fix(node): ANR fixes and additions (#9998)
This PR makes a few fixes and additions to the ANR feature. - Stops passing the `sdk` property with the event since `enhanceEventWithSdkInfo` results in duplicate integrations/packages being sent! - Allows the passings of `staticTags` to attach to ANR events - Required by Electron SDK to tag the source `process/origin/environment` - Could also be useful for other users - Optionally enable normalising of stack frame paths to the app root - Required for Electron - Soon required for Node with #9072 The path normalisation code (and tests) are from the Electron SDK and is well tested on all platforms. However, it will only be called when `appRootPath` is supplied. If/when we add path normalisation to Node, it will have a default which can be overridden. The Electron SDK will then wrap the Node Anr integration something like this: ```ts class Anr extends NodeAnr { public constructor(options: Partial<Options> = {}) { super({ ...options, staticTags: { 'event.environment': 'javascript', 'event.origin': 'electron', 'event.process': 'browser', ...options.tags, }, appRootPath: app.getAppPath(), }); } } ```
1 parent 25356d2 commit 011ba71

File tree

5 files changed

+134
-7
lines changed

5 files changed

+134
-7
lines changed

Diff for: packages/node/src/integrations/anr/common.ts

+11-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { Contexts, DsnComponents, SdkMetadata } from '@sentry/types';
1+
import type { Contexts, DsnComponents, Primitive, SdkMetadata } from '@sentry/types';
22

33
export interface Options {
44
/**
@@ -21,6 +21,16 @@ export interface Options {
2121
* This uses the node debugger which enables the inspector API and opens the required ports.
2222
*/
2323
captureStackTrace: boolean;
24+
/**
25+
* Tags to include with ANR events.
26+
*/
27+
staticTags: { [key: string]: Primitive };
28+
/**
29+
* @ignore Internal use only.
30+
*
31+
* If this is supplied, stack frame filenames will be rewritten to be relative to this path.
32+
*/
33+
appRootPath: string | undefined;
2434
}
2535

2636
export interface WorkerStartData extends Options {

Diff for: packages/node/src/integrations/anr/index.ts

+2
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,11 @@ async function _startWorker(client: NodeClient, _options: Partial<Options>): Pro
101101
release: initOptions.release,
102102
dist: initOptions.dist,
103103
sdkMetadata,
104+
appRootPath: _options.appRootPath,
104105
pollInterval: _options.pollInterval || DEFAULT_INTERVAL,
105106
anrThreshold: _options.anrThreshold || DEFAULT_HANG_THRESHOLD,
106107
captureStackTrace: !!_options.captureStackTrace,
108+
staticTags: _options.staticTags || {},
107109
contexts,
108110
};
109111

Diff for: packages/node/src/integrations/anr/worker.ts

+27-6
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {
66
updateSession,
77
} from '@sentry/core';
88
import type { Event, Session, StackFrame, TraceContext } from '@sentry/types';
9-
import { callFrameToStackFrame, stripSentryFramesAndReverse, watchdogTimer } from '@sentry/utils';
9+
import { callFrameToStackFrame, normalizeUrlToBase, stripSentryFramesAndReverse, watchdogTimer } from '@sentry/utils';
1010
import { Session as InspectorSession } from 'inspector';
1111
import { parentPort, workerData } from 'worker_threads';
1212
import { makeNodeTransport } from '../../transports';
@@ -56,6 +56,28 @@ async function sendAbnormalSession(): Promise<void> {
5656

5757
log('Started');
5858

59+
function prepareStackFrames(stackFrames: StackFrame[] | undefined): StackFrame[] | undefined {
60+
if (!stackFrames) {
61+
return undefined;
62+
}
63+
64+
// Strip Sentry frames and reverse the stack frames so they are in the correct order
65+
const strippedFrames = stripSentryFramesAndReverse(stackFrames);
66+
67+
// If we have an app root path, rewrite the filenames to be relative to the app root
68+
if (options.appRootPath) {
69+
for (const frame of strippedFrames) {
70+
if (!frame.filename) {
71+
continue;
72+
}
73+
74+
frame.filename = normalizeUrlToBase(frame.filename, options.appRootPath);
75+
}
76+
}
77+
78+
return strippedFrames;
79+
}
80+
5981
async function sendAnrEvent(frames?: StackFrame[], traceContext?: TraceContext): Promise<void> {
6082
if (hasSentAnrEvent) {
6183
return;
@@ -68,7 +90,6 @@ async function sendAnrEvent(frames?: StackFrame[], traceContext?: TraceContext):
6890
log('Sending event');
6991

7092
const event: Event = {
71-
sdk: options.sdkMetadata.sdk,
7293
contexts: { ...options.contexts, trace: traceContext },
7394
release: options.release,
7495
environment: options.environment,
@@ -80,13 +101,13 @@ async function sendAnrEvent(frames?: StackFrame[], traceContext?: TraceContext):
80101
{
81102
type: 'ApplicationNotResponding',
82103
value: `Application Not Responding for at least ${options.anrThreshold} ms`,
83-
stacktrace: { frames },
104+
stacktrace: { frames: prepareStackFrames(frames) },
84105
// This ensures the UI doesn't say 'Crashed in' for the stack trace
85106
mechanism: { type: 'ANR' },
86107
},
87108
],
88109
},
89-
tags: { 'process.name': 'ANR' },
110+
tags: options.staticTags,
90111
};
91112

92113
log(JSON.stringify(event));
@@ -130,8 +151,8 @@ if (options.captureStackTrace) {
130151
// copy the frames
131152
const callFrames = [...event.params.callFrames];
132153

133-
const stackFrames = stripSentryFramesAndReverse(
134-
callFrames.map(frame => callFrameToStackFrame(frame, scripts.get(frame.location.scriptId), () => undefined)),
154+
const stackFrames = callFrames.map(frame =>
155+
callFrameToStackFrame(frame, scripts.get(frame.location.scriptId), () => undefined),
135156
);
136157

137158
// Evaluate a script in the currently paused context

Diff for: packages/utils/src/normalize.ts

+30
Original file line numberDiff line numberDiff line change
@@ -277,3 +277,33 @@ function utf8Length(value: string): number {
277277
function jsonSize(value: any): number {
278278
return utf8Length(JSON.stringify(value));
279279
}
280+
281+
/**
282+
* Normalizes URLs in exceptions and stacktraces to a base path so Sentry can fingerprint
283+
* across platforms and working directory.
284+
*
285+
* @param url The URL to be normalized.
286+
* @param basePath The application base path.
287+
* @returns The normalized URL.
288+
*/
289+
export function normalizeUrlToBase(url: string, basePath: string): string {
290+
const escapedBase = basePath
291+
// Backslash to forward
292+
.replace(/\\/g, '/')
293+
// Escape RegExp special characters
294+
.replace(/[|\\{}()[\]^$+*?.]/g, '\\$&');
295+
296+
let newUrl = url;
297+
try {
298+
newUrl = decodeURI(url);
299+
} catch (_Oo) {
300+
// Sometime this breaks
301+
}
302+
return (
303+
newUrl
304+
.replace(/\\/g, '/')
305+
.replace(/webpack:\/?/g, '') // Remove intermediate base path
306+
// eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor
307+
.replace(new RegExp(`(file://)?/*${escapedBase}/*`, 'ig'), 'app:///')
308+
);
309+
}

Diff for: packages/utils/test/normalize-url.test.ts

+64
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
import { normalizeUrlToBase } from '../src/normalize';
2+
3+
describe('normalizeUrlToBase()', () => {
4+
it('Example app on Windows', () => {
5+
const base = 'c:/Users/Username/sentry-electron/example';
6+
7+
expect(normalizeUrlToBase('C:\\Users\\Username\\sentry-electron\\example\\renderer.js', base)).toEqual(
8+
'app:///renderer.js',
9+
);
10+
11+
expect(
12+
normalizeUrlToBase('C:\\Users\\Username\\sentry-electron\\example\\sub-directory\\renderer.js', base),
13+
).toEqual('app:///sub-directory/renderer.js');
14+
15+
expect(normalizeUrlToBase('file:///C:/Users/Username/sentry-electron/example/index.html', base)).toEqual(
16+
'app:///index.html',
17+
);
18+
});
19+
20+
it('Example app with parentheses', () => {
21+
const base = 'c:/Users/Username/sentry-electron (beta)/example';
22+
23+
expect(normalizeUrlToBase('C:\\Users\\Username\\sentry-electron%20(beta)\\example\\renderer.js', base)).toEqual(
24+
'app:///renderer.js',
25+
);
26+
27+
expect(
28+
normalizeUrlToBase('C:\\Users\\Username\\sentry-electron%20(beta)\\example\\sub-directory\\renderer.js', base),
29+
).toEqual('app:///sub-directory/renderer.js');
30+
31+
expect(normalizeUrlToBase('file:///C:/Users/Username/sentry-electron%20(beta)/example/index.html', base)).toEqual(
32+
'app:///index.html',
33+
);
34+
});
35+
36+
it('Asar packaged app in Windows Program Files', () => {
37+
const base = 'C:/Program Files/My App/resources/app.asar';
38+
39+
expect(normalizeUrlToBase('/C:/Program%20Files/My%20App/resources/app.asar/dist/bundle-app.js', base)).toEqual(
40+
'app:///dist/bundle-app.js',
41+
);
42+
43+
expect(normalizeUrlToBase('file:///C:/Program%20Files/My%20App/resources/app.asar/index.html', base)).toEqual(
44+
'app:///index.html',
45+
);
46+
47+
expect(normalizeUrlToBase('file:///C:/Program%20Files/My%20App/resources/app.asar/a/index.html', base)).toEqual(
48+
'app:///a/index.html',
49+
);
50+
});
51+
52+
it('Webpack builds', () => {
53+
const base = '/home/haza/Desktop/foo/app/';
54+
expect(
55+
normalizeUrlToBase('/home/haza/Desktop/foo/app/webpack:/electron/src/common/models/ipc-request.ts', base),
56+
).toEqual('app:///electron/src/common/models/ipc-request.ts');
57+
});
58+
59+
it('Only modifies file URLS', () => {
60+
const base = 'c:/Users/Username/sentry-electron/example';
61+
expect(normalizeUrlToBase('https://some.host/index.html', base)).toEqual('https://some.host/index.html');
62+
expect(normalizeUrlToBase('http://localhost:43288/index.html', base)).toEqual('http://localhost:43288/index.html');
63+
});
64+
});

0 commit comments

Comments
 (0)