doc: match debugger output & instructions to master behavior #13885

jkrems · 2017-06-23T02:20:22Z

Follow-up to #13777. Updates debugger.md with instructions & output that match behavior in node 8 / master.

Previously the instructions would at best trigger deprecation warnings and at worst fail completely.

Checklist

documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

doc

vsemozhetbyt · 2017-06-23T04:49:47Z

doc/api/debugger.md

+$ node inspect myscript.js
+< Debugger listening on ws://127.0.0.1:9229/80e7a814-7cd3-49fb-921a-2e02228cd5ba
+< For help see https://nodejs.org/en/docs/inspector
+< Debugger attached.


I do not see this line in 8.1.2 output.

vsemozhetbyt · 2017-06-23T04:50:54Z

doc/api/debugger.md

  2 setTimeout(() => {
-  3   debugger;
+  3   console.log('world');


If you have not changed the code example below, this should still remain:

3 debugger;

Yeah, I wasn't sure about the exact "order" here. Because it first shows this and then later/below says (emphasis mine):

Inserting the statement debugger; into the source code of a script will
enable a breakpoint at that position in the code:

I interpreted that as "this is showing the original script run, then you insert debugger, then you run it again".

vsemozhetbyt · 2017-06-23T04:52:05Z

doc/api/debugger.md

-> 1 global.x = 5;
+< Debugger listening on ws://127.0.0.1:9229/80e7a814-7cd3-49fb-921a-2e02228cd5ba
+< For help see https://nodejs.org/en/docs/inspector
+< Debugger attached.


I do not see this line in 8.1.2 output.

Weird, I'm seeing it on master/this branch:

I also see it on 8.1.2:

I'm taking a TODO to check where the message is generated and why it wouldn't happen consistently. Might be a timing and/or platform issue..?

vsemozhetbyt · 2017-06-23T04:57:42Z

doc/api/debugger.md

@@ -69,14 +71,14 @@ Press Ctrl + C to leave debug repl
 > 2+2
 4
 debug> next
-break in /home/indutny/Code/git/indutny/myscript.js:5
+break in myscript.js:5


I see this order in 8.1.2:

debug> next < world break in myscript.js:5

Yep! Missed that one.

vsemozhetbyt · 2017-06-23T05:02:00Z

doc/api/debugger.md

@@ -41,22 +42,23 @@ Once the debugger is run, a breakpoint will occur at line 3:

 ```txt
 $ node debug myscript.js


It seems this also should be:

$ node inspect myscript.js

Good catch!

vsemozhetbyt · 2017-06-23T05:03:15Z

doc/api/debugger.md

+$ node inspect test/fixtures/break-in-module/main.js
+< Debugger listening on ws://127.0.0.1:9229/4e3db158-9791-4274-8909-914f7facf3bd
+< For help see https://nodejs.org/en/docs/inspector
+< Debugger attached.


I do not see this line in 8.1.2 output.

vsemozhetbyt · 2017-06-23T05:08:33Z

doc/api/debugger.md

+< For help see https://nodejs.org/en/docs/inspector
+< Debugger attached.
+Break on start in test/fixtures/break-in-module/main.js:1
+> 1 (function (exports, require, module, __filename, __dirname) { const mod = require('./mod.js');
  2 mod.hello();
  3 mod.hello();
 debug> setBreakpoint('mod.js', 2)


2 -> 22 (due to copyright added recently):

debug> setBreakpoint('mod.js', 22)

Interesting. I think the current instruction happened to work because it was smart enough to "shift" beyond the comment. But using 22 is definitely less confusing. 👍

vsemozhetbyt · 2017-06-23T05:10:00Z

doc/api/debugger.md

-  3 mod.hello();
-  4 debugger;
-  5
-  6 });
 debug> c
 break in test/fixtures/break-in-module/mod.js:2


2 -> 22:

break in test/fixtures/break-in-module/mod.js:22

vsemozhetbyt · 2017-06-23T05:16:41Z

Maybe we also should update the link to redirected one (line 194):
https://chromedevtools.github.io/debugger-protocol-viewer/
->
https://chromedevtools.github.io/devtools-protocol/

vsemozhetbyt · 2017-06-23T05:20:04Z

the last example may also be updated to:

$ node --inspect index.js
Debugger listening on ws://127.0.0.1:9229/dc9010dd-f8b8-4ac5-a510-c1a114ec7d29
For help see https://nodejs.org/en/docs/inspector

benjamingr

I'm LGTM if the nits raised by @vsemozhetbyt are attached - I see the same output as @vsemozhetbyt - without the "Debugger attached." line.

jkrems · 2017-06-23T15:31:19Z

I'm not sure what's up with the "Debugger attached". I'm testing on macOS 10.12.5 if that makes any difference..? Anyhow, the straight-up wrong stuff should be fixed now. :)

cjihrig

LGTM. If the output is different between master and Node 8, then this might need to be adjusted during backporting.

vsemozhetbyt · 2017-06-23T15:58:34Z

I cannot see "Debugger attached" in any recent version for Windows, including the master and canary:

screenshot:

vsemozhetbyt · 2017-06-23T16:14:29Z

There is a small glimpse on the start, so I supposed this line was just rewritten on Windows. I've run this with redirection to a file and even have recorded a video and watched it frame by frame. There are some rewritten lines, but still, no "Debugger attached" among them:

node.8.1.2.v8-5.8.20170615.exe inspect test.js > debug.log:

�< Debugger listening on ws://127.0.0.1:9229/9ab1ed89-f254-4415-97eb-c99d61555806
< For help see https://nodejs.org/en/docs/inspector
�connecting to 127.0.0.1:9229 ... okdebug> �Break on start in test.js:1
> 1 (function (exports, require, module, __filename, __dirname) { global.x = 5;
  2 setTimeout(() => {
  3   debugger;
debug>

jkrems · 2017-06-23T16:27:56Z

Interesting. Will investigate what's up on Windows. One possible answer could be stdout vs. stderr -

node/src/inspector_io.cc

Line 396 in 6e2c29b

fprintf(stderr, "Debugger attached.\n");

.

Given current information I suspect this is a bug (?) on Windows. The message should be printed.

jkrems · 2017-07-08T16:28:08Z

Landed in b647f04.

PR-URL: #13885 Reviewed-By: Colin Ihrig <[email protected]>

jkrems added the doc Issues and PRs related to the documentations. label Jun 23, 2017

nodejs-github-bot added debugger doc Issues and PRs related to the documentations. labels Jun 23, 2017

vsemozhetbyt reviewed Jun 23, 2017

View reviewed changes

benjamingr reviewed Jun 23, 2017

View reviewed changes

cjihrig approved these changes Jun 23, 2017

View reviewed changes

jkrems closed this Jul 8, 2017

jkrems force-pushed the jk-update-debugger-docs branch from 6496ca3 to b647f04 Compare July 8, 2017 16:27

jkrems merged commit b647f04 into nodejs:master Jul 8, 2017

jkrems deleted the jk-update-debugger-docs branch July 8, 2017 16:28

addaleax pushed a commit that referenced this pull request Jul 11, 2017

doc: match debugger output & instructions to master behavior

227d4b1

PR-URL: #13885 Reviewed-By: Colin Ihrig <[email protected]>

addaleax mentioned this pull request Jul 11, 2017

v8.2.0 proposal #13744

Merged

addaleax pushed a commit that referenced this pull request Jul 18, 2017

doc: match debugger output & instructions to master behavior

32ddb66

PR-URL: #13885 Reviewed-By: Colin Ihrig <[email protected]>

Fishrock123 pushed a commit that referenced this pull request Jul 19, 2017

doc: match debugger output & instructions to master behavior

c4b6059

PR-URL: #13885 Reviewed-By: Colin Ihrig <[email protected]>

MylesBorins added the dont-land-on-v6.x label Aug 16, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

doc: match debugger output & instructions to master behavior #13885

doc: match debugger output & instructions to master behavior #13885

jkrems commented Jun 23, 2017

vsemozhetbyt Jun 23, 2017 •

edited

Loading

vsemozhetbyt Jun 23, 2017 •

edited

Loading

jkrems Jun 23, 2017 •

edited

Loading

vsemozhetbyt Jun 23, 2017 •

edited

Loading

jkrems Jun 23, 2017

vsemozhetbyt Jun 23, 2017 •

edited

Loading

jkrems Jun 23, 2017

vsemozhetbyt Jun 23, 2017

jkrems Jun 23, 2017

vsemozhetbyt Jun 23, 2017

vsemozhetbyt Jun 23, 2017 •

edited

Loading

jkrems Jun 23, 2017

vsemozhetbyt Jun 23, 2017

vsemozhetbyt commented Jun 23, 2017

vsemozhetbyt commented Jun 23, 2017 •

edited

Loading

benjamingr left a comment

jkrems commented Jun 23, 2017

cjihrig left a comment

vsemozhetbyt commented Jun 23, 2017 •

edited

Loading

vsemozhetbyt commented Jun 23, 2017 •

edited

Loading

jkrems commented Jun 23, 2017

jkrems commented Jul 8, 2017

		@@ -41,22 +42,23 @@ Once the debugger is run, a breakpoint will occur at line 3:

		```txt
		$ node debug myscript.js

doc: match debugger output & instructions to master behavior #13885

doc: match debugger output & instructions to master behavior #13885

Conversation

jkrems commented Jun 23, 2017

Checklist

Affected core subsystem(s)

vsemozhetbyt Jun 23, 2017 • edited Loading

Choose a reason for hiding this comment

vsemozhetbyt Jun 23, 2017 • edited Loading

Choose a reason for hiding this comment

jkrems Jun 23, 2017 • edited Loading

Choose a reason for hiding this comment

vsemozhetbyt Jun 23, 2017 • edited Loading

Choose a reason for hiding this comment

jkrems Jun 23, 2017

Choose a reason for hiding this comment

vsemozhetbyt Jun 23, 2017 • edited Loading

Choose a reason for hiding this comment

jkrems Jun 23, 2017

Choose a reason for hiding this comment

vsemozhetbyt Jun 23, 2017

Choose a reason for hiding this comment

jkrems Jun 23, 2017

Choose a reason for hiding this comment

vsemozhetbyt Jun 23, 2017

Choose a reason for hiding this comment

vsemozhetbyt Jun 23, 2017 • edited Loading

Choose a reason for hiding this comment

jkrems Jun 23, 2017

Choose a reason for hiding this comment

vsemozhetbyt Jun 23, 2017

Choose a reason for hiding this comment

vsemozhetbyt commented Jun 23, 2017

vsemozhetbyt commented Jun 23, 2017 • edited Loading

benjamingr left a comment

Choose a reason for hiding this comment

jkrems commented Jun 23, 2017

cjihrig left a comment

Choose a reason for hiding this comment

vsemozhetbyt commented Jun 23, 2017 • edited Loading

vsemozhetbyt commented Jun 23, 2017 • edited Loading

jkrems commented Jun 23, 2017

jkrems commented Jul 8, 2017

vsemozhetbyt Jun 23, 2017 •

edited

Loading

vsemozhetbyt Jun 23, 2017 •

edited

Loading

jkrems Jun 23, 2017 •

edited

Loading

vsemozhetbyt Jun 23, 2017 •

edited

Loading

vsemozhetbyt Jun 23, 2017 •

edited

Loading

vsemozhetbyt Jun 23, 2017 •

edited

Loading

vsemozhetbyt commented Jun 23, 2017 •

edited

Loading

vsemozhetbyt commented Jun 23, 2017 •

edited

Loading

vsemozhetbyt commented Jun 23, 2017 •

edited

Loading