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

repl: migrate errors to internal/errors #17716

Closed
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -1330,6 +1330,11 @@ The `REPL` module was unable parse data from the REPL history file.

An attempt was made to `require()` an [ES6 module][].

<a id="ERR_SCRIPT_EXECUTION_INTERRUPTED"></a>
### ERR_SCRIPT_EXECUTION_INTERRUPTED

A script execution interrupted.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate a bit more on this? For example:

Script execution was interrupted by `SIGINT` (Ctrl+C).


<a id="ERR_SERVER_ALREADY_LISTEN"></a>
### ERR_SERVER_ALREADY_LISTEN

Expand Down
1 change: 1 addition & 0 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,7 @@ E('ERR_OUTOFMEMORY', 'Out of memory');
E('ERR_OUT_OF_RANGE', 'The "%s" argument is out of range');
E('ERR_PARSE_HISTORY_DATA', 'Could not parse history data in %s');
E('ERR_REQUIRE_ESM', 'Must use import to load ES Module: %s');
E('ERR_SCRIPT_EXECUTION_INTERRUPTED', 'Script execution interrupted.');
E('ERR_SERVER_ALREADY_LISTEN',
'Listen method has been called more than once without closing.');
E('ERR_SOCKET_ALREADY_BOUND', 'Socket is already bound');
Expand Down
5 changes: 3 additions & 2 deletions lib/repl.js
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,8 @@ function REPLServer(prompt,
} catch (e) {
err = e;

if (err && err.message === 'Script execution interrupted.') {
if (err && err.message ===
errors.message('ERR_SCRIPT_EXECUTION_INTERRUPTED')) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can check if err.code === 'ERR_SCRIPT_EXECUTION_INTERRUPTED' here instead (which is kinda the point of this PR, the code instead of the message should be used to match a type of error)

// The stack trace for this case is not very useful anyway.
Object.defineProperty(err, 'stack', { value: '' });
}
Expand All @@ -335,7 +336,7 @@ function REPLServer(prompt,
if (self.breakEvalOnSigint) {
const interrupt = new Promise((resolve, reject) => {
sigintListener = () => {
reject(new Error('Script execution interrupted.'));
reject(new errors.Error('ERR_SCRIPT_EXECUTION_INTERRUPTED'));
};
prioritizedSigintQueue.add(sigintListener);
});
Expand Down
7 changes: 7 additions & 0 deletions test/parallel/test-internal-errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,13 @@ assert.strictEqual(
);
}

// Test ERR_SCRIPT_EXECUTION_INTERRUPTED
assert.strictEqual(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test does not look particularly useful since the error message formatter does not take any parameters for this error.

errors.message('ERR_SCRIPT_EXECUTION_INTERRUPTED'),
'Script execution interrupted.'
);


// Test that `code` property is mutable and that changing it does not change the
// name.
{
Expand Down
4 changes: 3 additions & 1 deletion test/parallel/test-repl-top-level-await.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const common = require('../common');
const assert = require('assert');
const errors = require('internal/errors');
const { stripVTControlCharacters } = require('internal/readline');
const repl = require('repl');

Expand Down Expand Up @@ -160,7 +161,8 @@ async function ctrlCTest() {
{ ctrl: true, name: 'c' }
]), [
'await timeout(100000)\r',
'Thrown: Error: Script execution interrupted.',
'Thrown: Error [ERR_SCRIPT_EXECUTION_INTERRUPTED]: ' +
errors.message('ERR_SCRIPT_EXECUTION_INTERRUPTED'),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are actually testing the error message, we don't really need to generate it with errors.message, can you change this to match the actual error message instead?

PROMPT
]);
}
Expand Down