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

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

Open
RaisinTen opened this issue Feb 27, 2021 · 4 comments
Open
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
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
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
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
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
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
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
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
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
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
Labels
debugger Issues and PRs related to the debugger subsystem.
Projects
None yet
Development

No branches or pull requests

3 participants