From f9197e0e4f68f8b1b912a9ac84b7a863247bbf75 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 26 Oct 2026 12:22:21 +0000 Subject: [PATCH] worker_threads: deliver a thrown Error whose fields cannot be read The worker error report clones the thrown value for the parent's 'error' event. The clone's ErrorInstance arm fails as a whole on the first field read that throws: a throwing `stack` or `name` getter, a Symbol `name` or `message`, a throwing Error.prepareStackTrace. The parent then gets an Error whose message is the rendered text. With SerializationContext::WorkerErrorReport the arm leaves out a field whose read throws. The report passes that context only when the thrown value is itself an Error. postMessage and structuredClone do not pass it and still throw. --- .../webcore/SerializedScriptValue.cpp | 20 +++++- .../bindings/webcore/SerializedScriptValue.h | 4 +- .../bindings/webcore/WorkerMessagingProxy.cpp | 5 +- .../worker_threads/worker_threads.test.ts | 65 +++++++++++++++++++ 4 files changed, 90 insertions(+), 4 deletions(-) diff --git a/src/jsc/bindings/webcore/SerializedScriptValue.cpp b/src/jsc/bindings/webcore/SerializedScriptValue.cpp index 034b51ccaf9f..cfd0e3ab67a2 100644 --- a/src/jsc/bindings/webcore/SerializedScriptValue.cpp +++ b/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,6 +1201,8 @@ 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); } } @@ -1197,6 +1210,7 @@ class CloneSerializer : public CloneBase { // 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); diff --git a/src/jsc/bindings/webcore/SerializedScriptValue.h b/src/jsc/bindings/webcore/SerializedScriptValue.h index 89c44513d1c8..3bdae1d9b0bc 100644 --- a/src/jsc/bindings/webcore/SerializedScriptValue.h +++ b/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, diff --git a/src/jsc/bindings/webcore/WorkerMessagingProxy.cpp b/src/jsc/bindings/webcore/WorkerMessagingProxy.cpp index e578ade0c3f9..f6db849c346d 100644 --- a/src/jsc/bindings/webcore/WorkerMessagingProxy.cpp +++ b/src/jsc/bindings/webcore/WorkerMessagingProxy.cpp @@ -39,6 +39,7 @@ #include "VectorSizeLimit.h" #include "Worker.h" #include "ZigGlobalObject.h" +#include #include #include @@ -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(value) ? SerializationContext::WorkerErrorReport : SerializationContext::Default; + auto serialized = SerializedScriptValue::create(workerGlobalObject, value, SerializationForStorage::No, SerializationErrorMode::NonThrowing, context); CLEAR_IF_EXCEPTION(scope); if (!serialized) return false; diff --git a/test/js/node/worker_threads/worker_threads.test.ts b/test/js/node/worker_threads/worker_threads.test.ts index 654d54aac7c3..b89f22b57316 100644 --- a/test/js/node/worker_threads/worker_threads.test.ts +++ b/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(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 */ `