errors,repl: migrate to use internal/errors.js #13299
Conversation
nodejs-github-bot
added
errors
| E('ERR_IPC_CHANNEL_CLOSED', 'channel closed'); | ||
| E('ERR_IPC_DISCONNECTED', 'IPC channel is already disconnected'); | ||
| E('ERR_IPC_ONE_PIPE', 'Child process can have only one IPC pipe'); | ||
| E('ERR_IPC_SYNC_FORK', 'IPC cannot be used with synchronous forks'); | ||
| E('ERR_MISSING_ARGS', missingArgs); | ||
| E('ERR_PARSE_HISTORY_DATA', | ||
| (oldHistoryPath) => `Could not parse history data in ${oldHistoryPath}`); |
There was a problem hiding this comment.
Just a nit, but it's a bit odd to have one of the new codes use %s while the other uses a template string.
There was a problem hiding this comment.
@jasnell I have used template string. So, do you want me to convert it to %s?
There was a problem hiding this comment.
I would switch the ERR_INVALID_REPL_HISTORY one to use a template string
There was a problem hiding this comment.
@jasnell Done.
lib/internal/repl.js
Outdated
| @@ -153,13 +154,14 @@ function setupHistory(repl, historyPath, oldHistoryPath, ready) { | |||
| if (oldReplJSONHistory) repl.history = JSON.parse(oldReplJSONHistory); | |||
|
|
|||
| if (!Array.isArray(repl.history)) { | |||
| throw new Error('Expected array, got ' + typeof repl.history); | |||
| throw new errors.Error('ERR_INVALID_REPL_HISTORY', | |||
There was a problem hiding this comment.
This should likely be a TypeError. Since this is already a semver-major change, may as well fix the type
There was a problem hiding this comment.
@jasnell I too thought on those lines. But to be consistent with previous code, I did in that manner. Now, it is fixed. Thanks.
jasnell
added
the
semver-major
lib/internal/repl.js
Outdated
| @@ -153,13 +154,14 @@ function setupHistory(repl, historyPath, oldHistoryPath, ready) { | |||
| if (oldReplJSONHistory) repl.history = JSON.parse(oldReplJSONHistory); | |||
|
|
|||
| if (!Array.isArray(repl.history)) { | |||
| throw new Error('Expected array, got ' + typeof repl.history); | |||
| throw new errors.TypeError('ERR_INVALID_ARG_TYPE', | |||
| typeof repl.history, 'Array'); | |||
There was a problem hiding this comment.
be sure to line up the arguments appropriate and run make lint
There was a problem hiding this comment.
@jasnell Unfortunately, it is not captured in lint test. Now, it's Done.
PR-URL: #13299 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>
|
Landed in 2822796 |
|
Too late, but doesn't LGTM. |
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
lib/internal/repl.js
lib/internal/errors.js
ref: #11273