Skip to content

Commit 1953f2e

Browse files
authored
feat(node): Local variables via async inspector in node 19+ (#9962)
This PR creates a new `LocalVariables` integration that uses the async inspector API. Rather than create a huge messy integration that supports both the sync (node 18) and async (node >= v19) APIs, I created a new integration and wrapped both the sync and async integrations with user facing integration that switches depending on node version. The async API doesn't require the stacking of callbacks that risks stack overflows and limits the number of frames we dare to evaluate. When we tried wrapping the sync API with promises, memory was leaked at an alarming rate! The inspector APIs are not available on all builds of Node so we have to lazily load it and catch any exceptions. I've had to use `dynamicRequire` because webpack picks up `import()` and reports missing dependency when bundling for older versions of node.
1 parent ff95fd5 commit 1953f2e

File tree

12 files changed

+565
-258
lines changed

12 files changed

+565
-258
lines changed

packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.js

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,10 @@ function one(name) {
2828
ty.two(name);
2929
}
3030

31-
try {
32-
one('some name');
33-
} catch (e) {
34-
Sentry.captureException(e);
35-
}
31+
setTimeout(() => {
32+
try {
33+
one('some name');
34+
} catch (e) {
35+
Sentry.captureException(e);
36+
}
37+
}, 1000);

packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-caught.mjs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,10 @@ function one(name) {
3030
ty.two(name);
3131
}
3232

33-
try {
34-
one('some name');
35-
} catch (e) {
36-
Sentry.captureException(e);
37-
}
33+
setTimeout(() => {
34+
try {
35+
one('some name');
36+
} catch (e) {
37+
Sentry.captureException(e);
38+
}
39+
}, 1000);

packages/node-integration-tests/suites/public-api/LocalVariables/local-variables.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,6 @@ function one(name) {
3232
ty.two(name);
3333
}
3434

35-
one('some name');
35+
setTimeout(() => {
36+
one('some name');
37+
}, 1000);

packages/node-integration-tests/suites/public-api/LocalVariables/no-local-variables.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,6 @@ function one(name) {
3131
ty.two(name);
3232
}
3333

34-
one('some name');
34+
setTimeout(() => {
35+
one('some name');
36+
}, 1000);

packages/node/src/integrations/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ export { Modules } from './modules';
66
export { ContextLines } from './contextlines';
77
export { Context } from './context';
88
export { RequestData } from '@sentry/core';
9-
export { LocalVariables } from './localvariables';
9+
export { LocalVariables } from './local-variables';
1010
export { Undici } from './undici';
1111
export { Spotlight } from './spotlight';
1212
export { Anr } from './anr';
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
import type { StackFrame, StackParser } from '@sentry/types';
2+
import type { Debugger } from 'inspector';
3+
4+
export type Variables = Record<string, unknown>;
5+
6+
export type RateLimitIncrement = () => void;
7+
8+
/**
9+
* Creates a rate limiter that will call the disable callback when the rate limit is reached and the enable callback
10+
* when a timeout has occurred.
11+
* @param maxPerSecond Maximum number of calls per second
12+
* @param enable Callback to enable capture
13+
* @param disable Callback to disable capture
14+
* @returns A function to call to increment the rate limiter count
15+
*/
16+
export function createRateLimiter(
17+
maxPerSecond: number,
18+
enable: () => void,
19+
disable: (seconds: number) => void,
20+
): RateLimitIncrement {
21+
let count = 0;
22+
let retrySeconds = 5;
23+
let disabledTimeout = 0;
24+
25+
setInterval(() => {
26+
if (disabledTimeout === 0) {
27+
if (count > maxPerSecond) {
28+
retrySeconds *= 2;
29+
disable(retrySeconds);
30+
31+
// Cap at one day
32+
if (retrySeconds > 86400) {
33+
retrySeconds = 86400;
34+
}
35+
disabledTimeout = retrySeconds;
36+
}
37+
} else {
38+
disabledTimeout -= 1;
39+
40+
if (disabledTimeout === 0) {
41+
enable();
42+
}
43+
}
44+
45+
count = 0;
46+
}, 1_000).unref();
47+
48+
return () => {
49+
count += 1;
50+
};
51+
}
52+
53+
// Add types for the exception event data
54+
export type PausedExceptionEvent = Debugger.PausedEventDataType & {
55+
data: {
56+
// This contains error.stack
57+
description: string;
58+
};
59+
};
60+
61+
/** Could this be an anonymous function? */
62+
export function isAnonymous(name: string | undefined): boolean {
63+
return name !== undefined && (name.length === 0 || name === '?' || name === '<anonymous>');
64+
}
65+
66+
/** Do the function names appear to match? */
67+
export function functionNamesMatch(a: string | undefined, b: string | undefined): boolean {
68+
return a === b || (isAnonymous(a) && isAnonymous(b));
69+
}
70+
71+
/** Creates a unique hash from stack frames */
72+
export function hashFrames(frames: StackFrame[] | undefined): string | undefined {
73+
if (frames === undefined) {
74+
return;
75+
}
76+
77+
// Only hash the 10 most recent frames (ie. the last 10)
78+
return frames.slice(-10).reduce((acc, frame) => `${acc},${frame.function},${frame.lineno},${frame.colno}`, '');
79+
}
80+
81+
/**
82+
* We use the stack parser to create a unique hash from the exception stack trace
83+
* This is used to lookup vars when the exception passes through the event processor
84+
*/
85+
export function hashFromStack(stackParser: StackParser, stack: string | undefined): string | undefined {
86+
if (stack === undefined) {
87+
return undefined;
88+
}
89+
90+
return hashFrames(stackParser(stack, 1));
91+
}
92+
93+
export interface FrameVariables {
94+
function: string;
95+
vars?: Variables;
96+
}
97+
98+
export interface Options {
99+
/**
100+
* Capture local variables for both caught and uncaught exceptions
101+
*
102+
* - When false, only uncaught exceptions will have local variables
103+
* - When true, both caught and uncaught exceptions will have local variables.
104+
*
105+
* Defaults to `true`.
106+
*
107+
* Capturing local variables for all exceptions can be expensive since the debugger pauses for every throw to collect
108+
* local variables.
109+
*
110+
* To reduce the likelihood of this feature impacting app performance or throughput, this feature is rate-limited.
111+
* Once the rate limit is reached, local variables will only be captured for uncaught exceptions until a timeout has
112+
* been reached.
113+
*/
114+
captureAllExceptions?: boolean;
115+
/**
116+
* Maximum number of exceptions to capture local variables for per second before rate limiting is triggered.
117+
*/
118+
maxExceptionsPerSecond?: number;
119+
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import { convertIntegrationFnToClass } from '@sentry/core';
2+
import type { IntegrationFn } from '@sentry/types';
3+
import { NODE_VERSION } from '../../nodeVersion';
4+
import type { Options } from './common';
5+
import { localVariablesAsync } from './local-variables-async';
6+
import { localVariablesSync } from './local-variables-sync';
7+
8+
const INTEGRATION_NAME = 'LocalVariables';
9+
10+
/**
11+
* Adds local variables to exception frames
12+
*/
13+
const localVariables: IntegrationFn = (options: Options = {}) => {
14+
return NODE_VERSION.major < 19 ? localVariablesSync(options) : localVariablesAsync(options);
15+
};
16+
17+
/**
18+
* Adds local variables to exception frames
19+
*/
20+
// eslint-disable-next-line deprecation/deprecation
21+
export const LocalVariables = convertIntegrationFnToClass(INTEGRATION_NAME, localVariables);

packages/node/src/integrations/inspector.d.ts renamed to packages/node/src/integrations/local-variables/inspector.d.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3357,3 +3357,31 @@ declare module 'node:inspector' {
33573357
import inspector = require('inspector');
33583358
export = inspector;
33593359
}
3360+
3361+
/**
3362+
* @types/node doesn't have a `node:inspector/promises` module, maybe because it's still experimental?
3363+
*/
3364+
declare module 'node:inspector/promises' {
3365+
/**
3366+
* Async Debugger session
3367+
*/
3368+
class Session {
3369+
constructor();
3370+
3371+
connect(): void;
3372+
3373+
post(method: 'Debugger.pause' | 'Debugger.resume' | 'Debugger.enable' | 'Debugger.disable'): Promise<void>;
3374+
post(method: 'Debugger.setPauseOnExceptions', params: Debugger.SetPauseOnExceptionsParameterType): Promise<void>;
3375+
post(
3376+
method: 'Runtime.getProperties',
3377+
params: Runtime.GetPropertiesParameterType,
3378+
): Promise<Runtime.GetPropertiesReturnType>;
3379+
3380+
on(
3381+
event: 'Debugger.paused',
3382+
listener: (message: InspectorNotification<Debugger.PausedEventDataType>) => void,
3383+
): Session;
3384+
3385+
on(event: 'Debugger.resumed', listener: () => void): Session;
3386+
}
3387+
}

0 commit comments

Comments
 (0)