repl: improve error output

1) Currently extra properties on an error will be ignored, if thrown. This information will from now on be visible. 2) In case someone threw a non error object it would have resulted in `[object Object]`. Instead, the full object will now be visible. 3) Some cases were not detected properly as error before and "Thrown: " was visible before. That is now fixed. PR-URL: #22436 Refs: #20253 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs · Sep 23, 2018 · b1ffda6 · b1ffda6
1 parent 39d7699
commit b1ffda6
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 24 deletions.
diff --git a/lib/repl.js b/lib/repl.js
@@ -410,29 +410,50 @@ function REPLServer(prompt,
 
   self._domain.on('error', function debugDomainError(e) {
     debug('domain error');
-    const top = replMap.get(self);
-    const pstrace = Error.prepareStackTrace;
-    Error.prepareStackTrace = prepareStackTrace(pstrace);
-    if (typeof e === 'object')
+    let errStack = '';
+
+    if (typeof e === 'object' && e !== null) {
+      const pstrace = Error.prepareStackTrace;
+      Error.prepareStackTrace = prepareStackTrace(pstrace);
       internalUtil.decorateErrorStack(e);
-    Error.prepareStackTrace = pstrace;
-    const isError = internalUtil.isError(e);
-    if (!self.underscoreErrAssigned)
-      self.lastError = e;
-    if (e instanceof SyntaxError && e.stack) {
-      // remove repl:line-number and stack trace
-      e.stack = e.stack
-        .replace(/^repl:\d+\r?\n/, '')
-        .replace(/^\s+at\s.*\n?/gm, '');
-    } else if (isError && self.replMode === exports.REPL_MODE_STRICT) {
-      e.stack = e.stack.replace(/(\s+at\s+repl:)(\d+)/,
-                                (_, pre, line) => pre + (line - 1));
+      Error.prepareStackTrace = pstrace;
+
+      if (e.domainThrown) {
+        delete e.domain;
+        delete e.domainThrown;
+      }
+
+      if (internalUtil.isError(e)) {
+        if (e.stack) {
+          if (e.name === 'SyntaxError') {
+            // Remove stack trace.
+            e.stack = e.stack
+              .replace(/^repl:\d+\r?\n/, '')
+              .replace(/^\s+at\s.*\n?/gm, '');
+          } else if (self.replMode === exports.REPL_MODE_STRICT) {
+            e.stack = e.stack.replace(/(\s+at\s+repl:)(\d+)/,
+                                      (_, pre, line) => pre + (line - 1));
+          }
+        }
+        errStack = util.inspect(e);
+
+        // Remove one line error braces to keep the old style in place.
+        if (errStack[errStack.length - 1] === ']') {
+          errStack = errStack.slice(1, -1);
+        }
+      }
     }
-    if (isError && e.stack) {
-      top.outputStream.write(`${e.stack}\n`);
-    } else {
-      top.outputStream.write(`Thrown: ${String(e)}\n`);
+
+    if (errStack === '') {
+      errStack = `Thrown: ${util.inspect(e)}`;
     }
+
+    if (!self.underscoreErrAssigned) {
+      self.lastError = e;
+    }
+
+    const top = replMap.get(self);
+    top.outputStream.write(`${errStack}\n`);
     top.clearBufferedCommand();
     top.lines.level = [];
     top.displayPrompt();

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

diff --git a/test/parallel/test-repl-underscore.js b/test/parallel/test-repl-underscore.js
@@ -177,8 +177,12 @@ function testError() {
       '[Error: foo]',
 
       // The sync error, with individual property echoes
-      /Error: ENOENT: no such file or directory, scandir '.*nonexistent.*'/,
+      /^{ Error: ENOENT: no such file or directory, scandir '.*nonexistent.*'/,
       /Object\.readdirSync/,
+      /^  errno: -(2|4058),$/,
+      "  syscall: 'scandir',",
+      "  code: 'ENOENT',",
+      "  path: '/nonexistent?' }",
       "'ENOENT'",
       "'scandir'",
 

diff --git a/test/parallel/test-repl.js b/test/parallel/test-repl.js
@@ -132,7 +132,11 @@ const errorTests = [
   // Uncaught error throws and prints out
   {
     send: 'throw new Error(\'test error\');',
-    expect: /^Error: test error/
+    expect: 'Error: test error'
+  },
+  {
+    send: "throw { foo: 'bar' };",
+    expect: "Thrown: { foo: 'bar' }"
   },
   // Common syntax error is treated as multiline command
   {
@@ -526,7 +530,7 @@ const errorTests = [
   {
     send: 'require("internal/repl")',
     expect: [
-      /^Error: Cannot find module 'internal\/repl'/,
+      /^{ Error: Cannot find module 'internal\/repl'/,
       /^    at .*/,
       /^    at .*/,
       /^    at .*/,