debugger, errors: migrate to use internal/errors.js #11380
Conversation
Use the new error code defined in internal/errors for _debugger.js
nodejs-github-bot
added
debugger
errors
doc/api/errors.md
Outdated
| ## Node.js Error Codes | ||
|
|
||
| <a id="ERR_UNK_STATE"></a> | ||
| ### ERR_INVALID_CALLBACK |
There was a problem hiding this comment.
Code name doesn't match the anchor here. Also probably spelling the UNKNOWN in full would be easier to understand? The description below is not particularly helpful if someone encounters this error and tries to look it up, probably ERR_UNKNOWN_DEBUGGER_STATE and some more explanation would be better.
There was a problem hiding this comment.
Sorry about that. I've fixed it in the latest commit. I'm not sure what more can I add to the explanation. Could you please guide me a little about that? Thanks!
jasnell
added
the
semver-major
Add more explanation for the error code and replace it with a more verbose alternative
doc/api/errors.md
Outdated
| <a id="ERR_UNKNOWN_DEBUGGER_STATE"></a> | ||
| ### ERR_UNKNOWN_DEBUGGER_STATE | ||
|
|
||
| The `'ERR_UNKNOWN_DEBUGGER_STATE'` error code is used to identify that the state of the debugger is not known. |
There was a problem hiding this comment.
@shubheksha According to the history I think the test of this comes from #8454 and the code dates back to 8d82ec2, this looks like just a safety net in case anyone accidentally set the .state from outside and break the state machine, so probably we can add a sentence at the end: ...not known, possibly set by code outside the debugger module.
On another note, debugger module is deprecated and is going to be replaced by node inspect, so this error probably would not stay that long in the code base. Thanks for the contribution anyway!
There was a problem hiding this comment.
Also can you add a line break in the sentence so it wraps in ~80 characters? Thanks!
- Added more documentation about ERR_UNKNOWN_DEBUGGER_STATE error code
|
@joyeecheung, fixed it! Let me know if any other changes are needed. Thanks! |
|
Still LGTM but I think this would need a review from @jasnell, and maybe more eyeballs :) |
|
@shubheksha, sorry for not informing you of this part of our plan beforehand, and thank you for contributing! There are still many files to migrate in #11273, so please feel free to take up some of them. |
Fixes #11273
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
debugger, errors, test