Repository navigation
Tracking Issue: Migrate errors to internal/errors.js #11273
Description
Activity
Am I wrong in thinking this could be a "good first contribution"? Would love to give it a try in that case.
Also, I'm assuming*/lib.jsis a typo forlib/*.js?Reacted by James M Snell, Anna Henningsen and Yuta HirotoI think this could be a good first contribution in general, though some of the errors could be more complicated(the ones that are more frequently getting parsed in userland).
Also, I'm assuming /lib.js is a typo for lib/.js?
Probably
lib/**/*.js:)Reacted by Martin Iwanowski@jasnell I've added links to those files, hope I am doing this right..
Reacted by James M Snell@fl0w ... yes, this can be a good first contribution. Please use my PR #11294 as a model. New error codes are added to
internal/errors.js. Please reuse existing codes as possible. For instance, my PR #11294 introduces theERR_INVALID_ARG_TYPEerror code. If you're going through and making changes before #11294 lands, and you needERR_INVALID_ARG_TYPE, duplicate it from #11294 as a separate commit in your PR. Then, if #11294 lands first, you can rebase and drop that commit, but if your PR lands first, then I can rebase and drop it from mine, etc. (hopefully that makes sense).Also please make sure that descriptions for the error codes are added to
docs/api/errors.mdthe way I've illustrated in #11294.Really appreciate your willingness to jump in! Let me know if you have any questions or issues!
Reacted by Martin Iwanowski, Bassem Ghoniem and Kreig ZimmermanModifying the error message is certainly possible. The biggest thing is to avoid duplicating error codes so make sure you check the other PRs for codes that may be reusable.
- added a commit that references this issue
on Feb 11, 2017 Do we need to launch CITGM for all these semver-major PRs? cc @nodejs/build
We should, yes.
124 remaining items
- added a commit that references this issue
on Sep 25, 2017 - added a commit that references this issue
on Oct 15, 2017 - added a commit that references this issue
on Oct 15, 2017 - added a commit that references this issue
on Oct 23, 2017 - added a commit that references this issue
on Oct 26, 2017 - added a commit that references this issue
on Dec 7, 2017 I am closing this as outdated. All regular JS errors got migrated but might need some more polishing here and there. There are a couple c++ errors that should be migrated but this was never tracked here.
@BridgeAR There seems to be a few new old-style errors added in JS since this PR opened, searching
new Error\(.+\)yields 23 results in the current master. We should probably open a new issue for that.I believe this issue is mostly done now except the punycode ones. I opened #27023 for that, so I am going to close this. Feel free to reopen if anyone thinks otherwise.
Oops, did not realize it was closed. Sorry about the noise.
Reacted by Steven
Now that #11220 has landed, we need to begin the process of migrating errors in the
*/lib.jssource over to use it. A basic guide is provided here.Note that moving existing errors over to this mechanism should, in general, be considered
semver-major.Please use the following list to track which files have been migrated over to using the new errors and provide a link back to this issue in the relevant PRs
_debug_agent.js(removed in 549e81bfa1)_debugger.js - debugger, errors: migrate to use internal/errors.js #11380(File removed from master)_linklist.jsstreamrelated (blocked)@refack: removed GFC label and commented out sentence in description + split off stream stuff
@BridgeAR: updated the list