Skip to content

Commit

Permalink
fix(node): ANR fixes and additions (#9998)
Browse files Browse the repository at this point in the history
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(),
    });
  }
}
```
  • Loading branch information
timfish authored and anonrig committed Jan 3, 2024
1 parent 0626bac commit 28cc9fd
Show file tree
Hide file tree
Showing 5 changed files with 134 additions and 7 deletions.
12 changes: 11 additions & 1 deletion packages/node/src/integrations/anr/common.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Contexts, DsnComponents, SdkMetadata } from '@sentry/types';
import type { Contexts, DsnComponents, Primitive, SdkMetadata } from '@sentry/types';

export interface Options {
/**
Expand All @@ -21,6 +21,16 @@ export interface Options {
* This uses the node debugger which enables the inspector API and opens the required ports.
*/
captureStackTrace: boolean;
/**
* Tags to include with ANR events.
*/
staticTags: { [key: string]: Primitive };
/**
* @ignore Internal use only.
*
* If this is supplied, stack frame filenames will be rewritten to be relative to this path.
*/
appRootPath: string | undefined;
}

export interface WorkerStartData extends Options {
Expand Down
2 changes: 2 additions & 0 deletions packages/node/src/integrations/anr/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,11 @@ async function _startWorker(client: NodeClient, _options: Partial<Options>): Pro
release: initOptions.release,
dist: initOptions.dist,
sdkMetadata,
appRootPath: _options.appRootPath,
pollInterval: _options.pollInterval || DEFAULT_INTERVAL,
anrThreshold: _options.anrThreshold || DEFAULT_HANG_THRESHOLD,
captureStackTrace: !!_options.captureStackTrace,
staticTags: _options.staticTags || {},
contexts,
};

Expand Down
33 changes: 27 additions & 6 deletions packages/node/src/integrations/anr/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
updateSession,
} from '@sentry/core';
import type { Event, Session, StackFrame, TraceContext } from '@sentry/types';
import { callFrameToStackFrame, stripSentryFramesAndReverse, watchdogTimer } from '@sentry/utils';
import { callFrameToStackFrame, normalizeUrlToBase, stripSentryFramesAndReverse, watchdogTimer } from '@sentry/utils';
import { Session as InspectorSession } from 'inspector';
import { parentPort, workerData } from 'worker_threads';
import { makeNodeTransport } from '../../transports';
Expand Down Expand Up @@ -56,6 +56,28 @@ async function sendAbnormalSession(): Promise<void> {

log('Started');

function prepareStackFrames(stackFrames: StackFrame[] | undefined): StackFrame[] | undefined {
if (!stackFrames) {
return undefined;
}

// Strip Sentry frames and reverse the stack frames so they are in the correct order
const strippedFrames = stripSentryFramesAndReverse(stackFrames);

// If we have an app root path, rewrite the filenames to be relative to the app root
if (options.appRootPath) {
for (const frame of strippedFrames) {
if (!frame.filename) {
continue;
}

frame.filename = normalizeUrlToBase(frame.filename, options.appRootPath);
}
}

return strippedFrames;
}

async function sendAnrEvent(frames?: StackFrame[], traceContext?: TraceContext): Promise<void> {
if (hasSentAnrEvent) {
return;
Expand All @@ -68,7 +90,6 @@ async function sendAnrEvent(frames?: StackFrame[], traceContext?: TraceContext):
log('Sending event');

const event: Event = {
sdk: options.sdkMetadata.sdk,
contexts: { ...options.contexts, trace: traceContext },
release: options.release,
environment: options.environment,
Expand All @@ -80,13 +101,13 @@ async function sendAnrEvent(frames?: StackFrame[], traceContext?: TraceContext):
{
type: 'ApplicationNotResponding',
value: `Application Not Responding for at least ${options.anrThreshold} ms`,
stacktrace: { frames },
stacktrace: { frames: prepareStackFrames(frames) },
// This ensures the UI doesn't say 'Crashed in' for the stack trace
mechanism: { type: 'ANR' },
},
],
},
tags: { 'process.name': 'ANR' },
tags: options.staticTags,
};

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

const stackFrames = stripSentryFramesAndReverse(
callFrames.map(frame => callFrameToStackFrame(frame, scripts.get(frame.location.scriptId), () => undefined)),
const stackFrames = callFrames.map(frame =>
callFrameToStackFrame(frame, scripts.get(frame.location.scriptId), () => undefined),
);

// Evaluate a script in the currently paused context
Expand Down
30 changes: 30 additions & 0 deletions packages/utils/src/normalize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,3 +277,33 @@ function utf8Length(value: string): number {
function jsonSize(value: any): number {
return utf8Length(JSON.stringify(value));
}

/**
* Normalizes URLs in exceptions and stacktraces to a base path so Sentry can fingerprint
* across platforms and working directory.
*
* @param url The URL to be normalized.
* @param basePath The application base path.
* @returns The normalized URL.
*/
export function normalizeUrlToBase(url: string, basePath: string): string {
const escapedBase = basePath
// Backslash to forward
.replace(/\\/g, '/')
// Escape RegExp special characters
.replace(/[|\\{}()[\]^$+*?.]/g, '\\$&');

let newUrl = url;
try {
newUrl = decodeURI(url);
} catch (_Oo) {
// Sometime this breaks
}
return (
newUrl
.replace(/\\/g, '/')
.replace(/webpack:\/?/g, '') // Remove intermediate base path
// eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor
.replace(new RegExp(`(file://)?/*${escapedBase}/*`, 'ig'), 'app:///')
);
}
64 changes: 64 additions & 0 deletions packages/utils/test/normalize-url.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { normalizeUrlToBase } from '../src/normalize';

describe('normalizeUrlToBase()', () => {
it('Example app on Windows', () => {
const base = 'c:/Users/Username/sentry-electron/example';

expect(normalizeUrlToBase('C:\\Users\\Username\\sentry-electron\\example\\renderer.js', base)).toEqual(
'app:///renderer.js',
);

expect(
normalizeUrlToBase('C:\\Users\\Username\\sentry-electron\\example\\sub-directory\\renderer.js', base),
).toEqual('app:///sub-directory/renderer.js');

expect(normalizeUrlToBase('file:///C:/Users/Username/sentry-electron/example/index.html', base)).toEqual(
'app:///index.html',
);
});

it('Example app with parentheses', () => {
const base = 'c:/Users/Username/sentry-electron (beta)/example';

expect(normalizeUrlToBase('C:\\Users\\Username\\sentry-electron%20(beta)\\example\\renderer.js', base)).toEqual(
'app:///renderer.js',
);

expect(
normalizeUrlToBase('C:\\Users\\Username\\sentry-electron%20(beta)\\example\\sub-directory\\renderer.js', base),
).toEqual('app:///sub-directory/renderer.js');

expect(normalizeUrlToBase('file:///C:/Users/Username/sentry-electron%20(beta)/example/index.html', base)).toEqual(
'app:///index.html',
);
});

it('Asar packaged app in Windows Program Files', () => {
const base = 'C:/Program Files/My App/resources/app.asar';

expect(normalizeUrlToBase('/C:/Program%20Files/My%20App/resources/app.asar/dist/bundle-app.js', base)).toEqual(
'app:///dist/bundle-app.js',
);

expect(normalizeUrlToBase('file:///C:/Program%20Files/My%20App/resources/app.asar/index.html', base)).toEqual(
'app:///index.html',
);

expect(normalizeUrlToBase('file:///C:/Program%20Files/My%20App/resources/app.asar/a/index.html', base)).toEqual(
'app:///a/index.html',
);
});

it('Webpack builds', () => {
const base = '/home/haza/Desktop/foo/app/';
expect(
normalizeUrlToBase('/home/haza/Desktop/foo/app/webpack:/electron/src/common/models/ipc-request.ts', base),
).toEqual('app:///electron/src/common/models/ipc-request.ts');
});

it('Only modifies file URLS', () => {
const base = 'c:/Users/Username/sentry-electron/example';
expect(normalizeUrlToBase('https://some.host/index.html', base)).toEqual('https://some.host/index.html');
expect(normalizeUrlToBase('http://localhost:43288/index.html', base)).toEqual('http://localhost:43288/index.html');
});
});

0 comments on commit 28cc9fd

Please sign in to comment.