Isaacs/jealous gx fix local variables 12588

dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-out-of-app-default.js

@@ -0,0 +1,28 @@
+/* eslint-disable no-unused-vars */
+
+const Sentry = require('@sentry/node');
+const { loggingTransport } = require('@sentry-internal/node-integration-tests');
+
+const externalFunctionFile = require.resolve('./node_modules/out-of-app-function.js');
+
+const { out_of_app_function } = require(externalFunctionFile);
+
+function in_app_function() {
+  const inAppVar = 'in app value';
+  out_of_app_function(`${inAppVar} modified value`);
+}
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  transport: loggingTransport,
+  includeLocalVariables: true,
+});
+
+setTimeout(async () => {
+  try {
+    in_app_function();
+  } catch (e) {
+    Sentry.captureException(e);
+    await Sentry.flush();
+  }
+}, 500);

dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-out-of-app.js

@@ -0,0 +1,33 @@
+/* eslint-disable no-unused-vars */
+
+const Sentry = require('@sentry/node');
+const { loggingTransport } = require('@sentry-internal/node-integration-tests');
+
+const externalFunctionFile = require.resolve('./node_modules/out-of-app-function.js');
+
+const { out_of_app_function } = require(externalFunctionFile);
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  transport: loggingTransport,
+  includeLocalVariables: true,
+  integrations: [
+    Sentry.localVariablesIntegration({
+      includeOutOfAppFrames: true,
+    }),
+  ],
+});
+
+function in_app_function() {
+  const inAppVar = 'in app value';
+  out_of_app_function(`${inAppVar} modified value`);
+}
+
+setTimeout(async () => {
+  try {
+    in_app_function();
+  } catch (e) {
+    Sentry.captureException(e);
+    await Sentry.flush();
+  }
+}, 500);

dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts

@@ -1,5 +1,6 @@
+import { mkdirSync, rmdirSync, unlinkSync, writeFileSync } from 'fs';
 import * as path from 'path';
-import { afterAll, describe, expect, test } from 'vitest';
+import { afterAll, beforeAll, describe, expect, test } from 'vitest';
 import { conditionalTest } from '../../../utils';
 import { cleanupChildProcesses, createRunner } from '../../../utils/runner';
 
@@ -39,8 +40,35 @@ const EXPECTED_LOCAL_VARIABLES_EVENT = {
 };
 
 describe('LocalVariables integration', () => {
+  const nodeModules = `${__dirname}/node_modules`;
+  const externalModule = `${nodeModules}//out-of-app-function.js`;
+  function cleanupExternalModuleFile() {
+    try {
+      unlinkSync(externalModule);
+      // eslint-disable-next-line no-empty
+    } catch {}
+    try {
+      rmdirSync(nodeModules);
+      // eslint-disable-next-line no-empty
+    } catch {}
+  }
+
+  beforeAll(() => {
+    cleanupExternalModuleFile();
+    mkdirSync(nodeModules, { recursive: true });
+    writeFileSync(
+      externalModule,
+      `
+function out_of_app_function(passedArg) {
+  const outOfAppVar = "out of app value " + passedArg.substring(13);
+  throw new Error("out-of-app error");
+}
+module.exports = { out_of_app_function };`,
+    );
+  });
   afterAll(() => {
     cleanupChildProcesses();
+    cleanupExternalModuleFile();
   });
 
   test('Should not include local variables by default', async () => {
@@ -127,4 +155,47 @@ describe('LocalVariables integration', () => {
       .start()
       .completed();
   });
+
+  test('adds local variables to out of app frames when includeOutOfAppFrames is true', async () => {
+    await createRunner(__dirname, 'local-variables-out-of-app.js')
+      .expect({
+        event: event => {
+          const frames = event.exception?.values?.[0]?.stacktrace?.frames || [];
+
+          const inAppFrame = frames.find(frame => frame.function === 'in_app_function');
+          const outOfAppFrame = frames.find(frame => frame.function === 'out_of_app_function');
+
+          expect(inAppFrame?.vars).toEqual({ inAppVar: 'in app value' });
+          expect(inAppFrame?.in_app).toEqual(true);
+
+          expect(outOfAppFrame?.vars).toEqual({
+            outOfAppVar: 'out of app value modified value',
+            passedArg: 'in app value modified value',
+          });
+          expect(outOfAppFrame?.in_app).toEqual(false);
+        },
+      })
+      .start()
+      .completed();
+  });
+
+  test('does not add local variables to out of app frames by default', async () => {
+    await createRunner(__dirname, 'local-variables-out-of-app-default.js')
+      .expect({
+        event: event => {
+          const frames = event.exception?.values?.[0]?.stacktrace?.frames || [];
+
+          const inAppFrame = frames.find(frame => frame.function === 'in_app_function');
+          const outOfAppFrame = frames.find(frame => frame.function === 'out_of_app_function');
+
+          expect(inAppFrame?.vars).toEqual({ inAppVar: 'in app value' });
+          expect(inAppFrame?.in_app).toEqual(true);
+
+          expect(outOfAppFrame?.vars).toBeUndefined();
+          expect(outOfAppFrame?.in_app).toEqual(false);
+        },
+      })
+      .start()
+      .completed();
+  });
 });

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 {

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)
       ) {