inspector: migrate errors from C++ to JS #19387
Conversation
targos
added
inspector
nodejs-github-bot
added
c++
targos
added
the
semver-major
doc/api/errors.md
Outdated
fhinkel
left a comment
fhinkel
left a comment
There was a problem hiding this comment.
LGTM with typo pointed out by @richardlau.
targos
force-pushed
the
error-codes-inspector
branch
from
2faae1f to
38a566e
Compare
doc/api/errors.md
Outdated
There was a problem hiding this comment.
Should this reuse the ERR_INSPECTOR_ALREADY_CONNECTED instead of creating a new error? From what I can tell, the only distinction is that if you connect from the same session twice, you get an ERR_INSPECTOR_ALREADY_CONNECTED, whereas if you connect from another session, you get an ERR_INSPECTOR_ALREADY_ATTACHED, but the cause are the same: there is a current connected session, whether it's the same one or a different one does not seem to make a big difference.
To get out of ERR_INSPECTOR_ALREADY_CONNECTED, the user can call.disconnect(), but there is not a method called .detach() that a user can use to get out of ERR_INSPECTOR_ALREADY_ATTACHED, they still need to .disconnect() to detach - and there is no explanation about attach in the inspector documentation. IMO the difference should probably be told in the error message, not in the code, so it's [ERR_INSPECTOR_ALREADY_CONNECTED]: the inspector session is already connected v.s. [ERR_INSPECTOR_ALREADY_CONNECTED]: another inspector session is already connected
There was a problem hiding this comment.
Good point. I'm going to change this.
Assign a code to a user-facing error. Turn other internal-only errors to checks.
targos
force-pushed
the
error-codes-inspector
branch
from
38a566e to
c08f887
Compare
|
Updated. New CI: https://ci.nodejs.org/job/node-test-pull-request/13796/ |
|
CI is green. Is everyone OK with @joyeecheung's suggestion? |
|
Landed in 4e1f090 |
Assign a code to a user-facing error. Turn other internal-only errors to checks. PR-URL: #19387 Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
8 participants
Assign a code to a user-facing error.
Turn other internal-only errors to checks.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes