sentry-javascript icon indicating copy to clipboard operation
sentry-javascript copied to clipboard

LocalVariables integration - debugger no longer works when enabled

Open Bruno-DaSilva opened this issue 1 year ago • 10 comments

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/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?

Bruno-DaSilva avatar Aug 17 '24 23:08 Bruno-DaSilva

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 avatar Aug 18 '24 00:08 Bruno-DaSilva

Hey @Bruno-DaSilva thanks for writing in!

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

Lms24 avatar Aug 19 '24 09:08 Lms24

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

  • [x] First thing we should definitely do is adjust the docs to mention this. (https://github.com/getsentry/sentry-docs/pull/11138)
  • [ ] Next thing is maybe to explore whether we can avoid adding or enabling the integration when another inspector session is attached to the process.

lforst avatar Aug 26 '24 07:08 lforst

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.

timfish avatar Aug 27 '24 10:08 timfish

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

lforst avatar Aug 27 '24 10:08 lforst

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 avatar Aug 27 '24 11:08 timfish

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?

Bruno-DaSilva avatar Aug 27 '24 15:08 Bruno-DaSilva

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.

timfish avatar Aug 27 '24 16:08 timfish

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?

Bruno-DaSilva avatar Aug 27 '24 16:08 Bruno-DaSilva

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.

timfish avatar Aug 27 '24 17:08 timfish

A PR closing this issue has just been released 🚀

This issue was referenced by PR #14643, which was included in the 8.45.0 release.

github-actions[bot] avatar Dec 13 '24 11:12 github-actions[bot]