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

Add ability to disable break on first line when using the CLI inspector #40857

Closed
Spakman opened this issue Nov 18, 2021 · 14 comments · Fixed by #40960
Closed

Add ability to disable break on first line when using the CLI inspector #40857

Spakman opened this issue Nov 18, 2021 · 14 comments · Fixed by #40960
Labels
debugger Issues and PRs related to the debugger subsystem. feature request Issues that request new features to be added to Node.js. inspector Issues and PRs related to the V8 inspector protocol

Comments

@Spakman
Copy link

Spakman commented Nov 18, 2021

Is your feature request related to a problem? Please describe.
I use Node.js to run unit tests in a (Linux) shell. The way this is set up to run is to evaluate each test script as a single invocation of Node.js in series. It's working great!

However, while I do have the ability to use a debugger; statement (when I invoke each test using node inspect a_test_file.js) in these scripts to use the built-in CLI inspector, because of the forced breakpoint on the first line, the debug shell is activated for every test script, which quickly becomes tedious. I'm looking to run something like node inspect a_test_file.js and only have it bring up the debug shell if/when a debugger; statement is executed.

Describe the solution you'd like
While I do understand the need to break on the first line in many other situations, some method to disable it would be very useful in this circumstance. An environment varible or a command line argument to achieve this would both work well for me.

Describe alternatives you've considered
I'm not aware of any way to work around this.

@targos targos added feature request Issues that request new features to be added to Node.js. inspector Issues and PRs related to the V8 inspector protocol labels Nov 18, 2021
@Trott
Copy link
Member

Trott commented Nov 19, 2021

@nodejs/diagnostics

@RafaelGSS
Copy link
Member

I'm generally +1 on this.

@Trott Trott added the debugger Issues and PRs related to the debugger subsystem. label Nov 19, 2021
@Trott
Copy link
Member

Trott commented Nov 20, 2021

I guess the obvious way to implement this would be as a command-line flag: node inspect --run index.js or node inspect --no-break-on-first-line index.js or something like that. Is there a better/preferable way?

@RafaelGSS
Copy link
Member

I'm thinking in disable by default the break on first line and land it as major. What do you think? @Trott

@Trott
Copy link
Member

Trott commented Nov 20, 2021

I'm thinking in disable by default the break on first line and land it as major. What do you think? @Trott

Does that conflict with the behavior most people would want and expect? Do CLI debuggers in general tend to break on or before the first executable statement by default? I'd be interested to know what the standard Python, Perl, and Ruby CLI debuggers do, for example.

@Trott
Copy link
Member

Trott commented Nov 20, 2021

python -m pdb script.py appears to break on the first line by default.

I suspect Perl and Ruby and interpreted/scripting languages in general do the same in their CLI debuggers, but correction is welcome.

I personally would find a different behavior (in a CLI debugger) surprising, but I'm certainly open to the idea if it's the right thing to do.

@targos
Copy link
Member

targos commented Nov 21, 2021

I would be -1 on making it the default. I don't think having debugger; statements in the code is the common use case.

@targos
Copy link
Member

targos commented Nov 21, 2021

Btw, I think we should still always break internally on first line, because otherwise there's a risk that the code with the breakpoint runs before we connect the inspector. An option to automatically continue after this initial pause SGTM.

@RafaelGSS
Copy link
Member

Btw, I think we should still always break internally on first line, because otherwise there's a risk that the code with the breakpoint runs before we connect the inspector. An option to automatically continue after this initial pause SGTM.

Yes, makes sense.

@Trott
Copy link
Member

Trott commented Nov 24, 2021

It looks like there is already an undocumented environment variable that does this. Set NODE_INSPECT_RESUME_ON_START=1 and it should not break on the first line.

@Spakman Can you test this and confirm that it works as you expect?

If so, perhaps the solution is to document the environment variable.

Trott added a commit to Trott/io.js that referenced this issue Nov 25, 2021
Trott added a commit to Trott/io.js that referenced this issue Nov 25, 2021
nodejs-github-bot pushed a commit that referenced this issue Nov 26, 2021
Closes: #40857

