Skip to content
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

LocalVariables integration - re-thrown errors can completely mess up frame variables #13416

Closed
3 tasks done
Bruno-DaSilva opened this issue Aug 18, 2024 · 4 comments · Fixed by #13644
Closed
3 tasks done
Assignees
Labels
Package: node Issues related to the Sentry Node SDK

Comments

@Bruno-DaSilva
Copy link

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/node

SDK Version

8.26.0

Framework Version

Node v20.12.2

Link to Sentry event

No response

Reproduction Example/SDK Setup

SDK setup:

Sentry.init({
  dsn: process.env.SENTRY_DSN,
  includeLocalVariables: true,
});

Steps to Reproduce

  1. init sentry with the above config
  2. throw an exception
  3. catch the exception in another file or function (eg. a parent function)
  4. re-throw the error
  5. capture the re-thrown error

Expected Result

The error is reported to sentry with its ORIGINAL variables from where in was first thrown

Actual Result

The error is reported with only some frames with variables or no variables at all.

I dug into this - the error hash of the re-throw will match the error hash of the original error, and so the local variable frame state will overridden by the re-throw when the worker sends the re-throw frames. But then the re-throw location's stack and vars will not necessarily match the original frame's stack (and the re-throw's frame length may not match the error's frame) so it will effectively just null out the variables.

@github-actions github-actions bot added the Package: node Issues related to the Sentry Node SDK label Aug 18, 2024
@Bruno-DaSilva
Copy link
Author

I'm hoping with the digging I've done to root cause these it shouldn't require a high effort repro example -- but if it's needed I can create one (it'll just take me some time that I don't immediately have).

@Bruno-DaSilva
Copy link
Author

I've created a PR #13418 that addresses this behaviour in worker.ts, but I'll need a maintainer to take it from here.

@Lms24
Copy link
Member

Lms24 commented Aug 19, 2024

Hey @Bruno-DaSilva thanks for writing in and for opening a PR!

We'll look into the issue and review the PR next week as this week is Hackweek at Sentry (see #13421).

@AbhiPrasad
Copy link
Member

This was released with https://github.com/getsentry/sentry-javascript/releases/tag/8.31.0 - thanks @Bruno-DaSilva for reporting, and ty @timfish for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment