diff --git a/packages/node/src/integrations/localvariables.ts b/packages/node/src/integrations/localvariables.ts index f98011d94832..39f250c23778 100644 --- a/packages/node/src/integrations/localvariables.ts +++ b/packages/node/src/integrations/localvariables.ts @@ -1,3 +1,4 @@ +/* eslint-disable max-lines */ import type { Event, EventProcessor, Exception, Hub, Integration, StackFrame, StackParser } from '@sentry/types'; import { logger } from '@sentry/utils'; import type { Debugger, InspectorNotification, Runtime, Session } from 'inspector'; @@ -11,6 +12,8 @@ type OnPauseEvent = InspectorNotification; export interface DebugSession { /** Configures and connects to the debug session */ configureAndConnect(onPause: (message: OnPauseEvent, complete: () => void) => void, captureAll: boolean): void; + /** Updates which kind of exceptions to capture */ + setPauseOnExceptions(captureAll: boolean): void; /** Gets local variables for an objectId */ getLocalVariables(objectId: string, callback: (vars: Variables) => void): void; } @@ -19,6 +22,52 @@ type Next = (result: T) => void; type Add = (fn: Next) => void; type CallbackWrapper = { add: Add; next: Next }; +type RateLimitIncrement = () => void; + +/** + * Creates a rate limiter + * @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; + }; +} + /** Creates a container for callbacks to be called sequentially */ export function createCallbackList(complete: Next): CallbackWrapper { // A collection of callbacks to be executed last to first @@ -103,6 +152,10 @@ class AsyncSession implements DebugSession { this._session.post('Debugger.setPauseOnExceptions', { state: captureAll ? 'all' : 'uncaught' }); } + public setPauseOnExceptions(captureAll: boolean): void { + this._session.post('Debugger.setPauseOnExceptions', { state: captureAll ? 'all' : 'uncaught' }); + } + /** @inheritdoc */ public getLocalVariables(objectId: string, complete: (vars: Variables) => void): void { this._getProperties(objectId, props => { @@ -245,19 +298,33 @@ export interface FrameVariables { vars?: Variables; } -/** There are no options yet. This allows them to be added later without breaking changes */ -// eslint-disable-next-line @typescript-eslint/no-empty-interface interface Options { /** - * Capture local variables for both handled and unhandled exceptions + * 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`. * - * Default: false - Only captures local variables for uncaught exceptions + * 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; } /** * Adds local variables to exception frames + * + * Default: 50 */ export class LocalVariables implements Integration { public static id: string = 'LocalVariables'; @@ -265,6 +332,7 @@ export class LocalVariables implements Integration { public readonly name: string = LocalVariables.id; private readonly _cachedFrames: LRUMap = new LRUMap(20); + private _rateLimiter: RateLimitIncrement | undefined; public constructor( private readonly _options: Options = {}, @@ -293,12 +361,32 @@ export class LocalVariables implements Integration { return; } + const captureAll = this._options.captureAllExceptions !== false; + this._session.configureAndConnect( (ev, complete) => this._handlePaused(clientOptions.stackParser, ev as InspectorNotification, complete), - !!this._options.captureAllExceptions, + captureAll, ); + if (captureAll) { + const max = this._options.maxExceptionsPerSecond || 50; + + this._rateLimiter = createRateLimiter( + max, + () => { + logger.log('Local variables rate-limit lifted.'); + this._session?.setPauseOnExceptions(true); + }, + seconds => { + logger.log( + `Local variables rate-limit exceeded. Disabling capturing of caught exceptions for ${seconds} seconds.`, + ); + this._session?.setPauseOnExceptions(false); + }, + ); + } + addGlobalEventProcessor(async event => this._addLocalVariables(event)); } } @@ -316,6 +404,8 @@ export class LocalVariables implements Integration { return; } + this._rateLimiter?.(); + // data.description contains the original error.stack const exceptionHash = hashFromStack(stackParser, data?.description); diff --git a/packages/node/test/integrations/localvariables.test.ts b/packages/node/test/integrations/localvariables.test.ts index 15f62f305231..55e179c879e6 100644 --- a/packages/node/test/integrations/localvariables.test.ts +++ b/packages/node/test/integrations/localvariables.test.ts @@ -4,10 +4,12 @@ import type { LRUMap } from 'lru_map'; import { defaultStackParser } from '../../src'; import type { DebugSession, FrameVariables } from '../../src/integrations/localvariables'; -import { createCallbackList, LocalVariables } from '../../src/integrations/localvariables'; +import { createCallbackList, createRateLimiter, LocalVariables } from '../../src/integrations/localvariables'; import { NODE_VERSION } from '../../src/nodeVersion'; import { getDefaultNodeClientOptions } from '../../test/helper/node-client-options'; +jest.setTimeout(20_000); + const describeIf = (condition: boolean) => (condition ? describe : describe.skip); interface ThrowOn { @@ -31,6 +33,8 @@ class MockDebugSession implements DebugSession { this._onPause = onPause; } + public setPauseOnExceptions(_: boolean): void {} + public getLocalVariables(objectId: string, callback: (vars: Record) => void): void { if (this._throwOn?.getLocalVariables) { throw new Error('getLocalVariables should not be called'); @@ -431,4 +435,67 @@ describeIf((NODE_VERSION.major || 0) >= 18)('LocalVariables', () => { next(10); }); }); + + describe('rateLimiter', () => { + it('calls disable if exceeded', done => { + const increment = createRateLimiter( + 5, + () => {}, + () => { + done(); + }, + ); + + for (let i = 0; i < 7; i++) { + increment(); + } + }); + + it('does not call disable if not exceeded', done => { + const increment = createRateLimiter( + 5, + () => { + throw new Error('Should not be called'); + }, + () => { + throw new Error('Should not be called'); + }, + ); + + let count = 0; + + const timer = setInterval(() => { + for (let i = 0; i < 4; i++) { + increment(); + } + + count += 1; + + if (count >= 5) { + clearInterval(timer); + done(); + } + }, 1_000); + }); + + it('re-enables after timeout', done => { + let called = false; + + const increment = createRateLimiter( + 5, + () => { + expect(called).toEqual(true); + done(); + }, + () => { + expect(called).toEqual(false); + called = true; + }, + ); + + for (let i = 0; i < 10; i++) { + increment(); + } + }); + }); });