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 - debugger no longer works when enabled #13414

Open
3 tasks done
Bruno-DaSilva opened this issue Aug 17, 2024 · 10 comments
Open
3 tasks done

LocalVariables integration - debugger no longer works when enabled #13414

Bruno-DaSilva opened this issue Aug 17, 2024 · 10 comments
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

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

Steps to Reproduce

  1. start the nodejs app with --inspect
  2. init sentry with includeLocalVariables: true
  3. connect a debugger to the process
  4. trigger a breakpoint

Expected Result

Process is paused, and can step into/over/resume the execution

Actual Result

Process appears to pause for a millisecond and then immediately resumed.
I believe this is because the LocalVariablesAsync integration will resume all breakpoints immediately.

This does not appear to be documented anywhere. Is this expected behaviour?

@github-actions github-actions bot added the Package: node Issues related to the Sentry Node SDK label Aug 17, 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).

@Lms24
Copy link
Member

Lms24 commented Aug 19, 2024

Hey @Bruno-DaSilva thanks for writing in!

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

@lforst
Copy link
Member

lforst commented Aug 26, 2024

@Bruno-DaSilva your assumption is correct in that the integration immediately continues execution when a breakpoint is hit.

@timfish
Copy link
Collaborator

timfish commented Aug 27, 2024

explore whether we can avoid adding or enabling the integration when another inspector session is attached to the process.

I think the main issue is that this would only work when using --inspect-brk, where the inspector is already attached when the integration is started.

When the Chrome debugger is attached, it auto sends some commands which we might be able to detect from the responses that are sent to all connected clients.

@lforst
Copy link
Member

lforst commented Aug 27, 2024

I wonder if we could listen for new sessions to connect in any way and then dynamically disable the integration. 🤔

@timfish
Copy link
Collaborator

timfish commented Aug 27, 2024

It's likely possible, I just can't find a specific debugger event for new debug connections. We'd have to watch for responses that indicate a new connection has been made and these are specific responses to what the debugger sends on connection. This can vary slightly depending on how the debugger is configured.

@timfish timfish self-assigned this Aug 27, 2024
@Bruno-DaSilva
Copy link
Author

In the worst case, could we have some sort of manual trigger to enable/disable the LocalVariables integration on the fly?

In a non-local workflow we perform some extra manual steps when we are eg. connecting to debug a pod in kubernetes (namely kill -SIGUSR1 {pid} to enable debugging - see this). So perhaps something similar could be used to disable the LocalVariables integration?

@timfish
Copy link
Collaborator

timfish commented Aug 27, 2024

We could check process.execArgv for --inspect which would cover a lot of cases. It's still possible to enable the debugger via thge inspector api but I suspect this is rarely used for manual debugging.

@Bruno-DaSilva
Copy link
Author

Bruno-DaSilva commented Aug 27, 2024

We could check process.execArgv for --inspect which would cover a lot of cases.

You mean disable the integration outright at startup if it sees the --inspect flag?

@timfish
Copy link
Collaborator

timfish commented Aug 27, 2024

disable the integration outright at startup if it sees the --inspect flag?

Yep, that would be the easiest solution.

There is a Debugger.detached event but unfortunately nothing for attached. When a new debugger connects we see repeated Debugger.scriptParsed events so we could also use those.

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
Projects
Status: No status
Development

No branches or pull requests

4 participants