-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix(node): Don't overwrite local variables for re-thrown errors #13644
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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://[email protected]/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); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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, | ||
)}; }`, | ||
Comment on lines
+121
to
+123
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shame it doesn't appear to work now I'm adding a test for it! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah no, I forgot the recent changes only fix this issue in Node v20+ 🤦🏻♂️ |
||
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'); | ||
} | ||
Comment on lines
+161
to
+164
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good idea to add some more resiliency. |
||
}, | ||
); | ||
}); | ||
|
Uh oh!
There was an error while loading. Please reload this page.