error: read name/message via full [[Get]] for the .stack header; drop inspect.js workaround

src/js/internal/util/inspect.js

@@ -1781,9 +1781,6 @@ function formatError(err, constructor, tag, ctx, keys) {
   const name = err.name != null ? String(err.name) : "Error";
   let stack = getStackString(err);
 
-  //! temp fix for Bun losing the error name from inherited errors + extraneous ": " with no message
-  stack = stack.replace(/^Error: /, `${name}${err.message ? ": " : ""}`);
-
   removeDuplicateErrorKeys(ctx, keys, err, stack);
 
   if ("cause" in err && (keys.length === 0 || !ArrayPrototypeIncludes(keys, "cause"))) {

src/jsc/bindings/FormatStackTraceForJS.cpp

@@ -409,14 +409,28 @@ static String computeErrorInfoWithoutPrepareStackTrace(
     WTF::String message;
 
     if (errorInstance) {
-        // Note that we are not allowed to allocate memory in here. It's called inside a finalizer.
-        if (auto* instance = dynamicDowncast<ErrorInstance>(errorInstance)) {
-            if (!lexicalGlobalObject) {
-                lexicalGlobalObject = errorInstance->globalObject();
-            }
-            name = instance->sanitizedNameString(lexicalGlobalObject);
+        // The GC-finalizer path (computeErrorInfoWrapperToString) always passes a null
+        // errorInstance, so this branch only runs from a mutator (lazy .stack getter,
+        // materializeErrorInfoIfNeeded, captureStackTrace) where user code may execute.
+        if (!lexicalGlobalObject) {
+            lexicalGlobalObject = errorInstance->globalObject();
+        }
+
+        // V8's ErrorUtils::ToString: [[Get]] "name"/"message" (prototype walk, getters,
+        // ToString coercion). An undefined name defaults to "Error", an undefined message
+        // to the empty string. sanitizedNameString/sanitizedMessageString skip getters and
+        // cap the prototype walk at depth 2, which drops subclass names and messages.
+        JSValue nameValue = errorInstance->get(lexicalGlobalObject, vm.propertyNames->name);
+        RETURN_IF_EXCEPTION(scope, {});
+        if (!nameValue.isUndefined()) {
+            name = nameValue.toWTFString(lexicalGlobalObject);
             RETURN_IF_EXCEPTION(scope, {});
-            message = instance->sanitizedMessageString(lexicalGlobalObject);
+        }
+
+        JSValue messageValue = errorInstance->get(lexicalGlobalObject, vm.propertyNames->message);
+        RETURN_IF_EXCEPTION(scope, {});
+        if (!messageValue.isUndefined()) {
+            message = messageValue.toWTFString(lexicalGlobalObject);
             RETURN_IF_EXCEPTION(scope, {});
         }
     }
@@ -778,6 +792,23 @@ JSC_DEFINE_HOST_FUNCTION(errorConstructorFuncCaptureStackTrace, (JSC::JSGlobalOb
     WTF::Vector<JSC::StackFrame> stackTrace;
     JSCStackTrace::getFramesForCaller(vm, callFrame, errorObject, caller, stackTrace, stackTraceLimit);
 
+    // Root the frames' cells: both eager-compute paths below read name/message via
+    // [[Get]] before formatting, which may allocate or run a user getter. The lazy
+    // path moves the frames into the ErrorInstance, which visits them; rooting is a
+    // no-op there.
+    JSC::MarkedArgumentBuffer protectedFrameCells;
+    protectedFrameCells.ensureCapacity(stackTrace.size() * 2);
+    for (auto& frame : stackTrace) {
+        if (auto* callee = frame.callee())
+            protectedFrameCells.append(callee);
+        if (auto* codeBlock = frame.codeBlock())
+            protectedFrameCells.append(codeBlock);
+    }
+    if (protectedFrameCells.hasOverflowed()) [[unlikely]] {
+        throwOutOfMemoryError(globalObject, scope);
+        return {};
+    }
+
     if (auto* instance = dynamicDowncast<JSC::ErrorInstance>(errorObject)) {
         if (instance->hasMaterializedErrorInfo()) {
             // Error info was already materialized (e.g. .stack was previously accessed).

test/js/node/util/node-inspect-tests/parallel/util-format.test.js

@@ -419,7 +419,7 @@ test("no assertion failures", () => {
 
   // Errors
   const err = new Error("foo");
-  assert(util.format(err).startsWith(err.stack), `Expected "${util.format(err)}" to start with "${err.stack}"`);
+  assert.strictEqual(util.format(err), err.stack);
 
   class CustomError extends Error {
     constructor(msg) {
@@ -433,7 +433,7 @@ test("no assertion failures", () => {
   customError.stack;
   delete customError.originalLine;
   delete customError.originalColumn;
-  assert.strictEqual(util.format(customError), customError.stack.replace(/^Error/, "Custom$&")); //! temp bug workaround
+  assert.strictEqual(util.format(customError), customError.stack);
   // Doesn't capture stack trace
   function BadCustomError(msg) {
     Error.call(this);

test/js/node/util/node-inspect-tests/parallel/util-inspect.test.js

@@ -591,14 +591,7 @@
   // Exceptions should print the error message, not '{}'.
   {
     [new Error(), new Error("FAIL"), new TypeError("FAIL"), new SyntaxError("FAIL")].forEach(err => {
-      assert(
-        //! temp bug workaround with replace()'s
-        util.inspect(err).startsWith(err.stack.replace(/^Error: /, err.message ? "$&" : "Error")),
-        `Expected "${util.inspect(err)}" to start with "${err.stack.replace(
-          /^Error: /,
-          err.message ? "$&" : "Error",
-        )}"`,
-      );
+      assert.strictEqual(util.inspect(err), err.stack);
     });
 
     assert.throws(
@@ -1854,11 +1847,10 @@
   ].forEach(([Class, message], i) => {
     const foo = new Class(message);
     const extra = Class.name.includes("Error") ? "" : ` [${foo.name}]`;
-    // TODO: Bun messes with `Error.stack` and this causes this to fail
-    // assert(
-    //   util.inspect(foo).startsWith(`${Class.name}${extra}${message ? `: ${message}` : "\n"}`),
-    //   util.inspect(foo) + "\n...did not start with: " + `${Class.name}${extra}${message ? `: ${message}` : "\n"}`,
-    // );
+    assert(
+      util.inspect(foo).startsWith(`${Class.name}${extra}${message ? `: ${message}` : "\n"}`),
+      util.inspect(foo) + "\n...did not start with: " + `${Class.name}${extra}${message ? `: ${message}` : "\n"}`,
+    );
     Object.defineProperty(foo, Symbol.toStringTag, {
       value: "WOW",
       writable: true,
@@ -3189,6 +3181,128 @@
   }
 });
 
+test("error inspect preserves stack header when name/message change after materialization", () => {
+  // Bun's native .stack already emits `${name}${message ? ": " + message : ""}` as the first line,
+  // so formatError must not rewrite it. These headers match Node's output for the same inputs.
+  const firstLine = e => util.inspect(e).split("\n")[0];
+
+  // message cleared after .stack was materialized: header is preserved verbatim
+  {
+    const err = new Error("msg");
+    void err.stack;
+    err.message = "";
+    assert.strictEqual(firstLine(err), "Error: msg");
+  }
+  {
+    const err = new Error("Error: nested");
+    void err.stack;
+    err.message = "";
+    assert.strictEqual(firstLine(err), "Error: Error: nested");
+  }
+
+  // user-assigned stack starting with "Error: " on an empty-message Error is preserved
+  {
+    const err = new Error();
+    err.stack = "Error: manually set\n    at foo";
+    assert.strictEqual(firstLine(err), "Error: manually set");
+  }
+
+  // name changed after .stack was materialized: header is not rewritten to the new name
+  {
+    const err = new Error("x");
+    void err.stack;
+    err.name = "Renamed";
+    assert.strictEqual(firstLine(err), "Error: x");
+  }
+
+  // native header is correct for subclassed errors and empty-message errors
+  {
+    class Foo extends Error {
+      name = "Foo";
+    }
+    const err = new Foo("x");
+    assert.strictEqual(err.stack.split("\n")[0], "Foo: x");
+    assert.strictEqual(firstLine(err), "Foo: x");
+  }
+  {
+    const err = new Error();
+    assert.strictEqual(err.stack.split("\n")[0], "Error");
+    assert.strictEqual(firstLine(err), "Error");
+  }
+});
+
+test("error stack header reads name/message via full [[Get]]", () => {
+  // V8 composes the .stack header with ErrorUtils::ToString: ordinary [[Get]] on "name"
+  // and "message" (prototype walk, accessors, ToString). Each expected value matches Node.
+  const header = e => e.stack.split("\n")[0];
+  const inspected = e => util.inspect(e).split("\n")[0];
+
+  // name on an intermediate prototype (2+ levels deep)
+  {
+    class Bar extends Error {}
+    class Foo extends Bar {}
+    Bar.prototype.name = "Bar";
+    const err = new Foo("x");
+    assert.strictEqual(header(err), "Bar: x");
+    assert.strictEqual(inspected(err), "Bar: x");
+  }
+
+  // name defined as an accessor
+  {
+    class G extends Error {
+      get name() {
+        return "G";
+      }
+    }
+    const err = new G("m");
+    assert.strictEqual(header(err), "G: m");
+    assert.strictEqual(inspected(err), "G: m");
+  }
+
+  // non-primitive name coerces via ToString
+  {
+    const err = new Error("m");
+    err.name = { toString: () => "O" };
+    assert.strictEqual(header(err), "O: m");
+  }
+
+  // message defined as an accessor / on an intermediate prototype
+  {
+    class M extends Error {
+      get message() {
+        return "acc-msg";
+      }
+    }
+    assert.strictEqual(header(new M()), "Error: acc-msg");
+  }
+  {
+    class A extends Error {}
+    class B extends A {}
+    A.prototype.message = "deep";
+    assert.strictEqual(header(new B()), "Error: deep");
+  }
+
+  // undefined name defaults to "Error"; null name stringifies
+  {
+    const e1 = new Error("m");
+    e1.name = undefined;
+    assert.strictEqual(header(e1), "Error: m");
+    const e2 = new Error("m");
+    e2.name = null;
+    assert.strictEqual(header(e2), "null: m");
+  }
+
+  // a name getter that throws propagates out of the .stack read
+  {
+    class T extends Error {
+      get name() {
+        throw new TypeError("name-boom");
+      }
+    }
+    assert.throws(() => new T("m").stack, /name-boom/);
+  }
+});
+
 // Utility functions
 function runCallChecks(exitCode) {
   if (exitCode !== 0) return;