src: assign ERR_SCRIPT_EXECUTION_* codes in C++

Also modifies the error messages so they include more information
and are more consistent.

- The message of ERR_SCRIPT_EXECUTION_INTERRUPTED now mentions
  SIGINT and the trailing period is dropped for consistency.
- Added ERR_SCRIPT_EXECUTION_TIMEOUT and include the timeout
  in the message.

PR-URL: https://github.com/nodejs/node/pull/20147
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This commit is contained in:
Joyee Cheung 2018-04-19 18:41:33 +08:00
parent 94e0e2c787
commit 3152b7c0d3
No known key found for this signature in database
GPG Key ID: F586868AAD831D0C
12 changed files with 83 additions and 39 deletions

View File

@ -1371,6 +1371,10 @@ An attempt was made to `require()` an [ES6 module][].
Script execution was interrupted by `SIGINT` (For example, when Ctrl+C was Script execution was interrupted by `SIGINT` (For example, when Ctrl+C was
pressed). pressed).
### ERR_SCRIPT_EXECUTION_TIMEOUT
Script execution timed out, possibly due to bugs in the script being executed.
<a id="ERR_SERVER_ALREADY_LISTEN"></a> <a id="ERR_SERVER_ALREADY_LISTEN"></a>
### ERR_SERVER_ALREADY_LISTEN ### ERR_SERVER_ALREADY_LISTEN

View File

@ -952,7 +952,7 @@ E('ERR_NO_LONGER_SUPPORTED', '%s is no longer supported', Error);
E('ERR_OUT_OF_RANGE', outOfRange, RangeError); E('ERR_OUT_OF_RANGE', outOfRange, RangeError);
E('ERR_REQUIRE_ESM', 'Must use import to load ES Module: %s', Error); E('ERR_REQUIRE_ESM', 'Must use import to load ES Module: %s', Error);
E('ERR_SCRIPT_EXECUTION_INTERRUPTED', E('ERR_SCRIPT_EXECUTION_INTERRUPTED',
'Script execution was interrupted by `SIGINT`.', Error); 'Script execution was interrupted by `SIGINT`', Error);
E('ERR_SERVER_ALREADY_LISTEN', E('ERR_SERVER_ALREADY_LISTEN',
'Listen method has been called more than once without closing.', Error); 'Listen method has been called more than once without closing.', Error);
E('ERR_SERVER_NOT_RUNNING', 'Server is not running.', Error); E('ERR_SERVER_NOT_RUNNING', 'Server is not running.', Error);

View File

@ -286,9 +286,9 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {
// which this timeout is nested, so check whether one of the watchdogs // which this timeout is nested, so check whether one of the watchdogs
// from this invocation is responsible for termination. // from this invocation is responsible for termination.
if (timed_out) { if (timed_out) {
env->ThrowError("Script execution timed out."); THROW_ERR_SCRIPT_EXECUTION_TIMEOUT(env, timeout);
} else if (received_signal) { } else if (received_signal) {
env->ThrowError("Script execution interrupted."); THROW_ERR_SCRIPT_EXECUTION_INTERRUPTED(env);
} }
} }

View File

@ -19,6 +19,7 @@
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
// USE OR OTHER DEALINGS IN THE SOFTWARE. // USE OR OTHER DEALINGS IN THE SOFTWARE.
#include "node_errors.h"
#include "node_internals.h" #include "node_internals.h"
#include "node_watchdog.h" #include "node_watchdog.h"
#include "base_object-inl.h" #include "base_object-inl.h"
@ -858,9 +859,9 @@ class ContextifyScript : public BaseObject {
// which this timeout is nested, so check whether one of the watchdogs // which this timeout is nested, so check whether one of the watchdogs
// from this invocation is responsible for termination. // from this invocation is responsible for termination.
if (timed_out) { if (timed_out) {
env->ThrowError("Script execution timed out."); node::THROW_ERR_SCRIPT_EXECUTION_TIMEOUT(env, timeout);
} else if (received_signal) { } else if (received_signal) {
env->ThrowError("Script execution interrupted."); node::THROW_ERR_SCRIPT_EXECUTION_INTERRUPTED(env);
} }
} }

View File

