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

JealousGx · JealousGx · commit d92b0a06a5cb · 2025-09-06T10:44:26.000+05:00
This commit addresses an issue where local variables were not being captured for out-of-app frames, even when the `includeOutOfAppFrames` option was enabled.

The `localVariablesSyncIntegration` had a race condition where it would process events before the debugger session was fully initialized. This was fixed by awaiting the session creation in `setupOnce`.

The tests for this integration were failing because they were not setting up a Sentry client, which is required for the integration to be enabled. This has been corrected by adding a client to the test setup.

Additionally, this commit adds tests for the `localVariablesAsyncIntegration` to ensure it correctly handles the `includeOutOfAppFrames` option.
diff --git a/packages/node-core/src/integrations/local-variables/common.ts b/packages/node-core/src/integrations/local-variables/common.ts
@@ -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 {
diff --git a/packages/node-core/src/integrations/local-variables/local-variables-async.ts b/packages/node-core/src/integrations/local-variables/local-variables-async.ts
@@ -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)
       ) {
diff --git a/packages/node-core/src/integrations/local-variables/local-variables-sync.ts b/packages/node-core/src/integrations/local-variables/local-variables-sync.ts
@@ -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 {
@@ -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 && !options.includeOutOfAppFrames) ||
         // The function names need to match
         !functionNamesMatch(frameVariable.function, cachedFrameVariable.function)
       ) {
@@ -288,6 +289,8 @@ const _localVariablesSyncIntegration = ((
     return event;
   }
 
+  const testHelperMethods = localVariablesTestHelperMethods(cachedFrames);
+
   return {
     name: INTEGRATION_NAME,
     async setupOnce() {
@@ -312,96 +315,95 @@ const _localVariablesSyncIntegration = ((
         return;
       }
 
-      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();
-            });
+      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;
+          }
 
-            // 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]!;
+          rateLimiter?.();
 
-              const localScope = scopeChain.find(scope => scope.type === 'local');
+          // data.description contains the original error.stack
+          const exceptionHash = hashFromStack(stackParser, data.description);
 
-              // obj.className is undefined in ESM modules
-              const fn =
-                obj.className === 'global' || !obj.className ? functionName : `${obj.className}.${functionName}`;
+          if (exceptionHash == undefined) {
+            complete();
+            return;
+          }
 
-              if (localScope?.object.objectId === undefined) {
-                add(frames => {
-                  frames[i] = { function: fn };
+          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);
-                });
-              } 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;
+          next([]);
+        }
 
-          session.configureAndConnect(
-            (ev, complete) =>
-              handlePaused(clientOptions.stackParser, ev as InspectorNotification<PausedExceptionEvent>, complete),
-            captureAll,
+        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);
+            },
           );
+        }
 
-          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;
-        },
-        error => {
-          debug.log('The `LocalVariables` integration failed to start.', error);
-        },
-      );
+        shouldProcessEvent = true;
+      } catch (error) {
+        debug.log('The `LocalVariables` integration failed to start.', error);
+      }
     },
     processEvent(event: Event): Event {
       if (shouldProcessEvent) {
@@ -410,13 +412,7 @@ const _localVariablesSyncIntegration = ((
 
       return event;
     },
-    // These are entirely for testing
-    _getCachedFramesCount(): number {
-      return cachedFrames.size;
-    },
-    _getFirstCachedFrame(): FrameVariables[] | undefined {
-      return cachedFrames.values()[0];
-    },
+    ...testHelperMethods,
   };
 }) satisfies IntegrationFn;
 
diff --git a/packages/node-core/src/integrations/local-variables/test-helpers.ts b/packages/node-core/src/integrations/local-variables/test-helpers.ts
@@ -0,0 +1,48 @@
+// // TEST-ONLY: allow tests to access the cache
+
+import type { LRUMap } from '@sentry/core';
+import type { FrameVariables } from './common';
+
+/**
+ * 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,
+  };
+}
diff --git a/packages/node-core/test/integrations/local-variables-async.test.ts b/packages/node-core/test/integrations/local-variables-async.test.ts
@@ -0,0 +1,73 @@
+import type { Event, EventHint, StackFrame } from '@sentry/core';
+import { beforeEach, describe, expect, it } from 'vitest';
+import { getCurrentScope, NodeClient } from '../../src';
+import type { FrameVariables } from '../../src/integrations/local-variables/common';
+import { LOCAL_VARIABLES_KEY } from '../../src/integrations/local-variables/common';
+import { localVariablesAsyncIntegration } from '../../src/integrations/local-variables/local-variables-async';
+import { getDefaultNodeClientOptions } from '../helpers/getDefaultNodeClientOptions';
+
+describe('LocalVariablesAsync', () => {
+  beforeEach(() => {
+    const options = getDefaultNodeClientOptions({
+      includeLocalVariables: true,
+      dsn: 'https://public@dsn.ingest.sentry.io/1337',
+    });
+    const client = new NodeClient(options);
+    getCurrentScope().setClient(client);
+  });
+
+  it('does not add local variables to out of app frames by default', async () => {
+    const eventName = 'test-exclude-LocalVariables-out-of-app-frames';
+    const event = getTestEvent(eventName);
+    const integration = localVariablesAsyncIntegration({});
+    await integration.setup?.(getCurrentScope().getClient<NodeClient>()!);
+
+    const hint: EventHint = {
+      originalException: {
+        [LOCAL_VARIABLES_KEY]: [{ function: eventName, vars: { foo: 'bar' } } as FrameVariables],
+      },
+    };
+
+    const processedEvent = integration.processEvent?.(event, hint, getCurrentScope().getClient<NodeClient>()!) as Event;
+
+    expect(processedEvent.exception?.values?.[0]?.stacktrace?.frames?.[0]?.vars).toBeUndefined();
+  });
+
+  it('adds local variables to out of app frames when includeOutOfAppFrames is true', async () => {
+    const eventName = 'test-include-LocalVariables-out-of-app-frames';
+    const event = getTestEvent(eventName);
+    const integration = localVariablesAsyncIntegration({ includeOutOfAppFrames: true });
+    await integration.setup?.(getCurrentScope().getClient<NodeClient>()!);
+
+    const hint: EventHint = {
+      originalException: {
+        [LOCAL_VARIABLES_KEY]: [{ function: eventName, vars: { foo: 'bar' } } as FrameVariables],
+      },
+    };
+
+    const processedEvent = integration.processEvent?.(event, hint, getCurrentScope().getClient<NodeClient>()!) as Event;
+
+    expect(processedEvent.exception?.values?.[0]?.stacktrace?.frames?.[0]?.vars).toEqual({ foo: 'bar' });
+  });
+});
+
+function getTestEvent(fnName = 'test'): Event {
+  return {
+    exception: {
+      values: [
+        {
+          stacktrace: {
+            frames: [
+              {
+                in_app: false,
+                function: fnName,
+                lineno: 1,
+                colno: 1,
+              } as StackFrame,
+            ],
+          },
+        },
+      ],
+    },
+  };
+}
diff --git a/packages/node-core/test/integrations/local-variables.test.ts b/packages/node-core/test/integrations/local-variables.test.ts
