From faf35cbac8cc804851df8cd1228ad2242b7559a9 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 19:42:36 +0000 Subject: [PATCH 1/3] worker_threads: deliver an Error to 'error' listeners when the thrown error cannot be printed or cloned --- .../webcore/SerializedScriptValue.cpp | 25 +++++-- .../bindings/webcore/SerializedScriptValue.h | 4 +- .../bindings/webcore/WorkerMessagingProxy.cpp | 2 +- src/jsc/web_worker.rs | 9 ++- .../test-worker-error-stack-getter-throws.js | 22 ++++++ .../worker_threads/worker_threads.test.ts | 70 +++++++++++++++++++ 6 files changed, 121 insertions(+), 11 deletions(-) create mode 100644 test/js/node/test/parallel/test-worker-error-stack-getter-throws.js diff --git a/src/jsc/bindings/webcore/SerializedScriptValue.cpp b/src/jsc/bindings/webcore/SerializedScriptValue.cpp index a6ada2d00b66..daf30730f121 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,8 @@ class CloneSerializer : public CloneBase { // prepareStackTrace propagates here instead of tripping the exception // assertion inside JSObject::getOwnPropertyDescriptor. errorInstance->materializeErrorInfoIfNeeded(vm); + // A throwing prepareStackTrace still leaves the default text in `stack`; node drops the field, so skip it. + bool stackUnreadable = fieldUnreadable(); RETURN_IF_EXCEPTION(scope, false); { JSC::PropertyDescriptor d; @@ -1240,8 +1255,10 @@ class CloneSerializer : public CloneBase { sourceURL = d.value().toWTFString(m_lexicalGlobalObject); RETURN_IF_EXCEPTION(scope, false); } - { + if (!stackUnreadable) { 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 +4452,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/test/parallel/test-worker-error-stack-getter-throws.js b/test/js/node/test/parallel/test-worker-error-stack-getter-throws.js new file mode 100644 index 000000000000..108fa3f5143d --- /dev/null +++ b/test/js/node/test/parallel/test-worker-error-stack-getter-throws.js @@ -0,0 +1,22 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const { Worker } = require('worker_threads'); + +const w = new Worker( + `const fn = (err) => { + if (err.message === 'fhqwhgads') + throw new Error('come on'); + return 'This is my custom stack trace!'; + }; + Error.prepareStackTrace = fn; + throw new Error('fhqwhgads'); + `, + { eval: true } +); +w.on('message', common.mustNotCall()); +w.on('error', common.mustCall((err) => { + assert.strictEqual(err.stack, undefined); + assert.strictEqual(err.message, 'fhqwhgads'); + assert.strictEqual(err.name, 'Error'); +})); diff --git a/test/js/node/worker_threads/worker_threads.test.ts b/test/js/node/worker_threads/worker_threads.test.ts index cc7acb16c723..5852d596e31d 100644 --- a/test/js/node/worker_threads/worker_threads.test.ts +++ b/test/js/node/worker_threads/worker_threads.test.ts @@ -715,6 +715,76 @@ 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" }, + ], + [ + "throwing Error.prepareStackTrace", + `Error.prepareStackTrace = () => { throw new Error("x"); }; throw new Error("boom");`, + { name: "Error", message: "boom", stack: undefined }, + ], + [ + "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", () => { From 5682f31e6a34b5b34ef0af080637d46471d9e154 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 21:18:05 +0000 Subject: [PATCH 2/3] ci: retrigger From fdf588649eb7f5122a43a287b3feb2080228a5cf Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 22:48:19 +0000 Subject: [PATCH 3/3] worker_threads: report the stack bun leaves behind after a throwing prepareStackTrace Whether Error.prepareStackTrace runs during the report depends on whether a GC already turned the error's frames into a stack string, so dropping the field only when it throws was nondeterministic. Carry whatever stack the error has, which is a string either way, and drop the vendored upstream test that pins undefined. --- .../webcore/SerializedScriptValue.cpp | 5 ++--- .../test-worker-error-stack-getter-throws.js | 22 ------------------- .../worker_threads/worker_threads.test.ts | 3 ++- 3 files changed, 4 insertions(+), 26 deletions(-) delete mode 100644 test/js/node/test/parallel/test-worker-error-stack-getter-throws.js diff --git a/src/jsc/bindings/webcore/SerializedScriptValue.cpp b/src/jsc/bindings/webcore/SerializedScriptValue.cpp index daf30730f121..651ec2028e91 100644 --- a/src/jsc/bindings/webcore/SerializedScriptValue.cpp +++ b/src/jsc/bindings/webcore/SerializedScriptValue.cpp @@ -1228,8 +1228,7 @@ class CloneSerializer : public CloneBase { // prepareStackTrace propagates here instead of tripping the exception // assertion inside JSObject::getOwnPropertyDescriptor. errorInstance->materializeErrorInfoIfNeeded(vm); - // A throwing prepareStackTrace still leaves the default text in `stack`; node drops the field, so skip it. - bool stackUnreadable = fieldUnreadable(); + (void)fieldUnreadable(); RETURN_IF_EXCEPTION(scope, false); { JSC::PropertyDescriptor d; @@ -1255,7 +1254,7 @@ class CloneSerializer : public CloneBase { sourceURL = d.value().toWTFString(m_lexicalGlobalObject); RETURN_IF_EXCEPTION(scope, false); } - if (!stackUnreadable) { + { JSValue v = errorInstance->get(m_lexicalGlobalObject, vm.propertyNames->stack); if (fieldUnreadable()) v = jsUndefined(); diff --git a/test/js/node/test/parallel/test-worker-error-stack-getter-throws.js b/test/js/node/test/parallel/test-worker-error-stack-getter-throws.js deleted file mode 100644 index 108fa3f5143d..000000000000 --- a/test/js/node/test/parallel/test-worker-error-stack-getter-throws.js +++ /dev/null @@ -1,22 +0,0 @@ -'use strict'; -const common = require('../common'); -const assert = require('assert'); -const { Worker } = require('worker_threads'); - -const w = new Worker( - `const fn = (err) => { - if (err.message === 'fhqwhgads') - throw new Error('come on'); - return 'This is my custom stack trace!'; - }; - Error.prepareStackTrace = fn; - throw new Error('fhqwhgads'); - `, - { eval: true } -); -w.on('message', common.mustNotCall()); -w.on('error', common.mustCall((err) => { - assert.strictEqual(err.stack, undefined); - assert.strictEqual(err.message, 'fhqwhgads'); - assert.strictEqual(err.name, 'Error'); -})); diff --git a/test/js/node/worker_threads/worker_threads.test.ts b/test/js/node/worker_threads/worker_threads.test.ts index 5852d596e31d..c330884162bc 100644 --- a/test/js/node/worker_threads/worker_threads.test.ts +++ b/test/js/node/worker_threads/worker_threads.test.ts @@ -757,10 +757,11 @@ describe("error event", () => { `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: undefined }, + { name: "Error", message: "boom", stack: expect.any(String) }, ], [ "AggregateError with a throwing stack getter",