From 68a6e4ca2efaad55ee97ff20301b0d8ee5832307 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 10 Sep 2024 12:00:16 +0200 Subject: [PATCH 1/3] fix(node): Don't overwrite local variables for re-thrown errors --- .../node/src/integrations/local-variables/worker.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/node/src/integrations/local-variables/worker.ts b/packages/node/src/integrations/local-variables/worker.ts index eb4fee87947c..f0b3e20ce9b2 100644 --- a/packages/node/src/integrations/local-variables/worker.ts +++ b/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 { }, 1_000); } }, - _ => { - // ignore any errors + async _ => { + if (isPaused) { + await session.post('Debugger.resume'); + } }, ); }); From 39409c56b544c64535484759244533abba3c851c Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 10 Sep 2024 12:41:25 +0200 Subject: [PATCH 2/3] Add integration test --- .../LocalVariables/local-variables-rethrow.js | 45 +++++++++++++++++++ .../suites/public-api/LocalVariables/test.ts | 8 ++++ 2 files changed, 53 insertions(+) create mode 100644 dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-rethrow.js diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-rethrow.js b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-rethrow.js new file mode 100644 index 000000000000..5967445a95c3 --- /dev/null +++ b/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); diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts index 2640ecf94461..bf01ed999708 100644 --- a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts +++ b/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); }); From 166290215af2cc83d7ce0771e0d3feef0f201fe1 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 10 Sep 2024 13:16:10 +0200 Subject: [PATCH 3/3] Fix linting --- .../suites/public-api/LocalVariables/local-variables-rethrow.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-rethrow.js b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-rethrow.js index 5967445a95c3..744cb747c70c 100644 --- a/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-rethrow.js +++ b/dev-packages/node-integration-tests/suites/public-api/LocalVariables/local-variables-rethrow.js @@ -32,7 +32,7 @@ function one(name) { } setTimeout(() => { - try{ + try { try { one('some name'); } catch (e) {