Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

fix(node): Fix local variables capturing for out-of-app frames #17545

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JealousGx wants to merge 5 commits into getsentry:develop
base: develop
Choose a base branch
Loading
from JealousGx:fix/local-variables-12588
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/node-core/src/integrations/local-variables/common.ts
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,12 @@ export interface LocalVariablesIntegrationOptions {
* Maximum number of exceptions to capture local variables for per second before rate limiting is triggered.
*/
maxExceptionsPerSecond?: number;
/**
* When true, local variables will be captured for all frames, including those that are not in_app.
*
* Defaults to `false`.
*/
includeOutOfAppFrames?: boolean;
}

export interface LocalVariablesWorkerArgs extends LocalVariablesIntegrationOptions {
Expand Down
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ export const localVariablesAsyncIntegration = defineIntegration(((
if (
// We need to have vars to add
frameLocalVariables.vars === undefined ||
// We're not interested in frames that are not in_app because the vars are not relevant
frame.in_app === false ||
// Only skip out-of-app frames if includeOutOfAppFrames is not true
(frame.in_app === false && integrationOptions.includeOutOfAppFrames !== true) ||
// The function names need to match
!functionNamesMatch(frame.function, frameLocalVariables.function)
) {
Expand Down
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type {
Variables,
} from './common';
import { createRateLimiter, functionNamesMatch } from './common';
import { localVariablesTestHelperMethods } from './test-helpers';

/** Creates a unique hash from stack frames */
export function hashFrames(frames: StackFrame[] | undefined): string | undefined {
Expand Down Expand Up @@ -268,8 +269,8 @@ const _localVariablesSyncIntegration = ((
if (
// We need to have vars to add
cachedFrameVariable.vars === undefined ||
// We're not interested in frames that are not in_app because the vars are not relevant
frameVariable.in_app === false ||
// Only skip out-of-app frames if includeOutOfAppFrames is not true
(frameVariable.in_app === false && options.includeOutOfAppFrames !== true) ||
// The function names need to match
!functionNamesMatch(frameVariable.function, cachedFrameVariable.function)
) {
Expand All @@ -288,135 +289,137 @@ const _localVariablesSyncIntegration = ((
return event;
}

return {
name: INTEGRATION_NAME,
async setupOnce() {
const client = getClient<NodeClient>();
const clientOptions = client?.getOptions();
const testHelperMethods = localVariablesTestHelperMethods(cachedFrames);

if (!clientOptions?.includeLocalVariables) {
return;
}
let setupPromise: Promise<void> | undefined;

// Only setup this integration if the Node version is >= v18
// https://github.com/getsentry/sentry-javascript/issues/7697
const unsupportedNodeVersion = NODE_MAJOR < 18;
async function setup(): Promise<void> {
const client = getClient<NodeClient>();
const clientOptions = client?.getOptions();

if (unsupportedNodeVersion) {
debug.log('The `LocalVariables` integration is only supported on Node >= v18.');
return;
}
if (!clientOptions?.includeLocalVariables) {
return;
}

if (await isDebuggerEnabled()) {
debug.warn('Local variables capture has been disabled because the debugger was already enabled');
return;
}
// Only setup this integration if the Node version is >= v18
// https://github.com/getsentry/sentry-javascript/issues/7697
const unsupportedNodeVersion = NODE_MAJOR < 18;

AsyncSession.create(sessionOverride).then(
session => {
function handlePaused(
stackParser: StackParser,
{ params: { reason, data, callFrames } }: InspectorNotification<PausedExceptionEvent>,
complete: () => void,
): void {
if (reason !== 'exception' && reason !== 'promiseRejection') {
complete();
return;
}

rateLimiter?.();

// data.description contains the original error.stack
const exceptionHash = hashFromStack(stackParser, data.description);

if (exceptionHash == undefined) {
complete();
return;
}

const { add, next } = createCallbackList<FrameVariables[]>(frames => {
cachedFrames.set(exceptionHash, frames);
complete();
});
if (unsupportedNodeVersion) {
debug.log('The `LocalVariables` integration is only supported on Node >= v18.');
return;
}

// Because we're queuing up and making all these calls synchronously, we can potentially overflow the stack
// For this reason we only attempt to get local variables for the first 5 frames
for (let i = 0; i < Math.min(callFrames.length, 5); i++) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const { scopeChain, functionName, this: obj } = callFrames[i]!;

const localScope = scopeChain.find(scope => scope.type === 'local');

// obj.className is undefined in ESM modules
const fn =
obj.className === 'global' || !obj.className ? functionName : `${obj.className}.${functionName}`;

if (localScope?.object.objectId === undefined) {
add(frames => {
frames[i] = { function: fn };
next(frames);
});
} else {
const id = localScope.object.objectId;
add(frames =>
session.getLocalVariables(id, vars => {
frames[i] = { function: fn, vars };
next(frames);
}),
);
}
}

next([]);
}
if (await isDebuggerEnabled()) {
debug.warn('Local variables capture has been disabled because the debugger was already enabled');
return;
}

try {
const session = await AsyncSession.create(sessionOverride);

const handlePaused = (
stackParser: StackParser,
{ params: { reason, data, callFrames } }: InspectorNotification<PausedExceptionEvent>,
complete: () => void,
): void => {
if (reason !== 'exception' && reason !== 'promiseRejection') {
complete();
return;
}

rateLimiter?.();

// data.description contains the original error.stack
const exceptionHash = hashFromStack(stackParser, data.description);

if (exceptionHash == undefined) {
complete();
return;
}

const captureAll = options.captureAllExceptions !== false;

session.configureAndConnect(
(ev, complete) =>
handlePaused(clientOptions.stackParser, ev as InspectorNotification<PausedExceptionEvent>, complete),
captureAll,
);

if (captureAll) {
const max = options.maxExceptionsPerSecond || 50;

rateLimiter = createRateLimiter(
max,
() => {
debug.log('Local variables rate-limit lifted.');
session.setPauseOnExceptions(true);
},
seconds => {
debug.log(
`Local variables rate-limit exceeded. Disabling capturing of caught exceptions for ${seconds} seconds.`,
);
session.setPauseOnExceptions(false);
},
const { add, next } = createCallbackList<FrameVariables[]>(frames => {
cachedFrames.set(exceptionHash, frames);
complete();
});

// Because we're queuing up and making all these calls synchronously, we can potentially overflow the stack
// For this reason we only attempt to get local variables for the first 5 frames
for (let i = 0; i < Math.min(callFrames.length, 5); i++) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const { scopeChain, functionName, this: obj } = callFrames[i]!;

const localScope = scopeChain.find(scope => scope.type === 'local');

// obj.className is undefined in ESM modules
const fn = obj.className === 'global' || !obj.className ? functionName : `${obj.className}.${functionName}`;

if (localScope?.object.objectId === undefined) {
add(frames => {
frames[i] = { function: fn };
next(frames);
});
} else {
const id = localScope.object.objectId;
add(frames =>
session.getLocalVariables(id, vars => {
frames[i] = { function: fn, vars };
next(frames);
}),
);
}
}

next([]);
};

const captureAll = options.captureAllExceptions !== false;

shouldProcessEvent = true;
},
error => {
debug.log('The `LocalVariables` integration failed to start.', error);
},
session.configureAndConnect(
(ev, complete) =>
handlePaused(clientOptions.stackParser, ev as InspectorNotification<PausedExceptionEvent>, complete),
captureAll,
);

if (captureAll) {
const max = options.maxExceptionsPerSecond || 50;

rateLimiter = createRateLimiter(
max,
() => {
debug.log('Local variables rate-limit lifted.');
session.setPauseOnExceptions(true);
},
seconds => {
debug.log(
`Local variables rate-limit exceeded. Disabling capturing of caught exceptions for ${seconds} seconds.`,
);
session.setPauseOnExceptions(false);
},
);
}

shouldProcessEvent = true;
} catch (error) {
debug.log('The `LocalVariables` integration failed to start.', error);
}
}

return {
name: INTEGRATION_NAME,
setupOnce() {
setupPromise = setup();
},
processEvent(event: Event): Event {
async processEvent(event: Event): Promise<Event> {
await setupPromise;

if (shouldProcessEvent) {
return addLocalVariablesToEvent(event);
}

return event;
},
// These are entirely for testing
_getCachedFramesCount(): number {
return cachedFrames.size;
},
_getFirstCachedFrame(): FrameVariables[] | undefined {
return cachedFrames.values()[0];
},
...testHelperMethods,
};
}) satisfies IntegrationFn;

Expand Down
View file Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// // TEST-ONLY: allow tests to access the cache

import type { LRUMap } from '@sentry/core';
import type { FrameVariables } from './common';
Copy link
Member

@AbhiPrasad AbhiPrasad Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer if we didn't do this (and not add unit tests). Instead I'd like us to add tests under our node integration tests to validate this: https://github.com/getsentry/sentry-javascript/tree/develop/dev-packages/node-integration-tests/suites/public-api/LocalVariables


/**
* Provides test helper methods for interacting with the local variables cache.
* These methods are intended for use in unit tests to inspect and manipulate
* the internal cache of frame variables used by the LocalVariables integration.
*
* @param cachedFrames - The LRUMap instance storing cached frame variables.
* @returns An object containing helper methods for cache inspection and mutation.
*/
export function localVariablesTestHelperMethods(cachedFrames: LRUMap<string, FrameVariables[]>): {
_getCachedFramesCount: () => number;
_getFirstCachedFrame: () => FrameVariables[] | undefined;
_setCachedFrame: (hash: string, frames: FrameVariables[]) => void;
} {
/**
* Returns the number of entries in the local variables cache.
*/
function _getCachedFramesCount(): number {
return cachedFrames.size;
}

/**
* Returns the first set of cached frame variables, or undefined if the cache is empty.
*/
function _getFirstCachedFrame(): FrameVariables[] | undefined {
return cachedFrames.values()[0];
}

/**
* Sets the cached frame variables for a given stack hash.
*
* @param hash - The stack hash to associate with the cached frames.
* @param frames - The frame variables to cache.
*/
function _setCachedFrame(hash: string, frames: FrameVariables[]): void {
cachedFrames.set(hash, frames);
}

return {
_getCachedFramesCount,
_getFirstCachedFrame,
_setCachedFrame,
};
}
Loading
Loading

AltStyle によって変換されたページ (->オリジナル) /