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: #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>
nodejs · Apr 25, 2018 · 3152b7c · 3152b7c
1 parent 94e0e2c
commit 3152b7c
Show file tree

Hide file tree

Showing 12 changed files with 83 additions and 39 deletions.
diff --git a/doc/api/errors.md b/doc/api/errors.md
@@ -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
 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>
 ### ERR_SERVER_ALREADY_LISTEN
 

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

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

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
@@ -19,6 +19,7 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
+#include "node_errors.h"
 #include "node_internals.h"
 #include "node_watchdog.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
       // from this invocation is responsible for termination.
       if (timed_out) {
-        env->ThrowError("Script execution timed out.");
+        node::THROW_ERR_SCRIPT_EXECUTION_TIMEOUT(env, timeout);
       } else if (received_signal) {
-        env->ThrowError("Script execution interrupted.");
+        node::THROW_ERR_SCRIPT_EXECUTION_INTERRUPTED(env);
       }
     }
 

diff --git a/src/node_errors.h b/src/node_errors.h
@@ -8,6 +8,10 @@
 #include "env-inl.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 {
 
 // Helpers to construct errors similar to the ones provided by
@@ -24,6 +28,8 @@ namespace node {
   V(ERR_MEMORY_ALLOCATION_FAILED, Error)                                     \
   V(ERR_MISSING_ARGS, TypeError)                                             \
   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_BUFFER_TOO_LARGE, Error)
 
@@ -49,7 +55,9 @@ namespace node {
 
 #define PREDEFINED_ERROR_MESSAGES(V)                                         \
   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)                                                     \
   inline v8::Local<v8::Value> code(v8::Isolate* isolate) {                   \
@@ -62,6 +70,13 @@ namespace node {
 #undef V
 
 // 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) {
   char message[128];

diff --git a/test/parallel/test-repl-sigint-nested-eval.js b/test/parallel/test-repl-sigint-nested-eval.js
@@ -34,10 +34,10 @@ child.stdout.once('data', common.mustCall(() => {
 }));
 
 child.on('close', function(code) {
-  assert.strictEqual(code, 0);
+  const expected = 'Script execution was interrupted by `SIGINT`';
   assert.ok(
-    stdout.includes('Script execution interrupted.'),
-    `Expected stdout to contain "Script execution interrupted.", got ${stdout}`
+    stdout.includes(expected),
+    `Expected stdout to contain "${expected}", got ${stdout}`
   );
   assert.ok(
     stdout.includes('foobar'),

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

diff --git a/test/parallel/test-repl-top-level-await.js b/test/parallel/test-repl-top-level-await.js
@@ -161,7 +161,7 @@ async function ctrlCTest() {
   ]), [
     'await timeout(100000)\r',
     'Thrown: Error [ERR_SCRIPT_EXECUTION_INTERRUPTED]: ' +
-      'Script execution was interrupted by `SIGINT`.',
+      'Script execution was interrupted by `SIGINT`',
     PROMPT
   ]);
 }

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

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

diff --git a/test/parallel/test-vm-timeout.js b/test/parallel/test-vm-timeout.js
@@ -25,35 +25,50 @@ const assert = require('assert');
 const vm = require('vm');
 
 // Timeout of 100ms executing endless loop
-assert.throws(function() {
-  vm.runInThisContext('while(true) {}', { timeout: 100 });
-}, /^Error: Script execution timed out\.$/);
+assert.throws(
+  function() {
+    vm.runInThisContext('while(true) {}', { timeout: 100 });
+  },
+  {
+    code: 'ERR_SCRIPT_EXECUTION_TIMEOUT',
+    message: 'Script execution timed out after 100ms'
+  });
 
 // Timeout of 1000ms, script finishes first
 vm.runInThisContext('', { timeout: 1000 });
 
 // Nested vm timeouts, inner timeout propagates out
-assert.throws(function() {
-  const context = {
-    log: console.log,
-    runInVM: function(timeout) {
-      vm.runInNewContext('while(true) {}', context, { timeout });
-    }
-  };
-  vm.runInNewContext('runInVM(10)', context, { timeout: 10000 });
-  throw new Error('Test 5 failed');
-}, /Script execution timed out\./);
+assert.throws(
+  function() {
+    const context = {
+      log: console.log,
+      runInVM: function(timeout) {
+        vm.runInNewContext('while(true) {}', context, { timeout });
+      }
+    };
+    vm.runInNewContext('runInVM(10)', context, { timeout: 10000 });
+    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.
-assert.throws(function() {
-  const context = {
-    runInVM: function(timeout) {
-      vm.runInNewContext('while(true) {}', context, { timeout });
-    }
-  };
-  vm.runInNewContext('runInVM(10000)', context, { timeout: 100 });
-  throw new Error('Test 6 failed');
-}, /Script execution timed out\./);
+assert.throws(
+  function() {
+    const context = {
+      runInVM: function(timeout) {
+        vm.runInNewContext('while(true) {}', context, { timeout });
+      }
+    };
+    vm.runInNewContext('runInVM(10000)', context, { timeout: 100 });
+    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.
 assert.throws(function() {

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