@ -8,6 +8,10 @@
#include "env-inl.h" #include "env-inl.h"
#include "v8.h" #include "v8.h"
// Use ostringstream to print exact-width integer types
// because the format specifiers are not available on AIX.
#include <sstream>
namespace node { namespace node {
// Helpers to construct errors similar to the ones provided by // Helpers to construct errors similar to the ones provided by
@ -24,6 +28,8 @@ namespace node {
V(ERR_MEMORY_ALLOCATION_FAILED, Error) \ V(ERR_MEMORY_ALLOCATION_FAILED, Error) \
V(ERR_MISSING_ARGS, TypeError) \ V(ERR_MISSING_ARGS, TypeError) \
V(ERR_MISSING_MODULE, Error) \ V(ERR_MISSING_MODULE, Error) \
V(ERR_SCRIPT_EXECUTION_INTERRUPTED, Error) \
V(ERR_SCRIPT_EXECUTION_TIMEOUT, Error) \
V(ERR_STRING_TOO_LONG, Error) \ V(ERR_STRING_TOO_LONG, Error) \
V(ERR_BUFFER_TOO_LARGE, Error) V(ERR_BUFFER_TOO_LARGE, Error)
@ -49,7 +55,9 @@ namespace node {
#define PREDEFINED_ERROR_MESSAGES(V) \ #define PREDEFINED_ERROR_MESSAGES(V) \
V(ERR_INDEX_OUT_OF_RANGE, "Index out of range") \ V(ERR_INDEX_OUT_OF_RANGE, "Index out of range") \
V(ERR_MEMORY_ALLOCATION_FAILED, "Failed to allocate memory") V(ERR_MEMORY_ALLOCATION_FAILED, "Failed to allocate memory") \
V(ERR_SCRIPT_EXECUTION_INTERRUPTED, \
"Script execution was interrupted by `SIGINT`")
#define V(code, message) \ #define V(code, message) \
inline v8::Local<v8::Value> code(v8::Isolate* isolate) { \ inline v8::Local<v8::Value> code(v8::Isolate* isolate) { \
@ -62,6 +70,13 @@ namespace node {
#undef V #undef V
// Errors with predefined non-static messages // Errors with predefined non-static messages
inline void THROW_ERR_SCRIPT_EXECUTION_TIMEOUT(Environment* env,
int64_t timeout) {
std::ostringstream message;
message << "Script execution timed out after ";
message << timeout << "ms";
THROW_ERR_SCRIPT_EXECUTION_TIMEOUT(env, message.str().c_str());
}
inline v8::Local<v8::Value> ERR_BUFFER_TOO_LARGE(v8::Isolate *isolate) { inline v8::Local<v8::Value> ERR_BUFFER_TOO_LARGE(v8::Isolate *isolate) {
char message[128]; char message[128];

View File

@ -34,10 +34,10 @@ child.stdout.once('data', common.mustCall(() => {
})); }));
child.on('close', function(code) { child.on('close', function(code) {
assert.strictEqual(code, 0); const expected = 'Script execution was interrupted by `SIGINT`';
assert.ok( assert.ok(
stdout.includes('Script execution interrupted.'), stdout.includes(expected),
`Expected stdout to contain "Script execution interrupted.", got ${stdout}` `Expected stdout to contain "${expected}", got ${stdout}`
); );
assert.ok( assert.ok(
stdout.includes('foobar'), stdout.includes('foobar'),

View File

@ -35,9 +35,10 @@ child.stdout.once('data', common.mustCall(() => {
child.on('close', function(code) { child.on('close', function(code) {
assert.strictEqual(code, 0); assert.strictEqual(code, 0);
const expected = 'Script execution was interrupted by `SIGINT`';
assert.ok( assert.ok(
stdout.includes('Script execution interrupted.\n'), stdout.includes(expected),
`Expected stdout to contain "Script execution interrupted.", got ${stdout}` `Expected stdout to contain "${expected}", got ${stdout}`
); );
assert.ok( assert.ok(
stdout.includes('42042\n'), stdout.includes('42042\n'),

View File

@ -161,7 +161,7 @@ async function ctrlCTest() {
]), [ ]), [
'await timeout(100000)\r', 'await timeout(100000)\r',
'Thrown: Error [ERR_SCRIPT_EXECUTION_INTERRUPTED]: ' + 'Thrown: Error [ERR_SCRIPT_EXECUTION_INTERRUPTED]: ' +
'Script execution was interrupted by `SIGINT`.', 'Script execution was interrupted by `SIGINT`',
PROMPT PROMPT
]); ]);
} }

View File

@ -36,8 +36,12 @@ if (process.argv[2] === 'child') {
[]; [];
const options = { breakOnSigint: true }; const options = { breakOnSigint: true };
assert.throws(() => { vm[method](script, ...args, options); }, common.expectsError(
/^Error: Script execution interrupted\.$/); () => { vm[method](script, ...args, options); },
{
code: 'ERR_SCRIPT_EXECUTION_INTERRUPTED',
message: 'Script execution was interrupted by `SIGINT`'
});
assert.strictEqual(firstHandlerCalled, 0); assert.strictEqual(firstHandlerCalled, 0);
assert.strictEqual(onceHandlerCalled, 0); assert.strictEqual(onceHandlerCalled, 0);

View File

@ -24,8 +24,12 @@ if (process.argv[2] === 'child') {
for (let i = 0; i < listeners; i++) for (let i = 0; i < listeners; i++)
process.on('SIGINT', common.mustNotCall()); process.on('SIGINT', common.mustNotCall());
assert.throws(() => { vm[method](script, ...args, options); }, common.expectsError(
/^Error: Script execution interrupted\.$/); () => { vm[method](script, ...args, options); },
{
code: 'ERR_SCRIPT_EXECUTION_INTERRUPTED',
message: 'Script execution was interrupted by `SIGINT`'
});
return; return;
} }

View File

@ -25,35 +25,50 @@ const assert = require('assert');
const vm = require('vm'); const vm = require('vm');
// Timeout of 100ms executing endless loop // Timeout of 100ms executing endless loop
assert.throws(function() { assert.throws(
vm.runInThisContext('while(true) {}', { timeout: 100 }); function() {
}, /^Error: Script execution timed out\.$/); vm.runInThisContext('while(true) {}', { timeout: 100 });
},
{
code: 'ERR_SCRIPT_EXECUTION_TIMEOUT',
message: 'Script execution timed out after 100ms'
});
// Timeout of 1000ms, script finishes first // Timeout of 1000ms, script finishes first
vm.runInThisContext('', { timeout: 1000 }); vm.runInThisContext('', { timeout: 1000 });
// Nested vm timeouts, inner timeout propagates out // Nested vm timeouts, inner timeout propagates out
assert.throws(function() { assert.throws(
const context = { function() {
log: console.log, const context = {
runInVM: function(timeout) { log: console.log,
vm.runInNewContext('while(true) {}', context, { timeout }); runInVM: function(timeout) {
} vm.runInNewContext('while(true) {}', context, { timeout });
}; }
vm.runInNewContext('runInVM(10)', context, { timeout: 10000 }); };
throw new Error('Test 5 failed'); vm.runInNewContext('runInVM(10)', context, { timeout: 10000 });
}, /Script execution timed out\./); throw new Error('Test 5 failed');
},
{
code: 'ERR_SCRIPT_EXECUTION_TIMEOUT',
message: 'Script execution timed out after 10ms'
});
// Nested vm timeouts, outer timeout is shorter and fires first. // Nested vm timeouts, outer timeout is shorter and fires first.
assert.throws(function() { assert.throws(
const context = { function() {
runInVM: function(timeout) { const context = {
vm.runInNewContext('while(true) {}', context, { timeout }); runInVM: function(timeout) {
} vm.runInNewContext('while(true) {}', context, { timeout });
}; }
vm.runInNewContext('runInVM(10000)', context, { timeout: 100 }); };
throw new Error('Test 6 failed'); vm.runInNewContext('runInVM(10000)', context, { timeout: 100 });
}, /Script execution timed out\./); throw new Error('Test 6 failed');
},
{
code: 'ERR_SCRIPT_EXECUTION_TIMEOUT',
message: 'Script execution timed out after 100ms'
});
// Nested vm timeouts, inner script throws an error. // Nested vm timeouts, inner script throws an error.
assert.throws(function() { assert.throws(function() {

View File

@ -39,6 +39,6 @@ if (process.argv[2] === 'child') {
}); });
process.on('exit', function() { process.on('exit', function() {
assert.ok(/Script execution timed out/.test(err)); assert.ok(/Script execution timed out after 1ms/.test(err));
}); });
} }