PR-URL: #40960
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Spakman
Copy link
Author

Spakman commented Nov 26, 2021

@Spakman Can you test this and confirm that it works as you expect?

Using this seems to get us part way there, but it's still not behaving in the way I'd expect.

Given this debugger.js:

const hello = 123;
console.log("Expect this to be displayed directly in STDOUT.");
debugger;
console.log("About to exit:", hello);

Running NODE_INSPECT_RESUME_ON_START=1 node inspect debugger.js correctly skips the first line, instead breaking on the debugger; statement. I'd expect the first console.log output to appear directly in STDOUT rather than after the debug shell's < (since the debugger; statement hasn't been executed yet). While I would find that behaviour cleaner, this isn't really the end of the world since if I'm wanting to step through code then I'm likely a bit less concerned about output correctness.

Here's the output from that command:

< Debugger listening on ws://127.0.0.1:9229/23c5ccc6-0db3-44bb-832d-66f09aa17b83
< For help, see: https://nodejs.org/en/docs/inspector
< 
connecting to 127.0.0.1:9229 ... ok
< Debugger attached.
< 
< Expect this to be displayed directly in STDOUT.
< 
break in debugger.js:3
  1 const hello = 123;
  2 console.log("Expect this to be displayed directly in STDOUT.");
> 3 debugger;
  4 console.log("About to exit:", hello);
  5 
debug> cont
< About to exit: 123
< 
< Waiting for the debugger to disconnect...
< 
debug> .exit

A bigger issue for me is that the debug shell is both brought up automatically without any debugger; statements (causing what I'd consider in this case to be malformed output) and doesn't terminate without user input. With this no_debugger.js, running NODE_INSPECT_RESUME_ON_START=1 node inspect no_debugger.js gives this output:

const hello = 123;
console.log("Expect this to be displayed directly in STDOUT.");
console.log("About to exit:", hello);

Here's the output:

< Debugger listening on ws://127.0.0.1:9229/bf6c2802-dfe0-422c-a1a5-7676ee21101e
< For help, see: https://nodejs.org/en/docs/inspector
< 
connecting to 127.0.0.1:9229 ... ok
< Debugger attached.
< 
< Expect this to be displayed directly in STDOUT.
< 
< About to exit: 123
< 
< Waiting for the debugger to disconnect...
< 
debug> .exit

In an ideal world, I'd expect running this no_debugger.js script to behave exactly as if I hadn't passed the inspect argument.

While it wouldn't solve any output issues, is there perhaps another environment variable that I'm missing to disconnect the debugger automatically when the script exits?

targos pushed a commit that referenced this issue Nov 26, 2021
Closes: #40857

PR-URL: #40960
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott
Copy link
Member

Trott commented Nov 26, 2021

Because the initial question was about "disable break on first line" and now we're talking about exiting the debugger automatically, I've opened a new issue for that in #40982.

@Spakman
Copy link
Author

Spakman commented Nov 26, 2021

Because the initial question was about "disable break on first line" and now we're talking about exiting the debugger automatically, I've opened a new issue for that in #40982.

My apologies. I was caught up in my own goals and had conflated somewhat unrelated issues. A new conversation makes perfect sense.

@Trott
Copy link
Member

Trott commented Nov 27, 2021

Because the initial question was about "disable break on first line" and now we're talking about exiting the debugger automatically, I've opened a new issue for that in #40982.

My apologies. I was caught up in my own goals and had conflated somewhat unrelated issues. A new conversation makes perfect sense.

Oh, no apology necessary. Scope creep in a conversation is pretty natural. Just trying to keep individual issues from having long conversations attached to them if they are not required.

danielleadams pushed a commit that referenced this issue Jan 30, 2022
Closes: #40857

PR-URL: #40960
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
danielleadams pushed a commit that referenced this issue Feb 1, 2022
Closes: #40857

PR-URL: #40960
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debugger Issues and PRs related to the debugger subsystem. feature request Issues that request new features to be added to Node.js. inspector Issues and PRs related to the V8 inspector protocol
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants