From c8c9211fa6220ccf9e2aa09819d1c07e0a635649 Mon Sep 17 00:00:00 2001
From: Ruben Bridgewater <ruben@bridgewater.de>
Date: Thu, 17 May 2018 13:20:27 +0200
Subject: [PATCH] util: improve error inspection

When inspecting errors with extra properties while setting the
compact option to false, it will now return:

[Error: foo] {
    at repl:1:5
    at Script.runInThisContext (vm.js:89:20)
  bla: true
}

Instead of:

Error: foo
    at repl:1:5
    at Script.runInThisContext (vm.js:91:20) {
  bla: true
}

PR-URL: https://github.com/nodejs/node/pull/20802
Refs: https://github.com/nodejs/node/issues/20253
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
---
 lib/util.js                           |  8 ++++-
 test/parallel/test-repl-underscore.js |  2 +-
 test/parallel/test-util-inspect.js    | 42 +++++++++++++++++++++++++--
 3 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/lib/util.js b/lib/util.js
index 36a74b2e3041ab..79f3cc7205398b 100644
--- a/lib/util.js
+++ b/lib/util.js
@@ -597,7 +597,8 @@ function formatValue(ctx, value, recurseTimes) {
       // Make error with message first say the error
       base = formatError(value);
       // Wrap the error in brackets in case it has no stack trace.
-      if (base.indexOf('\n    at') === -1) {
+      const stackStart = base.indexOf('\n    at');
+      if (stackStart === -1) {
         base = `[${base}]`;
       }
       // The message and the stack have to be indented as well!
@@ -607,6 +608,11 @@ function formatValue(ctx, value, recurseTimes) {
       }
       if (keyLength === 0)
         return base;
+
+      if (ctx.compact === false && stackStart !== -1) {
+        braces[0] += `${base.slice(stackStart)}`;
+        base = `[${base.slice(0, stackStart)}]`;
+      }
     } else if (isAnyArrayBuffer(value)) {
       // Fast path for ArrayBuffer and SharedArrayBuffer.
       // Can't do the same for DataView because it has a non-primitive
diff --git a/test/parallel/test-repl-underscore.js b/test/parallel/test-repl-underscore.js
index 57929244ae4374..628a7739fecf4e 100644
--- a/test/parallel/test-repl-underscore.js
+++ b/test/parallel/test-repl-underscore.js
@@ -174,7 +174,7 @@ function testError() {
 
       // The error, both from the original throw and the `_error` echo.
       'Error: foo',
-      'Error: foo',
+      '[Error: foo]',
 
       // The sync error, with individual property echoes
       /Error: ENOENT: no such file or directory, scandir '.*nonexistent.*'/,
diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js
index b7aa752952ce4c..2899beecf64346 100644
--- a/test/parallel/test-util-inspect.js
+++ b/test/parallel/test-util-inspect.js
@@ -519,11 +519,49 @@ assert.strictEqual(util.inspect(-5e-324), '-5e-324');
   const tmp = Error.stackTraceLimit;
   Error.stackTraceLimit = 0;
   const err = new Error('foo');
-  assert.strictEqual(util.inspect(err), '[Error: foo]');
+  const err2 = new Error('foo\nbar');
+  assert.strictEqual(util.inspect(err, { compact: true }), '[Error: foo]');
   assert(err.stack);
   delete err.stack;
   assert(!err.stack);
-  assert.strictEqual(util.inspect(err), '[Error: foo]');
+  assert.strictEqual(util.inspect(err, { compact: true }), '[Error: foo]');
+  assert.strictEqual(
+    util.inspect(err2, { compact: true }),
+    '[Error: foo\nbar]'
+  );
+
+  err.bar = true;
+  err2.bar = true;
+
+  assert.strictEqual(
+    util.inspect(err, { compact: true }),
+    '{ [Error: foo] bar: true }'
+  );
+  assert.strictEqual(
+    util.inspect(err2, { compact: true }),
+    '{ [Error: foo\nbar] bar: true }'
+  );
+  assert.strictEqual(
+    util.inspect(err, { compact: true, breakLength: 5 }),
+    '{ [Error: foo]\n  bar: true }'
+  );
+  assert.strictEqual(
+    util.inspect(err, { compact: true, breakLength: 1 }),
+    '{ [Error: foo]\n  bar:\n   true }'
+  );
+  assert.strictEqual(
+    util.inspect(err2, { compact: true, breakLength: 5 }),
+    '{ [Error: foo\nbar]\n  bar: true }'
+  );
+  assert.strictEqual(
+    util.inspect(err, { compact: false }),
+    '[Error: foo] {\n  bar: true\n}'
+  );
+  assert.strictEqual(
+    util.inspect(err2, { compact: false }),
+    '[Error: foo\nbar] {\n  bar: true\n}'
+  );
+
   Error.stackTraceLimit = tmp;
 }