worker_threads: deliver an Error to 'error' listeners when the thrown error cannot be printed or cloned

src/jsc/bindings/webcore/SerializedScriptValue.cpp

@@ -1187,10 +1187,21 @@ class CloneSerializer : public CloneBase {
                 if (!startObjectInternal(errorInstance)) // handle duplicates
                     return true;
                 auto& vm = m_lexicalGlobalObject->vm();
+                // A WorkerErrorReport keeps the fields it can read (node's error_serdes.js): a throw costs the field, not the clone.
+                auto fieldUnreadable = [&] {
+                    return m_context == SerializationContext::WorkerErrorReport && scope.exception() && scope.tryClearException();
+                };
                 auto errorTypeValue = errorInstance->get(m_lexicalGlobalObject, vm.propertyNames->name);
+                bool nameUnreadable = fieldUnreadable();
                 RETURN_IF_EXCEPTION(scope, false);
-                auto errorTypeString = errorTypeValue.toWTFString(m_lexicalGlobalObject);
-                RETURN_IF_EXCEPTION(scope, false);
+                String errorTypeString;
+                if (!nameUnreadable) {
+                    errorTypeString = errorTypeValue.toWTFString(m_lexicalGlobalObject);
+                    nameUnreadable = fieldUnreadable();
+                    RETURN_IF_EXCEPTION(scope, false);
+                }
+                if (nameUnreadable)
+                    errorTypeString = errorTypeName(errorInstance->errorType());
 
                 // .message/.line/.column/.sourceURL: HTML spec + Node/WebKit read
                 // OWN data descriptors only (an inherited or accessor .message is
@@ -1208,13 +1219,16 @@ class CloneSerializer : public CloneBase {
                     RETURN_IF_EXCEPTION(scope, false);
                     if (found && d.isDataDescriptor() && d.value()) {
                         message = d.value().toWTFString(m_lexicalGlobalObject);
+                        if (fieldUnreadable())
+                            message = String();
                         RETURN_IF_EXCEPTION(scope, false);
                     }
                 }
                 // Trigger ErrorInstance's lazy materialization up front so a throwing
                 // prepareStackTrace propagates here instead of tripping the exception
                 // assertion inside JSObject::getOwnPropertyDescriptor.
                 errorInstance->materializeErrorInfoIfNeeded(vm);
+                (void)fieldUnreadable();
                 RETURN_IF_EXCEPTION(scope, false);
                 {
                     JSC::PropertyDescriptor d;
@@ -1242,6 +1256,8 @@ class CloneSerializer : public CloneBase {
                 }
                 {
                     JSValue v = errorInstance->get(m_lexicalGlobalObject, vm.propertyNames->stack);
+                    if (fieldUnreadable())
+                        v = jsUndefined();
                     RETURN_IF_EXCEPTION(scope, false);
                     if (v.isString())
                         stack = v.toWTFString(m_lexicalGlobalObject);
@@ -4435,7 +4451,7 @@ ExceptionOr<Ref<SerializedScriptValue>> SerializedScriptValue::create(JSGlobalOb
     auto scope = DECLARE_THROW_SCOPE(vm);
 
     // Fast path optimization: for postMessage/structuredClone with pure strings and no transfers
-    const bool canUseFastPath = (context == SerializationContext::WorkerPostMessage || context == SerializationContext::WindowPostMessage || context == SerializationContext::Default)
+    const bool canUseFastPath = (context == SerializationContext::WorkerPostMessage || context == SerializationContext::WindowPostMessage || context == SerializationContext::Default || context == SerializationContext::WorkerErrorReport)
         && forStorage == SerializationForStorage::No
         && forTransfer == SerializationForCrossProcessTransfer::No
         && transferList.isEmpty()

src/jsc/bindings/webcore/SerializedScriptValue.h

@@ -88,7 +88,9 @@ enum class SerializationErrorMode { NonThrowing,
     Throwing };
 enum class SerializationContext { Default,
     WorkerPostMessage,
-    WindowPostMessage };
+    WindowPostMessage,
+    // A worker's uncaught error on its way to the parent's 'error' event: Error fields that cannot be read are left out.
+    WorkerErrorReport };
 enum class SerializationForStorage : bool { No,
     Yes };
 enum class SerializationForCrossProcessTransfer : bool { No,

src/jsc/bindings/webcore/WorkerMessagingProxy.cpp

@@ -439,7 +439,7 @@ bool WorkerMessagingProxy::postSerializedErrorToWorkerObject(Zig::GlobalObject&
     auto& vm = JSC::getVM(&workerGlobalObject);
     auto scope = DECLARE_TOP_EXCEPTION_SCOPE(vm);
 
-    auto serialized = SerializedScriptValue::create(workerGlobalObject, value, SerializationForStorage::No, SerializationErrorMode::NonThrowing);
+    auto serialized = SerializedScriptValue::create(workerGlobalObject, value, SerializationForStorage::No, SerializationErrorMode::NonThrowing, SerializationContext::WorkerErrorReport);
     CLEAR_IF_EXCEPTION(scope);
     if (!serialized)
         return false;

src/jsc/web_worker.rs

@@ -1199,11 +1199,10 @@ fn on_unhandled_rejection(
             ..Default::default()
         },
     );
-    if let Err(err) = format_result {
-        error_instance = global_object.take_exception(err);
-    }
-    // Formatting ran script; if this worker was terminated meanwhile there is no error to dispatch.
-    if error_instance.is_termination_exception() {
+    // A formatter throw costs only the text (the fallback payload), unless it is the termination.
+    if let Err(err) = format_result
+        && global_object.take_exception(err).is_termination_exception()
+    {
         return;
     }
     jsc::mark_binding();

test/js/node/worker_threads/worker_threads.test.ts

@@ -715,6 +715,77 @@ describe("error event", () => {
     expect(err).toBeInstanceOf(Error);
     expect(err.message).toMatch(/MessagePort \[EventTarget\] \{.*\}/s);
   });
+
+  // node (lib/internal/error_serdes.js) copies the error field by field, so a field that cannot be
+  // read costs that field, not the error: the listener always gets an Error carrying the message.
+  // Bun used to emit null when the error could not be printed, and an Error whose message was the
+  // whole rendered diagnostic (source lines included) when it could not be structured-cloned.
+  describe.concurrent("delivers an Error when the thrown error cannot be printed or cloned", () => {
+    // A row leaves out what is incidental to its shape.
+    const shapes: [shape: string, workerSource: string, expected: object][] = [
+      // Bun.inspect() throws on these; the error itself clones fine.
+      [
+        "throwing inspect.custom",
+        `const e = new TypeError("boom"); e.code = "E_INSPECT";
+         e[Symbol.for("nodejs.util.inspect.custom")] = () => { throw new Error("x"); };
+         throw e;`,
+        { name: "TypeError", message: "boom", code: "E_INSPECT", stack: expect.any(String) },
+      ],
+      [
+        "AggregateError whose errors is not iterable",
+        `const e = new AggregateError([new Error("inner")], "boom"); e.errors = 42; throw e;`,
+        { name: "Error", message: "boom", stack: expect.any(String) },
+      ],
+      // postMessage would refuse to clone these; the error report keeps the fields it can read.
+      [
+        "throwing stack getter",
+        `const e = new RangeError("boom"); e.code = "E_STACK";
+         Object.defineProperty(e, "stack", { get() { throw new Error("x"); } });
+         throw e;`,
+        { name: "RangeError", message: "boom", code: "E_STACK", stack: undefined },
+      ],
+      // With the name unreadable, the error's own constructor decides the type.
+      [
+        "throwing name getter",
+        `const e = new TypeError("boom");
+         Object.defineProperty(e, "name", { get() { throw new Error("x"); } });
+         throw e;`,
+        { name: "TypeError", message: "boom", stack: expect.any(String) },
+      ],
+      [
+        "Symbol name",
+        `const e = new TypeError("boom"); e.name = Symbol("name"); throw e;`,
+        { name: "TypeError", message: "boom" },
+      ],
+      // bun leaves the default stack text on the error when prepareStackTrace throws, and reports it.
+      [
+        "throwing Error.prepareStackTrace",
+        `Error.prepareStackTrace = () => { throw new Error("x"); }; throw new Error("boom");`,
+        { name: "Error", message: "boom", stack: expect.any(String) },
+      ],
+      [
+        "AggregateError with a throwing stack getter",
+        `const e = new AggregateError([new Error("inner")], "boom");
+         Object.defineProperty(e, "stack", { get() { throw new Error("x"); } });
+         throw e;`,
+        { name: "Error", message: "boom", stack: undefined },
+      ],
+    ];
+
+    test.each(shapes)("%s", async (_shape, workerSource, expected) => {
+      const worker = new Worker(workerSource, { eval: true });
+      const exited = new Promise<number>(resolve => worker.once("exit", resolve));
+      // 'error' precedes 'exit'; a worker that exits without one fails here instead of timing out.
+      const [err] = await Promise.race([
+        once(worker, "error"),
+        exited.then(code => Promise.reject(new Error(`worker exited with code ${code} without emitting 'error'`))),
+      ]);
+      await exited;
+
+      expect(err).toBeInstanceOf(Error);
+      expect({ name: err.name, message: err.message, code: err.code, stack: err.stack }).toMatchObject(expected);
+    });
+  });
 });
 
 describe("getHeapSnapshot", () => {