Local variables missing when exception originates in node_modules dependency

andy-tazxap commented 2 weeks ago

Is there an existing issue for this?

[X] I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
[X] I have reviewed the documentation https://docs.sentry.io/
[X] I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/browser

SDK Version

7.117.0

Framework Version

@sentry/node

Link to Sentry event

No response

SDK Setup

    Sentry.init({
        dsn: SENTRY_DSN,
        integrations: [
            new Sentry.Integrations.Http({ tracing: true }),
            new Sentry.Integrations.Express({ app }),
            new ProfilingIntegration(),
        ],
        environment: ENV,
        tracesSampleRate: 1.0,
        profilesSampleRate: 1.0,
        includeLocalVariables: true,
    });

Steps to Reproduce

We have noticed a number of events in production missing local variables even though local variables are present in other events. We have noticed that it appears local variables are missing when the exception originates in dependency code, as opposed to our code.

Expected Result

Local variables to be present regardless of where the exception originates.

Actual Result

We have noticed a number of events in production missing local variables even though local variables are present in other events. We have noticed that it appears local variables are missing when the exception originates in dependency code, as opposed to our code.

AbhiPrasad commented 2 weeks ago

Hey @andy-tazxap thanks for writing in! The current local variables implementation only grabs local variables from in_app frames (errors from direct app code)

https://github.com/getsentry/sentry-javascript/blob/dfa863a3601771eb5a8ae858b31e79c708f3a652/packages/node/src/integrations/local-variables/local-variables-async.ts#L60

We can consider exposing an opt-in option to attach variables to any kind of frame. Would you be interested in making a PR for this change?

andy-tazxap commented 2 weeks ago

Hey @AbhiPrasad thanks for getting back to me! Ahh that explains it, is that specified in the docs anywhere, just wondering if I was reading the wrong thing?

Yeah maybe making this opt-in would make sense me, I could definitely make the PR but I probably won't have the chance to do this for a month or so. I'll update here when I have it ready, if anyone needs this feature more urgently feel free to create the PR ahead of me!

AbhiPrasad commented 2 weeks ago

is that specified in the docs anywhere

This is something we missed, I added a PR to update the docs accordingly: https://github.com/getsentry/sentry-docs/pull/10461

Thanks for letting us know about this!

andy-tazxap commented 2 weeks ago

No worries, thanks for the speedy response here!

getsentry / sentry-javascript