From ccb303e03795cee25527edb867f7903477ed5de6 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Tue, 21 Aug 2018 03:22:18 +0200
Subject: 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: https://github.com/nodejs/node/pull/22436
Refs: https://github.com/nodejs/node/issues/20253
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 lib/repl.js                                | 61 ++++++++++++++++++++----------
 test/parallel/test-repl-top-level-await.js |  2 +-
 test/parallel/test-repl-underscore.js      |  6 ++-
 test/parallel/test-repl.js                 |  8 +++-
 4 files changed, 53 insertions(+), 24 deletions(-)

diff --git a/lib/repl.js b/lib/repl.js
index a0cf2c1dd0..ad1a4bab91 100644
--- 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
index 662adeb2e9..9194c4a449 100644
--- 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
index f7fe0bcea8..dea2ed4370 100644
--- 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
index 8cb4b686b8..49e718da0f 100644
--- 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 .*/,
-- 
cgit v1.2.3