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.

Sign up for GitHub

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

Jump to bottom

Error when restarting a node inspect run after setting break point twice #41789

Open
RaisinTen opened this issue Feb 27, 2021 · 4 comments
Open

Error when restarting a node inspect run after setting break point twice #41789

RaisinTen opened this issue Feb 27, 2021 · 4 comments
Labels
debugger Issues and PRs related to the debugger subsystem.

Comments

@RaisinTen
Copy link
Contributor 

❯ node -v
v15.2.0
❯ npm -v
7.0.8
❯ cat test.js
process.stdin.resume();
❯ ./node_modules/node-inspect/cli.js test.js
< Debugger listening on ws://127.0.0.1:9229/da6a93a7-27a4-4eb0-9abd-da7bf4d7e812
< For help, see: https://nodejs.org/en/docs/inspector
< Debugger attached.
Break on start in test.js:1
> 1 process.stdin.resume();
  2 
debug> setBreakpoint(1)
> 1 process.stdin.resume();
  2 
debug> restart
< Debugger listening on ws://127.0.0.1:9229/bd969e56-b38e-4c0e-a202-e99cbc34dbd2
< For help, see: https://nodejs.org/en/docs/inspector
< Debugger attached.
Warning: script 'file:///home/raisinten/Desktop/universe/temp/project/report/report2/test.js' was not loaded yet.
1 breakpoints restored.
Break on start in test.js:1
> 1 process.stdin.resume();
  2 
debug> setBreakpoint(1)
> 1 process.stdin.resume();
  2 
debug> restart
< Debugger listening on ws://127.0.0.1:9229/3f518903-5194-437a-ac6d-5f68e9186f6e
< For help, see: https://nodejs.org/en/docs/inspector
< Debugger attached.
Warning: script 'file:///home/raisinten/Desktop/universe/temp/project/report/report2/test.js' was not loaded yet.
There was an internal error in node-inspect. Please report this bug.
Breakpoint at specified location already exists. - undefined
Error: Breakpoint at specified location already exists. - undefined
    at _pending.<computed> (/home/raisinten/Desktop/universe/temp/project/report/report2/node_modules/node-inspect/lib/internal/inspect_client.js:243:27)
    at Client._handleChunk (/home/raisinten/Desktop/universe/temp/project/report/report2/node_modules/node-inspect/lib/internal/inspect_client.js:213:11)
    at Socket.emit (node:events:329:20)
    at Socket.EventEmitter.emit (node:domain:529:15)
    at addChunk (node:internal/streams/readable:304:12)
    at readableAddChunk (node:internal/streams/readable:279:9)
    at Socket.Readable.push (node:internal/streams/readable:218:10)
    at TCP.onStreamRead (node:internal/stream_base_commons:192:23)
    at TCP.callbackTrampoline (node:internal/async_hooks:129:14)

I would expect it to not crash and work like the first iteration of setBreakpoint + restart.
@Trott
Copy link
Member

Trott commented Jul 29, 2021

I'm able to replicate this in Node.js 15. In Node.js 16, this still produces the "Breakpoint at specified location already exists." error but does not crash. I'm going to close this, although I want to investigate what that "undefined" is all about after the error message.

@Trott Trott closed this as completed Jul 29, 2021
Trott referenced this issue in Trott/io.js Jul 29, 2021
@Trott
test: add test-debugger-breakpint-exists
e69ec73 
In Node.js 15, calling `setBreakpoint(1)` and `restart` twice in a row
caused the debugger to exit. In Node.js 16, it no longer exits but
throws an error that is expected, or at least reasonable, or at least
better than exiting.

The error message previously had `undefined` appended to it. It no
longer does.

This adds test coverage to `unpackError()` in
`lib/internal/debugger/inspect_client.js`. That function previously was
untested.

