worker_threads: deliver a thrown Error to 'error' listeners when one of its fields cannot be read

src/jsc/bindings/webcore/SerializedScriptValue.cpp

@@ -1169,10 +1169,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
@@ -1190,13 +1201,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;
@@ -1224,6 +1238,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);

src/jsc/bindings/webcore/SerializedScriptValue.h

@@ -87,7 +87,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

@@ -39,6 +39,7 @@
 #include "VectorSizeLimit.h"
 #include "Worker.h"
 #include "ZigGlobalObject.h"
+#include <JavaScriptCore/ErrorInstance.h>
 #include <JavaScriptCore/JSPromise.h>
 #include <wtf/TZoneMallocInlines.h>
 
@@ -471,7 +472,9 @@ 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);
+    // Only a thrown Error is read field by field, as in node's error_serdes.js. An Error inside a thrown value is cloned strictly.
+    auto context = dynamicDowncast<JSC::ErrorInstance>(value) ? SerializationContext::WorkerErrorReport : SerializationContext::Default;
+    auto serialized = SerializedScriptValue::create(workerGlobalObject, value, SerializationForStorage::No, SerializationErrorMode::NonThrowing, context);
     CLEAR_IF_EXCEPTION(scope);
     if (!serialized)
         return false;

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

@@ -721,6 +721,71 @@ describe("error event", () => {
     expect(err.message).toBe("oh no");
   });
 
+  // Like node (lib/internal/error_serdes.js): a field that cannot be read costs that field, not the Error.
+  describe.concurrent("an Error with a field that cannot be read", () => {
+    const errorEventOf = async (workerSource: string) => {
+      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;
+      return err;
+    };
+
+    // A row leaves out what is incidental to its shape.
+    test.each([
+      [
+        "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" },
+      ],
+      [
+        "Symbol name",
+        `const e = new TypeError("boom"); e.name = Symbol("name"); throw e;`,
+        { name: "TypeError", message: "boom" },
+      ],
+      // An unreadable message is left out, as node does.
+      [
+        "Symbol message",
+        `const e = new Error("boom"); e.message = Symbol("message"); throw e;`,
+        { name: "Error", message: "" },
+      ],
+      [
+        "throwing Error.prepareStackTrace",
+        `Error.prepareStackTrace = () => { throw new Error("x"); }; throw new Error("boom");`,
+        { name: "Error", message: "boom" },
+      ],
+    ])("is delivered without that field: %s", async (_shape, workerSource, expected) => {
+      const err = await errorEventOf(workerSource);
+      expect(err).toBeInstanceOf(Error);
+      expect({ name: err.name, message: err.message, code: err.code, stack: err.stack }).toMatchObject(expected);
+    });
+
+    // node reads only a thrown Error field by field. An Error inside a thrown value is cloned strictly.
+    test("inside a thrown object still takes the text fallback", async () => {
+      const err = await errorEventOf(
+        `const e = new RangeError("boom");
+         Object.defineProperty(e, "stack", { get() { throw new Error("x"); } });
+         throw { e };`,
+      );
+      expect(err).toBeInstanceOf(Error);
+      expect(err.message).toStartWith("{\n  e: ");
+    });
+  });
+
   test("falls back to string when the error cannot be serialized", async () => {
     const worker = new Worker(
       /* js */ `