fix(node): Don't overwrite local variables for re-thrown errors (#13644)

Another way to fix this issue would be to check via the debugger if the
property exists already on the error object and bail early before we
have local variables. However, this would add an extra round trip call
via the debugger for every error. Since re-thrown errors are far less
likely, I decided instead to just not overwrite existing local
variables.

This PR also adds a `Debugger.resume` call in the catch case to ensure
that they debugger will always resume even if we get errors while
debugging.

It's worth noting that this only fixes the issue in Node v20+ where we
use the async debugging interface.

Author: timfish

dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-rethrow.js

@@ -0,0 +1,45 @@
+/* eslint-disable no-unused-vars */
+const Sentry = require('@sentry/node');
+const { loggingTransport } = require('@sentry-internal/node-integration-tests');
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  includeLocalVariables: true,
+  transport: loggingTransport,
+});
+
+class Some {
+  two(name) {
+    throw new Error('Enough!');
+  }
+}
+
+function one(name) {
+  const arr = [1, '2', null];
+  const obj = {
+    name,
+    num: 5,
+  };
+  const bool = false;
+  const num = 0;
+  const str = '';
+  const something = undefined;
+  const somethingElse = null;
+
+  const ty = new Some();
+
+  ty.two(name);
+}
+
+setTimeout(() => {
+  try {
+    try {
+      one('some name');
+    } catch (e) {
+      const more = 'here';
+      throw e;
+    }
+  } catch (e) {
+    Sentry.captureException(e);
+  }
+}, 1000);

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

@@ -78,6 +78,14 @@ conditionalTest({ min: 18 })('LocalVariables integration', () => {
     });
   });
 
+  conditionalTest({ min: 20 })('Node v20+', () => {
+    test('Should retain original local variables when error is re-thrown', done => {
+      createRunner(__dirname, 'local-variables-rethrow.js')
+        .expect({ event: EXPECTED_LOCAL_VARIABLES_EVENT })
+        .start(done);
+    });
+  });
+
   test('Includes local variables for caught exceptions when enabled', done => {
     createRunner(__dirname, 'local-variables-caught.js').expect({ event: EXPECTED_LOCAL_VARIABLES_EVENT }).start(done);
   });

packages/node/src/integrations/local-variables/worker.ts

@@ -118,7 +118,9 @@ async function handlePaused(
   // We write the local variables to a property on the error object. These can be read by the integration as the error
   // event pass through the SDK event pipeline
   await session.post('Runtime.callFunctionOn', {
-    functionDeclaration: `function() { this.${LOCAL_VARIABLES_KEY} = ${JSON.stringify(frames)}; }`,
+    functionDeclaration: `function() { this.${LOCAL_VARIABLES_KEY} = this.${LOCAL_VARIABLES_KEY} || ${JSON.stringify(
+      frames,
+    )}; }`,
     silent: true,
     objectId,
   });
@@ -156,8 +158,10 @@ async function startDebugger(): Promise<void> {
           }, 1_000);
         }
       },
-      _ => {
-        // ignore any errors
+      async _ => {
+        if (isPaused) {
+          await session.post('Debugger.resume');
+        }
       },
     );
   });