Refs: https://github.com/nodejs/node-inspect/issues/101
Trott referenced this issue in Trott/io.js Jul 29, 2021
@Trott
test: add test-debugger-breakpint-exists
2e04b73 
In Node.js 15, calling `setBreakpoint(1)` and `restart` twice in a row
caused the debugger to exit. In Node.js 16, it no longer exits but
throws an error that is expected, or at least reasonable, or at least
better than exiting.

The error message previously had `undefined` appended to it. It no
longer does.

This adds test coverage to `unpackError()` in
`lib/internal/debugger/inspect_client.js`. That function previously was
untested.

Refs: https://github.com/nodejs/node-inspect/issues/101
@Trott Trott mentioned this issue Jul 29, 2021
Closed
Trott referenced this issue in Trott/io.js Jul 29, 2021
@Trott
test: add test-debugger-breakpint-exists
9cd7913 
In Node.js 15, calling `setBreakpoint(1)` and `restart` twice in a row
caused the debugger to exit. In Node.js 16, it no longer exits but
throws an error that is expected, or at least reasonable, or at least
better than exiting.

The error message previously had `undefined` appended to it. It no
longer does.

This adds test coverage to `unpackError()` in
`lib/internal/debugger/inspect_client.js`. That function previously was
untested.

Refs: https://github.com/nodejs/node-inspect/issues/101
@Trott
Copy link
Member

Trott commented Jul 29, 2021

Looking more closely, this is still a bug.

@Trott Trott reopened this Jul 29, 2021
@Trott
Copy link
Member

Trott commented Jul 29, 2021

Looking more closely, this is still a bug.

Specifically, the error should happen at the second setBreakpoint() and not after restart().

@Trott
Copy link
Member

Trott commented Jul 29, 2021

I think is is a bug in the inspector code and not in the debugger/node-inspect code.

nodejs-github-bot referenced this issue Aug 1, 2021
@Trott @nodejs-github-bot
debugger: remove undefined parameter
1c6f26d 
The data parameter of unpackError() is typically undefined.

PR-URL: #39570
Refs: https://github.com/nodejs/node-inspect/issues/101
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
nodejs-github-bot referenced this issue Aug 1, 2021
@Trott @nodejs-github-bot
test: add test-debugger-breakpoint-exists
42d3144 
This adds test coverage to `unpackError()` in
`lib/internal/debugger/inspect_client.js`. That function previously was
untested.

PR-URL: #39570
Refs: https://github.com/nodejs/node-inspect/issues/101
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
danielleadams referenced this issue Aug 16, 2021
@Trott @danielleadams
debugger: remove undefined parameter
6c375e1 
The data parameter of unpackError() is typically undefined.

PR-URL: #39570
Refs: https://github.com/nodejs/node-inspect/issues/101
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
danielleadams referenced this issue Aug 16, 2021
@Trott @danielleadams
test: add test-debugger-breakpoint-exists
635e1a0 
This adds test coverage to `unpackError()` in
`lib/internal/debugger/inspect_client.js`. That function previously was
untested.

PR-URL: #39570
Refs: https://github.com/nodejs/node-inspect/issues/101
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
targos referenced this issue Sep 4, 2021
@Trott @targos
debugger: remove undefined parameter
f46e8cd 
The data parameter of unpackError() is typically undefined.

PR-URL: #39570
Refs: https://github.com/nodejs/node-inspect/issues/101
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
targos referenced this issue Sep 4, 2021
@Trott @targos
test: add test-debugger-breakpoint-exists
78db43c 
This adds test coverage to `unpackError()` in
`lib/internal/debugger/inspect_client.js`. That function previously was
untested.

PR-URL: #39570
Refs: https://github.com/nodejs/node-inspect/issues/101
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@jkrems jkrems changed the title Error when restarting after setting break point twice Error when restarting a node inspect run after setting break point twice Jan 31, 2022
@jkrems jkrems transferred this issue from nodejs/node-inspect Jan 31, 2022
@jkrems jkrems added the debugger Issues and PRs related to the debugger subsystem. label Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
debugger Issues and PRs related to the debugger subsystem.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jkrems @Trott @RaisinTen