diff --git a/src/jsc/bindings/webcore/SerializedScriptValue.cpp b/src/jsc/bindings/webcore/SerializedScriptValue.cpp index a6ada2d00b66..651ec2028e91 100644 --- a/src/jsc/bindings/webcore/SerializedScriptValue.cpp +++ b/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,6 +1219,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); } } @@ -1215,6 +1228,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; @@ -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> 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() diff --git a/src/jsc/bindings/webcore/SerializedScriptValue.h b/src/jsc/bindings/webcore/SerializedScriptValue.h index 78347c7980c3..bcc996c9dbff 100644 --- a/src/jsc/bindings/webcore/SerializedScriptValue.h +++ b/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, diff --git a/src/jsc/bindings/webcore/WorkerMessagingProxy.cpp b/src/jsc/bindings/webcore/WorkerMessagingProxy.cpp index 22d8e944916c..43fb7dc108d3 100644 --- a/src/jsc/bindings/webcore/WorkerMessagingProxy.cpp +++ b/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; diff --git a/src/jsc/web_worker.rs b/src/jsc/web_worker.rs index 114ad0091bff..0d9e62682a69 100644 --- a/src/jsc/web_worker.rs +++ b/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(); diff --git a/test/js/node/worker_threads/worker_threads.test.ts b/test/js/node/worker_threads/worker_threads.test.ts index cc7acb16c723..c330884162bc 100644 --- a/test/js/node/worker_threads/worker_threads.test.ts +++ b/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(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", () => {