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

includeLocalVariables fails to capture local variables across sync/async boundaries #11194

Closed
3 tasks done
andymccurdy opened this issue Mar 19, 2024 · 5 comments · Fixed by #11234
Closed
3 tasks done
Assignees
Labels
Package: node Issues related to the Sentry Node SDK Type: Bug

Comments

@andymccurdy
Copy link

andymccurdy commented Mar 19, 2024

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

7.103.0

Framework Version

No response

Link to Sentry event

No response

SDK Setup

    Sentry.init({
        dsn: process.env.SENTRY_DSN,
        enabled: true,
        environment: 'dev',
        includeLocalVariables: true,
        integrations: [
            Sentry.localVariablesIntegration({
              captureAllExceptions: true,
            }),
        ],
        release: version
    });

Steps to Reproduce

Full repro here: https://github.com/andymccurdy/sentry-node-koa-localvars

See README for instructions on installing and running.

I've tested on Node versions 18.19.1 and 20.11.1. Both versions produce identical behavior.

Expected Result

Local variables included in every stack frame

Actual Result

Local variables are only present on stack frames within the same async context where an error is thrown from. It is unclear if this is a symptom of #8928 or something different.

Sync Example:
sync

The request handler errorSync and the subsequent sync functions it calls (someSyncFunc, syncStepA and syncStepB) all include local variables. However, no middleware functions (Koa internal or user defined) have local variables as they are all async.

Async Example:
async

The request handler errorAsync awaits for the async function someAsyncFunc. Notice that someAsyncFunc, syncStepA and syncStepB all include local variables as they are all executed in the same tick. However no local variables are included on the errroAsync request handler as it was executed in a prior iteration of the event loop. As with the sync example, no middleware functions (Koa internal or user defined) have local variables.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 2 Mar 19, 2024
@github-actions github-actions bot added the Package: node Issues related to the Sentry Node SDK label Mar 19, 2024
@Lms24
Copy link
Member

Lms24 commented Mar 20, 2024

Hey @andymccurdy thanks for writing in!

@timfish when you have some time, would you mind taking a look? Thanks!

@timfish
Copy link
Collaborator

timfish commented Mar 21, 2024

We capture local variables for exceptions via the v8 debugging interface. When events are captured by the SDK we have to try and match the captured debugger stack frames to the Sentry frames which are parsed from error.stack.

It looks like the above issue is caused by the debugger stack frames not matching those parsed from error.stack. Notably, new Promise frames are missing entirely in the debugger call stack!

@andymccurdy
Copy link
Author

@timfish FWIW this isn't just an issue with new Promise(...). I'm seeing this same behavior on all async calls -- querying a database, fetching content from an HTTP service, etc. It seems like the SDK only captures locals for stack frames that were executed within the same event loop tick that threw the error.

The repro case used new Promise() purely for demonstration purposes... I didn't want to complicate matters by including a database or http client, though I can if it would be helpful.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 2 Mar 21, 2024
@timfish
Copy link
Collaborator

timfish commented Mar 21, 2024

It seems like the SDK only captures locals for stack frames that were executed within the same event loop tick that threw the error

In my testing, local variables were getting captured across async calls without issue, they just weren't all getting added to the frames in the Sentry event because the frames didn't match up exactly.

When you call into http or database libraries, they are likely also creating promises which would cause the issue I have fixed.

There could be other cases where the frames don't match which we haven't considered yet but we'd need to reproduce those too!

@AbhiPrasad
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: node Issues related to the Sentry Node SDK Type: Bug
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants