Skip to content

Commit

Permalink
feat(node): Local variables via async inspector in node 19+ (#9962)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
timfish authored Jan 2, 2024
1 parent ff95fd5 commit 1953f2e
Show file tree
Hide file tree
Showing 12 changed files with 565 additions and 258 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,10 @@ function one(name) {
ty.two(name);
}

try {
one('some name');
} catch (e) {
Sentry.captureException(e);
}
setTimeout(() => {
try {
one('some name');
} catch (e) {
Sentry.captureException(e);
}
}, 1000);
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,10 @@ function one(name) {
ty.two(name);
}

try {
one('some name');
} catch (e) {
Sentry.captureException(e);
}
setTimeout(() => {
try {
one('some name');
} catch (e) {
Sentry.captureException(e);
}
}, 1000);
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,6 @@ function one(name) {
ty.two(name);
}

one('some name');
setTimeout(() => {
one('some name');
}, 1000);
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,6 @@ function one(name) {
ty.two(name);
}

one('some name');
setTimeout(() => {
one('some name');
}, 1000);
2 changes: 1 addition & 1 deletion packages/node/src/integrations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export { Modules } from './modules';
export { ContextLines } from './contextlines';
export { Context } from './context';
export { RequestData } from '@sentry/core';
export { LocalVariables } from './localvariables';
export { LocalVariables } from './local-variables';
export { Undici } from './undici';
export { Spotlight } from './spotlight';
export { Anr } from './anr';
Expand Down
119 changes: 119 additions & 0 deletions packages/node/src/integrations/local-variables/common.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
import type { StackFrame, StackParser } from '@sentry/types';
import type { Debugger } from 'inspector';

export type Variables = Record<string, unknown>;

export type RateLimitIncrement = () => void;

/**
* Creates a rate limiter that will call the disable callback when the rate limit is reached and the enable callback
* when a timeout has occurred.
* @param maxPerSecond Maximum number of calls per second
* @param enable Callback to enable capture
* @param disable Callback to disable capture
* @returns A function to call to increment the rate limiter count
*/
export function createRateLimiter(
maxPerSecond: number,
enable: () => void,
disable: (seconds: number) => void,
): RateLimitIncrement {
let count = 0;
let retrySeconds = 5;
let disabledTimeout = 0;

setInterval(() => {
if (disabledTimeout === 0) {
if (count > maxPerSecond) {
retrySeconds *= 2;
disable(retrySeconds);

// Cap at one day
if (retrySeconds > 86400) {
retrySeconds = 86400;
}
disabledTimeout = retrySeconds;
}
} else {
disabledTimeout -= 1;

if (disabledTimeout === 0) {
enable();
}
}

count = 0;
}, 1_000).unref();

return () => {
count += 1;
};
}

// Add types for the exception event data
export type PausedExceptionEvent = Debugger.PausedEventDataType & {
data: {
// This contains error.stack
description: string;
};
};

/** Could this be an anonymous function? */
export function isAnonymous(name: string | undefined): boolean {
return name !== undefined && (name.length === 0 || name === '?' || name === '<anonymous>');
}

/** Do the function names appear to match? */
export function functionNamesMatch(a: string | undefined, b: string | undefined): boolean {
return a === b || (isAnonymous(a) && isAnonymous(b));
}

/** Creates a unique hash from stack frames */
export function hashFrames(frames: StackFrame[] | undefined): string | undefined {
if (frames === undefined) {
return;
}

// Only hash the 10 most recent frames (ie. the last 10)
return frames.slice(-10).reduce((acc, frame) => `${acc},${frame.function},${frame.lineno},${frame.colno}`, '');
}

/**
* We use the stack parser to create a unique hash from the exception stack trace
* This is used to lookup vars when the exception passes through the event processor
*/
export function hashFromStack(stackParser: StackParser, stack: string | undefined): string | undefined {
if (stack === undefined) {
return undefined;
}

return hashFrames(stackParser(stack, 1));
}

export interface FrameVariables {
function: string;
vars?: Variables;
}

export interface Options {
/**
* Capture local variables for both caught and uncaught exceptions
*
* - When false, only uncaught exceptions will have local variables
* - When true, both caught and uncaught exceptions will have local variables.
*
* Defaults to `true`.
*
* Capturing local variables for all exceptions can be expensive since the debugger pauses for every throw to collect
* local variables.
*
* To reduce the likelihood of this feature impacting app performance or throughput, this feature is rate-limited.
* Once the rate limit is reached, local variables will only be captured for uncaught exceptions until a timeout has
* been reached.
*/
captureAllExceptions?: boolean;
/**
* Maximum number of exceptions to capture local variables for per second before rate limiting is triggered.
*/
maxExceptionsPerSecond?: number;
}
21 changes: 21 additions & 0 deletions packages/node/src/integrations/local-variables/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { convertIntegrationFnToClass } from '@sentry/core';
import type { IntegrationFn } from '@sentry/types';
import { NODE_VERSION } from '../../nodeVersion';
import type { Options } from './common';
import { localVariablesAsync } from './local-variables-async';
import { localVariablesSync } from './local-variables-sync';

const INTEGRATION_NAME = 'LocalVariables';

/**
* Adds local variables to exception frames
*/
const localVariables: IntegrationFn = (options: Options = {}) => {
return NODE_VERSION.major < 19 ? localVariablesSync(options) : localVariablesAsync(options);
};

/**
* Adds local variables to exception frames
*/
// eslint-disable-next-line deprecation/deprecation
export const LocalVariables = convertIntegrationFnToClass(INTEGRATION_NAME, localVariables);
Original file line number Diff line number Diff line change
Expand Up @@ -3357,3 +3357,31 @@ declare module 'node:inspector' {
import inspector = require('inspector');
export = inspector;
}

/**
* @types/node doesn't have a `node:inspector/promises` module, maybe because it's still experimental?
*/
declare module 'node:inspector/promises' {
/**
* Async Debugger session
*/
class Session {
constructor();

connect(): void;

post(method: 'Debugger.pause' | 'Debugger.resume' | 'Debugger.enable' | 'Debugger.disable'): Promise<void>;
post(method: 'Debugger.setPauseOnExceptions', params: Debugger.SetPauseOnExceptionsParameterType): Promise<void>;
post(
method: 'Runtime.getProperties',
params: Runtime.GetPropertiesParameterType,
): Promise<Runtime.GetPropertiesReturnType>;

on(
event: 'Debugger.paused',
listener: (message: InspectorNotification<Debugger.PausedEventDataType>) => void,
): Session;

on(event: 'Debugger.resumed', listener: () => void): Session;
}
}
Loading

0 comments on commit 1953f2e

Please sign in to comment.