From e110ebbb685c54fca71dfb0854e4353313357738 Mon Sep 17 00:00:00 2001 From: Adrian Niculescu <15037449+adrian-niculescu@users.noreply.github.com> Date: Tue, 6 Oct 2026 19:19:29 +0300 Subject: [PATCH 1/4] fix(worker): route parentPort listener throws and handled Node-style errors like the web surface A parentPort listener that threw was dispatched without rethrowing, so the error went to the uncaught-error reporter and never reached the worker's onerror or its parent. The relay now dispatches the way worker-global message delivery does. The worker_threads Worker's onerror handler returned nothing, so an error its 'error' listeners took was also reported to the parent's global scope as unhandled. It returns whether a listener ran, which cancels the event. --- .../messaging/parentPortThrowingWorker.js | 4 ++ .../main/assets/app/tests/testMessaging.js | 42 +++++++++++++++++++ .../src/main/cpp/js/node-worker-threads.js | 18 +++++--- 3 files changed, 58 insertions(+), 6 deletions(-) create mode 100644 test-app/app/src/main/assets/app/tests/messaging/parentPortThrowingWorker.js diff --git a/test-app/app/src/main/assets/app/tests/messaging/parentPortThrowingWorker.js b/test-app/app/src/main/assets/app/tests/messaging/parentPortThrowingWorker.js new file mode 100644 index 000000000..7e2f08c34 --- /dev/null +++ b/test-app/app/src/main/assets/app/tests/messaging/parentPortThrowingWorker.js @@ -0,0 +1,4 @@ +var parentPort = require("node:worker_threads").parentPort; +parentPort.on("message", function () { + throw new Error("thrown by a parentPort listener"); +}); diff --git a/test-app/app/src/main/assets/app/tests/testMessaging.js b/test-app/app/src/main/assets/app/tests/testMessaging.js index c06a46037..ffd77bfae 100644 --- a/test-app/app/src/main/assets/app/tests/testMessaging.js +++ b/test-app/app/src/main/assets/app/tests/testMessaging.js @@ -255,6 +255,48 @@ describe("Messaging runtime edges", function () { } }; }); + + it("lets a node:worker_threads 'error' listener consume the error", function (done) { + var wt = require("node:worker_threads"); + var globalErrors = []; + var listener = function (event) { + globalErrors.push(event.message); + event.preventDefault(); + }; + addEventListener("error", listener); + var worker = new wt.Worker("~/tests/messaging/throwingWorker.js"); + worker.on("error", function (error) { + setTimeout(function () { + removeEventListener("error", listener); + expect(error.message).toContain("boom from worker"); + expect(globalErrors).toEqual([]); + worker.terminate(); + done(); + }, SETTLE); + }); + }); + + it("routes a throw from a parentPort listener to the parent's 'error' listeners", function (done) { + var wt = require("node:worker_threads"); + var worker = new wt.Worker("~/tests/messaging/parentPortThrowingWorker.js"); + var messages = []; + var finish = function () { + expect(messages.length).toBe(1); + expect(messages[0]).toContain("thrown by a parentPort listener"); + worker.terminate(); + done(); + }; + // Nothing else settles the spec when the error never arrives. + var guard = setTimeout(finish, 10000); + worker.on("error", function (error) { + messages.push(error.message); + if (messages.length === 1) { + clearTimeout(guard); + setTimeout(finish, SETTLE); + } + }); + worker.postMessage("go"); + }); }); describe("AbortSignal handler attribute accounting", function () { diff --git a/test-app/runtime/src/main/cpp/js/node-worker-threads.js b/test-app/runtime/src/main/cpp/js/node-worker-threads.js index da60893b3..094ea397e 100644 --- a/test-app/runtime/src/main/cpp/js/node-worker-threads.js +++ b/test-app/runtime/src/main/cpp/js/node-worker-threads.js @@ -44,6 +44,7 @@ const { BroadcastChannel } = require("internal/broadcast-channel"); const { EventTarget, defineEventHandler, + dispatchEventRethrowing, globalEventTarget, } = require("internal/events"); @@ -63,7 +64,6 @@ const globalPostMessage = g.postMessage; const addEventListener = EventTarget.prototype.addEventListener; const removeEventListener = EventTarget.prototype.removeEventListener; -const dispatchEvent = EventTarget.prototype.dispatchEvent; // Runs `fn` after the caller returns. Node reports 'online' and 'exit' from // the thread's own lifecycle; the runtime's Worker has no equivalent signal, @@ -123,10 +123,11 @@ class WorkerEmitter { return this.removeListener(type, listener); } + // Whether a listener was registered, as Node's EventEmitter reports it. emit(type, arg) { const list = this.#listeners[type]; - if (list === undefined) { - return; + if (list === undefined || list.length === 0) { + return false; } const snapshot = ArrayPrototypeSlice(list); for (let i = 0; i < snapshot.length; i++) { @@ -139,6 +140,7 @@ class WorkerEmitter { } FunctionPrototypeCall(entry.listener, this, arg); } + return true; } } @@ -179,8 +181,10 @@ class Worker extends WorkerEmitter { worker.onmessageerror = function (event) { self.emit("messageerror", event.data); }; + // A truthy return cancels the error, so one an 'error' listener took is + // not reported to the parent's global scope as well. worker.onerror = function (error) { - self.emit("error", error); + return self.emit("error", error); }; soon(function () { self.emit("online", undefined); @@ -312,9 +316,11 @@ ObjectDefineProperty(ParentPort.prototype, SymbolToStringTag, { let parentPort = null; if (!isMainThread) { parentPort = new ParentPort(); + // Rethrowing, so a listener that throws reaches the worker's error chain + // (the scope's onerror, then the parent's Worker) the way a throwing + // onmessage on the global scope does. const relay = function (event) { - FunctionPrototypeCall( - dispatchEvent, + dispatchEventRethrowing( parentPort, new (getMessageEvent())(event.type, { data: event.data, ports: event.ports }) ); From 1d9337ccd4762e98384f69b55784e2b14bf66f80 Mon Sep 17 00:00:00 2001 From: Adrian Niculescu <15037449+adrian-niculescu@users.noreply.github.com> Date: Tue, 6 Oct 2026 20:10:33 +0300 Subject: [PATCH 2/4] fix(worker): fire a worker_threads once listener at most once under a reentrant emit An earlier listener that emitted the same event again let the nested emit fire and remove a later once listener, and the outer emit then called it a second time from its snapshot. A once registration now records that it fired, as Node's once wrapper does. --- .../src/main/assets/app/tests/testMessaging.js | 17 +++++++++++++++++ .../src/main/cpp/js/node-worker-threads.js | 9 ++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/test-app/app/src/main/assets/app/tests/testMessaging.js b/test-app/app/src/main/assets/app/tests/testMessaging.js index ffd77bfae..2b1e72917 100644 --- a/test-app/app/src/main/assets/app/tests/testMessaging.js +++ b/test-app/app/src/main/assets/app/tests/testMessaging.js @@ -276,6 +276,23 @@ describe("Messaging runtime edges", function () { }); }); + it("calls a node:worker_threads once listener once when an earlier listener emits again", function () { + var wt = require("node:worker_threads"); + var worker = new wt.Worker("~/tests/eventLoopEchoWorker.js"); + var calls = 0; + var nested = false; + worker.on("probe", function () { + if (!nested) { + nested = true; + worker.emit("probe"); + } + }); + worker.once("probe", function () { calls++; }); + worker.emit("probe"); + worker.terminate(); + expect(calls).toBe(1); + }); + it("routes a throw from a parentPort listener to the parent's 'error' listeners", function (done) { var wt = require("node:worker_threads"); var worker = new wt.Worker("~/tests/messaging/parentPortThrowingWorker.js"); diff --git a/test-app/runtime/src/main/cpp/js/node-worker-threads.js b/test-app/runtime/src/main/cpp/js/node-worker-threads.js index 094ea397e..11ddb1373 100644 --- a/test-app/runtime/src/main/cpp/js/node-worker-threads.js +++ b/test-app/runtime/src/main/cpp/js/node-worker-threads.js @@ -100,7 +100,7 @@ class WorkerEmitter { } const key = `${type}`; const list = this.#listeners[key] || (this.#listeners[key] = []); - ArrayPrototypePush(list, { listener, once: true }); + ArrayPrototypePush(list, { listener, once: true, fired: false }); return this; } @@ -133,6 +133,13 @@ class WorkerEmitter { for (let i = 0; i < snapshot.length; i++) { const entry = snapshot[i]; if (entry.once) { + // Node's once wrapper: a registration fires at most once, even when + // an earlier listener emits the same event again and the nested emit + // fires it first. + if (entry.fired) { + continue; + } + entry.fired = true; const index = ArrayPrototypeIndexOf(list, entry); if (index !== -1) { ArrayPrototypeSplice(list, index, 1); From 7cca6e9aeba310a3b890fd4147eb4ab2e89b28a7 Mon Sep 17 00:00:00 2001 From: Adrian Niculescu <15037449+adrian-niculescu@users.noreply.github.com> Date: Thu, 8 Oct 2026 16:04:23 +0300 Subject: [PATCH 3/4] fix(worker): hand worker_threads 'error' listeners an Error rebuilt from the thrown value A worker_threads 'error' listener received the runtime's ErrorEvent, not an Error, and the message it carried was V8's "Uncaught Error: ..." text. The worker now reads the thrown value's name and message and forwards them with the error payload. The parent rebuilds an Error from them with the worker's stack, using the built-in constructor the name belongs to, and the parent's global error event carries that same Error when nothing handled it. The Worker shim cancels an error its listeners took with preventDefault() rather than through the truthy-return contract of onerror. --- docs/ns-builtin-modules.md | 11 +-- docs/worker-threads.md | 18 ++++- eslint.config.mjs | 2 +- .../messaging/domExceptionThrowingWorker.js | 1 + .../messaging/parentPortThrowingWorker.js | 2 +- .../main/assets/app/tests/testMessaging.js | 30 +++++-- .../runtime/src/main/cpp/CallbackHandlers.cpp | 6 +- .../src/main/cpp/NativeScriptException.cpp | 4 +- .../runtime/src/main/cpp/NsBuiltinModules.cpp | 1 + .../runtime/src/main/cpp/WorkerEvents.cpp | 24 +++--- test-app/runtime/src/main/cpp/WorkerEvents.h | 24 +++--- .../runtime/src/main/cpp/WorkerWrapper.cpp | 81 ++++++++++++++----- test-app/runtime/src/main/cpp/WorkerWrapper.h | 17 ++-- .../src/main/cpp/js/node-worker-threads.js | 12 ++- .../runtime/src/main/cpp/js/primordials.js | 4 + .../runtime/src/main/cpp/js/worker-events.js | 70 ++++++++++++++-- 16 files changed, 231 insertions(+), 76 deletions(-) create mode 100644 test-app/app/src/main/assets/app/tests/messaging/domExceptionThrowingWorker.js diff --git a/docs/ns-builtin-modules.md b/docs/ns-builtin-modules.md index 24f01c771..fde20fdc7 100644 --- a/docs/ns-builtin-modules.md +++ b/docs/ns-builtin-modules.md @@ -732,12 +732,13 @@ resolvers read the same table differently: Seven rows are public today: `ns:module`, `ns:runtime`, `ns:util`, `node:module`, `node:url`, `node:util`, `node:worker_threads`. - The **internal require** builtins receive (previous section) is the only - thing that can name an internal-only row. Five rows are marked that way: + thing that can name an internal-only row. Six rows are marked that way: `internal/broadcast-channel`, `internal/dom-exception`, `internal/events`, - `internal/message-channel`, `internal/message-event`. Their exports carry - capabilities app code must not hold — listener-accounting hook keys, the - error-reporter setter, base classes that must be the runtime's own rather - than whatever a global currently names. + `internal/message-channel`, `internal/message-event`, + `internal/worker-events`. Their exports carry capabilities app code must not + hold — listener-accounting hook keys, the error-reporter setter, the key a + worker's rebuilt error travels under, base classes that must be the + runtime's own rather than whatever a global currently names. - Builtins with **no row at all** (the intrinsics snapshot, the require factory, the console formatter) are invoked straight from their native call sites. There is no specifier that could reach them and nothing to mark. diff --git a/docs/worker-threads.md b/docs/worker-threads.md index 35078728b..95c23c6ca 100644 --- a/docs/worker-threads.md +++ b/docs/worker-threads.md @@ -54,7 +54,7 @@ means deliberately unsupported. | `threadName` | shim | Always `undefined`. | | `workerData` | shim | Always `null` — see below. | | `parentPort` | shim | `null` on the main isolate. Inside a worker, a `MessagePort`-shaped `EventTarget` over the worker's existing parent channel: `postMessage` forwards to the global `postMessage`, `message`/`messageerror` are re-dispatched from the worker global scope, `start()` and `close()` are no-ops. It is **not** a real port: not transferable, no queue of its own. | -| `Worker` | shim | A class over the runtime's global `Worker` with a small Node-style emitter (`on`/`once`/`off`/`removeListener`) for `message`, `messageerror`, `error`, `online` and `exit`. `postMessage(value, transfer)` and `terminate()` forward. `online` is emitted off a microtask after construction, not from the thread. Unsupported options throw a `TypeError` naming the option: `workerData`, `env`, `eval`, `transferList`, and `stdin`/`stdout`/`stderr` when explicitly truthy. The runtime's own `Worker` options (`androidPriority`) ride along untouched — the native constructor ignores keys it does not know. | +| `Worker` | shim | A class over the runtime's global `Worker` with a small Node-style emitter (`on`/`once`/`off`/`removeListener`) for `message`, `messageerror`, `error`, `online` and `exit`. An `error` listener receives the worker's error rebuilt as an `Error` (see below). `postMessage(value, transfer)` and `terminate()` forward. `online` is emitted off a microtask after construction, not from the thread. Unsupported options throw a `TypeError` naming the option: `workerData`, `env`, `eval`, `transferList`, and `stdin`/`stdout`/`stderr` when explicitly truthy. The runtime's own `Worker` options (`androidPriority`) ride along untouched — the native constructor ignores keys it does not know. | | `postMessageToThread` | throws | `Error: postMessageToThread is not supported in this runtime`. | | `moveMessagePortToContext` | throws | `Error: moveMessagePortToContext is not supported in this runtime`. | | `locks` | absent | Web Locks are not implemented; the property does not exist. | @@ -79,6 +79,22 @@ finished. `terminate()` therefore resolves with `0` and emits `exit` with code `0` on the way, and that is the only path that emits it. A worker that ends by its own `close()` produces no `exit`. +### An `error` listener receives a rebuilt `Error` + +Node hands an `error` listener the worker's error deserialized on the parent. +Only strings cross the isolate boundary here, so the parent rebuilds it from +the thrown value's `name` and `message`, with the worker's stack as its +`stack`. A built-in name (`TypeError`, `RangeError`, …) rebuilds with that +constructor, so `instanceof` holds; any other name, a subclass's or a +`DOMException`'s, is an own `name` on an `Error`. Other properties, such as a +`code` or a `cause`, are not carried. A thrown value with no string `message`, +such as a string or a number, arrives as an `Error` whose message is its string +form, where Node hands over the value itself. + +Once an `error` listener has received it, the error counts as handled and is +not reported on the parent's global scope. With no `error` listener, the +parent's global `error` event carries the same rebuilt `Error`. + ### A worker error carries no `error` object, and the worker scope's `onerror` is not an event An error the worker scope leaves unhandled reaches the parent as a real diff --git a/eslint.config.mjs b/eslint.config.mjs index f467cd4d2..7804b5e45 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -32,7 +32,7 @@ const capturedStatics = [ // Captured constructors. A destructure from `primordials` shadows the global, // so these only fire on the unguarded reference. -const restrictedGlobals = ['Date', 'FinalizationRegistry', 'Map', 'Number', 'Promise', 'Proxy', 'RangeError', 'Set', 'String', 'TypeError', 'Uint8Array', 'Uint32Array', 'WeakRef', 'WeakSet'].map((name) => ({ +const restrictedGlobals = ['Date', 'EvalError', 'FinalizationRegistry', 'Map', 'Number', 'Promise', 'Proxy', 'RangeError', 'ReferenceError', 'Set', 'String', 'SyntaxError', 'TypeError', 'Uint8Array', 'Uint32Array', 'URIError', 'WeakRef', 'WeakSet'].map((name) => ({ name, message: `Destructure ${name} from primordials — builtins must not read intrinsics off globals user code can replace.`, })); diff --git a/test-app/app/src/main/assets/app/tests/messaging/domExceptionThrowingWorker.js b/test-app/app/src/main/assets/app/tests/messaging/domExceptionThrowingWorker.js new file mode 100644 index 000000000..e280aff0c --- /dev/null +++ b/test-app/app/src/main/assets/app/tests/messaging/domExceptionThrowingWorker.js @@ -0,0 +1 @@ +throw new DOMException("aborted in a worker", "AbortError"); diff --git a/test-app/app/src/main/assets/app/tests/messaging/parentPortThrowingWorker.js b/test-app/app/src/main/assets/app/tests/messaging/parentPortThrowingWorker.js index 7e2f08c34..3505b8e3c 100644 --- a/test-app/app/src/main/assets/app/tests/messaging/parentPortThrowingWorker.js +++ b/test-app/app/src/main/assets/app/tests/messaging/parentPortThrowingWorker.js @@ -1,4 +1,4 @@ var parentPort = require("node:worker_threads").parentPort; parentPort.on("message", function () { - throw new Error("thrown by a parentPort listener"); + throw new TypeError("thrown by a parentPort listener"); }); diff --git a/test-app/app/src/main/assets/app/tests/testMessaging.js b/test-app/app/src/main/assets/app/tests/testMessaging.js index 2b1e72917..d5ab776cf 100644 --- a/test-app/app/src/main/assets/app/tests/testMessaging.js +++ b/test-app/app/src/main/assets/app/tests/testMessaging.js @@ -232,6 +232,7 @@ describe("Messaging runtime edges", function () { expect(seen.length).toBe(1); expect(seen[0].message).toContain("boom from worker"); expect(seen[0].error instanceof Error).toBe(true); + expect(seen[0].error.message).toBe("boom from worker"); worker.terminate(); done(); }; @@ -268,7 +269,10 @@ describe("Messaging runtime edges", function () { worker.on("error", function (error) { setTimeout(function () { removeEventListener("error", listener); - expect(error.message).toContain("boom from worker"); + expect(error instanceof Error).toBe(true); + expect(error.name).toBe("Error"); + expect(error.message).toBe("boom from worker"); + expect(error.stack).toContain("boom from worker"); expect(globalErrors).toEqual([]); worker.terminate(); done(); @@ -276,6 +280,18 @@ describe("Messaging runtime edges", function () { }); }); + it("hands a node:worker_threads 'error' listener a thrown DOMException's name and message", function (done) { + var wt = require("node:worker_threads"); + var worker = new wt.Worker("~/tests/messaging/domExceptionThrowingWorker.js"); + worker.on("error", function (error) { + expect(error instanceof Error).toBe(true); + expect(error.name).toBe("AbortError"); + expect(error.message).toBe("aborted in a worker"); + worker.terminate(); + done(); + }); + }); + it("calls a node:worker_threads once listener once when an earlier listener emits again", function () { var wt = require("node:worker_threads"); var worker = new wt.Worker("~/tests/eventLoopEchoWorker.js"); @@ -296,18 +312,20 @@ describe("Messaging runtime edges", function () { it("routes a throw from a parentPort listener to the parent's 'error' listeners", function (done) { var wt = require("node:worker_threads"); var worker = new wt.Worker("~/tests/messaging/parentPortThrowingWorker.js"); - var messages = []; + var errors = []; var finish = function () { - expect(messages.length).toBe(1); - expect(messages[0]).toContain("thrown by a parentPort listener"); + expect(errors.length).toBe(1); + expect(errors[0] instanceof TypeError).toBe(true); + expect(errors[0].name).toBe("TypeError"); + expect(errors[0].message).toBe("thrown by a parentPort listener"); worker.terminate(); done(); }; // Nothing else settles the spec when the error never arrives. var guard = setTimeout(finish, 10000); worker.on("error", function (error) { - messages.push(error.message); - if (messages.length === 1) { + errors.push(error); + if (errors.length === 1) { clearTimeout(guard); setTimeout(finish, SETTLE); } diff --git a/test-app/runtime/src/main/cpp/CallbackHandlers.cpp b/test-app/runtime/src/main/cpp/CallbackHandlers.cpp index 27e031c07..f7dae3c84 100644 --- a/test-app/runtime/src/main/cpp/CallbackHandlers.cpp +++ b/test-app/runtime/src/main/cpp/CallbackHandlers.cpp @@ -1902,14 +1902,16 @@ void CallbackHandlers::CallWorkerScopeOnErrorHandle(Isolate *isolate, TryCatch & // parent sees the handler's own error, and only that one. if (innerTc.HasCaught()) { ExtractTryCatchInfo(isolate, context, innerTc, message, source, stackTrace, lineno); - wrapper->PassUncaughtExceptionFromWorkerToParent(message, source, stackTrace, lineno); + wrapper->PassUncaughtExceptionFromWorkerToParent(message, source, stackTrace, lineno, + innerTc.Exception()); return; } // Unhandled at the worker scope - including when there is no scope // handler at all - so it becomes the parent's error event. ExtractTryCatchInfo(isolate, context, tc, message, source, stackTrace, lineno); - wrapper->PassUncaughtExceptionFromWorkerToParent(message, source, stackTrace, lineno); + wrapper->PassUncaughtExceptionFromWorkerToParent(message, source, stackTrace, lineno, + tc.Exception()); } catch (NativeScriptException &ex) { ex.ReThrowToV8(); } catch (std::exception e) { diff --git a/test-app/runtime/src/main/cpp/NativeScriptException.cpp b/test-app/runtime/src/main/cpp/NativeScriptException.cpp index 16fde7c03..2c4b57ebf 100644 --- a/test-app/runtime/src/main/cpp/NativeScriptException.cpp +++ b/test-app/runtime/src/main/cpp/NativeScriptException.cpp @@ -918,7 +918,9 @@ void PromiseRejectionTracker::Drain() { !workerWrapper->IsTerminating() && !workerWrapper->IsDisposed()) { string forwarded = message; string forwardedStack = stackTrace; + Local forwardedValue = reason; if (!thrown.IsEmpty()) { + forwardedValue = thrown; forwarded = ToDetailString(isolate, thrown); // The handler's own stack replaces the reason's; an accessor // that throws costs the stack, never the forward. @@ -930,7 +932,7 @@ void PromiseRejectionTracker::Drain() { } } workerWrapper->PassUncaughtExceptionFromWorkerToParent( - forwarded, "", forwardedStack, 0); + forwarded, "", forwardedStack, 0, forwardedValue); } } } else { diff --git a/test-app/runtime/src/main/cpp/NsBuiltinModules.cpp b/test-app/runtime/src/main/cpp/NsBuiltinModules.cpp index ee5e81f7a..69c8b5c49 100644 --- a/test-app/runtime/src/main/cpp/NsBuiltinModules.cpp +++ b/test-app/runtime/src/main/cpp/NsBuiltinModules.cpp @@ -68,6 +68,7 @@ constexpr Registration kRegistry[] = { {"internal/events", BuiltinId::kEvents, nullptr, true}, {"internal/message-channel", BuiltinId::kMessageChannel, messaging::CreateBinding, true}, {"internal/message-event", BuiltinId::kMessageEvent, nullptr, true}, + {"internal/worker-events", BuiltinId::kWorkerEvents, nullptr, true}, }; constexpr const char* kDebugKey = "debug"; diff --git a/test-app/runtime/src/main/cpp/WorkerEvents.cpp b/test-app/runtime/src/main/cpp/WorkerEvents.cpp index aea42802a..667328494 100644 --- a/test-app/runtime/src/main/cpp/WorkerEvents.cpp +++ b/test-app/runtime/src/main/cpp/WorkerEvents.cpp @@ -98,28 +98,28 @@ void WorkerEvents::EmitMessage(Isolate* isolate, Local receiver, (void)state->emitMessage.Get(isolate)->Call(context, receiver, 3, args).ToLocal(&result); } -bool WorkerEvents::EmitError(Isolate* isolate, Local receiver, - const std::string& message, const std::string& source, - const std::string& stackTrace, int lineNumber) { +MaybeLocal WorkerEvents::EmitError(Isolate* isolate, Local receiver, + const std::string& message, const std::string& source, + const std::string& stackTrace, int lineNumber, + const std::string& errorName, + const std::string& errorMessage) { auto* state = RuntimeState::For(isolate); if (state == nullptr || state->emitError.IsEmpty()) { - return false; + return MaybeLocal(); } Runtime* runtime = Runtime::TryGetRuntime(isolate); if (runtime == nullptr) { - return false; + return MaybeLocal(); } Local context = runtime->GetContext(); - Local args[4]{ArgConverter::ConvertToV8String(isolate, message), + Local args[6]{ArgConverter::ConvertToV8String(isolate, message), ArgConverter::ConvertToV8String(isolate, source), Number::New(isolate, lineNumber), - ArgConverter::ConvertToV8String(isolate, stackTrace)}; - Local result; - if (!state->emitError.Get(isolate)->Call(context, receiver, 4, args).ToLocal(&result)) { - return false; - } - return result->BooleanValue(isolate); + ArgConverter::ConvertToV8String(isolate, stackTrace), + ArgConverter::ConvertToV8String(isolate, errorName), + ArgConverter::ConvertToV8String(isolate, errorMessage)}; + return state->emitError.Get(isolate)->Call(context, receiver, 6, args); } } // namespace tns diff --git a/test-app/runtime/src/main/cpp/WorkerEvents.h b/test-app/runtime/src/main/cpp/WorkerEvents.h index 9a011df69..8df375280 100644 --- a/test-app/runtime/src/main/cpp/WorkerEvents.h +++ b/test-app/runtime/src/main/cpp/WorkerEvents.h @@ -39,16 +39,22 @@ class WorkerEvents { /* * Dispatches a cancelable `error` ErrorEvent on `receiver` (the Worker - * object, on the parent isolate) and returns whether a handler took - * ownership of it - either by returning truthy from the `onerror` - * attribute or by calling preventDefault(). Only primitives cross the - * isolate boundary, so the event carries no error object. A listener that - * throws leaves the exception pending for the caller's TryCatch and - * reports as unhandled. False before Init has run. + * object, on the parent isolate). Only primitives cross the isolate + * boundary, so the event carries no error object; the worker's error is + * rebuilt from `errorName`, `errorMessage` and `stackTrace`. Returns that + * error when no handler took ownership of the event, for the caller to + * report on the parent's global scope, and undefined when one did - either + * by returning truthy from the `onerror` attribute or by calling + * preventDefault(). Empty when a listener threw, which leaves the + * exception pending for the caller's TryCatch, and before Init has run. */ - static bool EmitError(v8::Isolate* isolate, v8::Local receiver, - const std::string& message, const std::string& source, - const std::string& stackTrace, int lineNumber); + static v8::MaybeLocal EmitError(v8::Isolate* isolate, + v8::Local receiver, + const std::string& message, + const std::string& source, + const std::string& stackTrace, int lineNumber, + const std::string& errorName, + const std::string& errorMessage); }; } // namespace tns diff --git a/test-app/runtime/src/main/cpp/WorkerWrapper.cpp b/test-app/runtime/src/main/cpp/WorkerWrapper.cpp index 7bc272b24..90f3e72c0 100644 --- a/test-app/runtime/src/main/cpp/WorkerWrapper.cpp +++ b/test-app/runtime/src/main/cpp/WorkerWrapper.cpp @@ -32,6 +32,39 @@ namespace tns { namespace { +/* + * The `name` and `message` the parent rebuilds the worker's error from, read + * off the value the worker threw. An object's `name` and `message` are taken + * when they are strings, so an Error or a DOMException keeps both; anything + * else becomes an Error whose message is the value's string form. Either + * property may be a getter, and one that throws leaves the default in place. + */ +void DescribeThrownValue(Isolate* isolate, Local context, Local thrown, + std::string& name, std::string& message) { + HandleScope handleScope(isolate); + TryCatch tc(isolate); + name = "Error"; + message.clear(); + Local detail; + if (thrown->ToDetailString(context).ToLocal(&detail)) { + message = ArgConverter::ConvertToString(detail); + } + if (!thrown->IsObject() || thrown->IsFunction()) { + return; + } + auto object = thrown.As(); + Local value; + if (object->Get(context, ArgConverter::ConvertToV8String(isolate, "name")).ToLocal(&value) && + value->IsString()) { + name = ArgConverter::ConvertToString(value.As()); + } + if (object->Get(context, ArgConverter::ConvertToV8String(isolate, "message")) + .ToLocal(&value) && + value->IsString()) { + message = ArgConverter::ConvertToString(value.As()); + } +} + /* * An uncaught exception from a JS callback the event loop's internal lane * drove. While a pump is on the stack, returning to Java is not the next act @@ -102,12 +135,12 @@ void ReportEntryRejection(Isolate* isolate, Local reason, thrownStack = NativeScriptException::GetErrorStackTrace(stack); } wrapper->PassUncaughtExceptionFromWorkerToParent( - ArgConverter::ToString(isolate, thrown), "", thrownStack, 0); + ArgConverter::ToString(isolate, thrown), "", thrownStack, 0, thrown); return; } } - wrapper->PassUncaughtExceptionFromWorkerToParent(message, "", stackTrace, 0); + wrapper->PassUncaughtExceptionFromWorkerToParent(message, "", stackTrace, 0, reason); } } // namespace @@ -374,19 +407,29 @@ void WorkerWrapper::FireMessageOnParentWorkerObject(int workerId, void WorkerWrapper::PassUncaughtExceptionFromWorkerToParent(const std::string& message, const std::string& filename, const std::string& stackTrace, - int lineno) { + int lineno, Local thrown) { auto parentTasks = parentTasks_.lock(); if (parentTasks == nullptr) { // the parent runtime is gone (e.g. a parent worker that shut down) return; } + // Read here, on the worker's isolate. A report with no thrown value, such + // as the heap-limit one made from inside a GC, touches no v8 handle. + std::string errorName = "Error"; + std::string errorMessage = message; + if (!thrown.IsEmpty()) { + Isolate* workerIsolate = Isolate::GetCurrent(); + DescribeThrownValue(workerIsolate, workerIsolate->GetCurrentContext(), thrown, errorName, + errorMessage); + } + int workerId = workerId_; std::string threadName = threadName_; Isolate* parentIsolate = parentIsolate_; parentTasks->PostInternal([workerId, message, filename, stackTrace, lineno, threadName, - parentIsolate]() { + parentIsolate, errorName, errorMessage]() { v8::Locker locker(parentIsolate); Isolate::Scope isolate_scope(parentIsolate); HandleScope handle_scope(parentIsolate); @@ -394,14 +437,17 @@ void WorkerWrapper::PassUncaughtExceptionFromWorkerToParent(const std::string& m Context::Scope context_scope(context); WorkerWrapper::FireErrorOnParentWorkerObject(workerId, message, stackTrace, filename, - lineno, threadName); + lineno, threadName, errorName, + errorMessage); }); } void WorkerWrapper::FireErrorOnParentWorkerObject(int workerId, const std::string& message, const std::string& stackTrace, const std::string& filename, int lineno, - const std::string& threadName) { + const std::string& threadName, + const std::string& errorName, + const std::string& errorMessage) { auto wrapper = WorkerWrapper::GetById(workerId); if (wrapper == nullptr) { DEBUG_WRITE("MAIN: no worker instance was found with workerId=%d.", workerId); @@ -419,11 +465,12 @@ void WorkerWrapper::FireErrorOnParentWorkerObject(int workerId, const std::strin } auto worker = Local::New(isolate, *wrapper->poWorker_); - auto context = Runtime::GetRuntime(isolate)->GetContext(); TryCatch tc(isolate); - bool handled = WorkerEvents::EmitError(isolate, worker, message, filename, stackTrace, - lineno); + Local error; + (void)WorkerEvents::EmitError(isolate, worker, message, filename, stackTrace, lineno, + errorName, errorMessage) + .ToLocal(&error); if (tc.HasCaught()) { // A listener that threw replaces the error it was handed; nothing // further is reported for the original. @@ -432,22 +479,14 @@ void WorkerWrapper::FireErrorOnParentWorkerObject(int workerId, const std::strin } return; } - if (handled) { + if (!error.IsEmpty() && error->IsUndefined()) { + // A handler took ownership of it. return; } // HTML: an error the Worker object leaves unhandled is reported to the - // parent's global scope. Only primitives crossed the isolate boundary, - // so the error object is rebuilt from them here. - Local error = - Exception::Error(ArgConverter::ConvertToV8String(isolate, message)); - if (error->IsObject() && !stackTrace.empty()) { - (void)error.As() - ->Set(context, ArgConverter::ConvertToV8String(isolate, "stack"), - ArgConverter::ConvertToV8String(isolate, stackTrace)) - .FromMaybe(false); - } - if (!ErrorEvents::DispatchError(isolate, error, message, stackTrace)) { + // parent's global scope. + if (error.IsEmpty() || !ErrorEvents::DispatchError(isolate, error, message, stackTrace)) { DEBUG_WRITE_FORCE( "Unhandled exception in '%s' thread. file: %s, line %d, message: %s\nStackTrace: %s", threadName.c_str(), filename.c_str(), lineno, message.c_str(), diff --git a/test-app/runtime/src/main/cpp/WorkerWrapper.h b/test-app/runtime/src/main/cpp/WorkerWrapper.h index 5dd3b2fa8..d5200e371 100644 --- a/test-app/runtime/src/main/cpp/WorkerWrapper.h +++ b/test-app/runtime/src/main/cpp/WorkerWrapper.h @@ -89,12 +89,15 @@ class WorkerWrapper : public std::enable_shared_from_this { /* * Posts the worker object's `onerror` invocation to the parent's thread. - * Strings only - must not hold any v8 handles from the worker isolate. + * `thrown` is the value the worker threw, when there is one: its name and + * message are read here, on the worker's isolate, for the parent to + * rebuild the error from. Without it the parent's error is an Error + * carrying `message`. Strings only cross to the parent's thread. */ - void PassUncaughtExceptionFromWorkerToParent(const std::string& message, - const std::string& filename, - const std::string& stackTrace, - int lineno); + void PassUncaughtExceptionFromWorkerToParent( + const std::string& message, const std::string& filename, + const std::string& stackTrace, int lineno, + v8::Local thrown = v8::Local()); /* * WHATWG parity: the worker's implicit port message queue starts disabled; @@ -172,7 +175,9 @@ class WorkerWrapper : public std::enable_shared_from_this { static void FireErrorOnParentWorkerObject(int workerId, const std::string& message, const std::string& stackTrace, const std::string& filename, int lineno, - const std::string& threadName); + const std::string& threadName, + const std::string& errorName, + const std::string& errorMessage); v8::Isolate* parentIsolate_; // The parent runtime's task queue; weak so a child outliving its parent diff --git a/test-app/runtime/src/main/cpp/js/node-worker-threads.js b/test-app/runtime/src/main/cpp/js/node-worker-threads.js index 11ddb1373..5ae799101 100644 --- a/test-app/runtime/src/main/cpp/js/node-worker-threads.js +++ b/test-app/runtime/src/main/cpp/js/node-worker-threads.js @@ -47,6 +47,7 @@ const { dispatchEventRethrowing, globalEventTarget, } = require("internal/events"); +const { kWorkerError } = require("internal/worker-events"); let MessageEvent; function getMessageEvent() { @@ -188,10 +189,13 @@ class Worker extends WorkerEmitter { worker.onmessageerror = function (event) { self.emit("messageerror", event.data); }; - // A truthy return cancels the error, so one an 'error' listener took is - // not reported to the parent's global scope as well. - worker.onerror = function (error) { - return self.emit("error", error); + // An 'error' listener receives the worker's error, as in Node, not the + // event. Once one has, the event is cancelled, so the error is not also + // reported to the parent's global scope. + worker.onerror = function (event) { + if (self.emit("error", event[kWorkerError])) { + event.preventDefault(); + } }; soon(function () { self.emit("online", undefined); diff --git a/test-app/runtime/src/main/cpp/js/primordials.js b/test-app/runtime/src/main/cpp/js/primordials.js index 53ac8fe94..b8bb6f2dd 100644 --- a/test-app/runtime/src/main/cpp/js/primordials.js +++ b/test-app/runtime/src/main/cpp/js/primordials.js @@ -22,16 +22,20 @@ const intrinsics = { // Constructors. Date, Error, + EvalError, FinalizationRegistry, Map, Number, Promise, RangeError, + ReferenceError, Set, String, + SyntaxError, TypeError, Uint8Array, Uint32Array, + URIError, URL, WeakRef, WeakSet, diff --git a/test-app/runtime/src/main/cpp/js/worker-events.js b/test-app/runtime/src/main/cpp/js/worker-events.js index 73536fc44..674c06aa9 100644 --- a/test-app/runtime/src/main/cpp/js/worker-events.js +++ b/test-app/runtime/src/main/cpp/js/worker-events.js @@ -9,7 +9,18 @@ // Eager, because the handler attributes have to exist before app code assigns // one. MessageEvent itself is pulled in on the first delivery, so a worker // nobody talks to never runs that builtin. -const { ObjectDefineProperty, ObjectSetPrototypeOf } = primordials; +const { + Error, + ErrorCaptureStackTrace, + EvalError, + ObjectDefineProperty, + ObjectSetPrototypeOf, + RangeError, + ReferenceError, + SyntaxError, + TypeError, + URIError, +} = primordials; const { EventTarget, @@ -52,15 +63,58 @@ function emitMessage(data, ports, type) { dispatchEventRethrowing(this, new MessageEventCtor(type, { data, ports })); } +// The key the worker's rebuilt error travels under on the `error` event, for +// node:worker_threads, whose listeners receive the error rather than the +// event. Reached only through require("internal/worker-events"). +const kWorkerError = Symbol("workerError"); + +// A built-in error name rebuilds with its own constructor, as Node's does, so +// `instanceof TypeError` holds on the parent. +const errorConstructors = { + __proto__: null, + Error, + EvalError, + RangeError, + ReferenceError, + SyntaxError, + TypeError, + URIError, +}; + +// The worker's error, rebuilt from the name and message the worker read off +// the thrown value. Any other name, a subclass's or a DOMException's, stays an +// own `name` on an Error. Without the worker's stack, the stack holds the +// header alone rather than the frames that rebuilt it. +function rebuildError(name, message, stack) { + const ErrorConstructor = errorConstructors[name]; + const error = + ErrorConstructor === undefined ? new Error(message) : new ErrorConstructor(message); + if (ErrorConstructor === undefined) { + ObjectDefineProperty(error, "name", { + __proto__: null, + value: name, + writable: true, + configurable: true, + }); + } + if (stack) { + error.stack = stack; + } else { + ErrorCaptureStackTrace(error, emitError); + } + return error; +} + // The parent-side error delivery callout, invoked by native with the Worker // object as `this` once the worker scope has left the error unhandled. Only // primitives cross the isolate boundary, so the event carries no `error` // object; `stackTrace` is this runtime's addition to the ErrorEvent fields. // -// Returns whether the error was handled: a truthy return from the `onerror` -// attribute cancels the event (HTML §8.1.7.3), as does preventDefault() from -// any listener. -function emitError(message, filename, lineno, stackTrace) { +// Returns the rebuilt error when the event was not handled, for native to +// report on the parent's global scope, and undefined when it was: a truthy +// return from the `onerror` attribute cancels the event (HTML §8.1.7.3), as +// does preventDefault() from any listener. +function emitError(message, filename, lineno, stackTrace, errorName, errorMessage) { const ErrorEventCtor = getErrorEvent(); const event = new ErrorEventCtor("error", { message, @@ -69,8 +123,10 @@ function emitError(message, filename, lineno, stackTrace) { cancelable: true, }); event.stackTrace = stackTrace; + const error = rebuildError(errorName, errorMessage, stackTrace); + ObjectDefineProperty(event, kWorkerError, { __proto__: null, value: error }); dispatchEventRethrowing(this, event); - return event.defaultPrevented; + return event.defaultPrevented ? undefined : error; } ObjectSetPrototypeOf(g.Worker.prototype, EventTarget.prototype); @@ -99,4 +155,4 @@ for (const name of ["onmessage", "onmessageerror"]) { }); } -module.exports = { emitMessage, emitError }; +module.exports = { emitMessage, emitError, kWorkerError }; From a2520fa94c185ac3a6606db3b72d27bb7e06d73a Mon Sep 17 00:00:00 2001 From: Adrian Niculescu <15037449+adrian-niculescu@users.noreply.github.com> Date: Thu, 8 Oct 2026 16:29:03 +0300 Subject: [PATCH 4/4] fix(worker): keep a forwarded worker error exact when its stack getter throws Reading the stack of the error a worker's onerror threw could run a `stack` getter, and a getter that threw replaced that error in the TryCatch holding it, so the parent rebuilt the getter's error instead. The stack read now runs under its own TryCatch. The thrown value's name and message travel to the parent as UTF-16, so an unpaired surrogate arrives as thrown rather than as U+FFFD. ArgConverter::ConvertToString copies by length, so an embedded NUL no longer cuts a converted string short. --- .../messaging/onerrorRethrowingWorker.js | 12 ++++ .../main/assets/app/tests/testMessaging.js | 12 ++++ test-app/runtime/src/main/cpp/ArgConverter.h | 2 +- .../runtime/src/main/cpp/CallbackHandlers.cpp | 3 + .../runtime/src/main/cpp/WorkerEvents.cpp | 8 +-- test-app/runtime/src/main/cpp/WorkerEvents.h | 4 +- .../runtime/src/main/cpp/WorkerWrapper.cpp | 68 +++++++++++++------ test-app/runtime/src/main/cpp/WorkerWrapper.h | 4 +- 8 files changed, 84 insertions(+), 29 deletions(-) create mode 100644 test-app/app/src/main/assets/app/tests/messaging/onerrorRethrowingWorker.js diff --git a/test-app/app/src/main/assets/app/tests/messaging/onerrorRethrowingWorker.js b/test-app/app/src/main/assets/app/tests/messaging/onerrorRethrowingWorker.js new file mode 100644 index 000000000..49e26dec8 --- /dev/null +++ b/test-app/app/src/main/assets/app/tests/messaging/onerrorRethrowingWorker.js @@ -0,0 +1,12 @@ +// The scope's onerror throws an error whose message holds a NUL and an +// unpaired surrogate, and whose stack getter throws. +onerror = function () { + var error = new TypeError("before\0after \uD800"); + Object.defineProperty(error, "stack", { + get: function () { throw new RangeError("thrown by the stack getter"); } + }); + throw error; +}; +onmessage = function () { + throw new Error("thrown by onmessage"); +}; diff --git a/test-app/app/src/main/assets/app/tests/testMessaging.js b/test-app/app/src/main/assets/app/tests/testMessaging.js index d5ab776cf..65188fe03 100644 --- a/test-app/app/src/main/assets/app/tests/testMessaging.js +++ b/test-app/app/src/main/assets/app/tests/testMessaging.js @@ -292,6 +292,18 @@ describe("Messaging runtime edges", function () { }); }); + it("rebuilds the error a worker's onerror threw exactly, even when its stack getter throws", function (done) { + var wt = require("node:worker_threads"); + var worker = new wt.Worker("~/tests/messaging/onerrorRethrowingWorker.js"); + worker.on("error", function (error) { + expect(error instanceof TypeError).toBe(true); + expect(error.message).toBe("before\0after \uD800"); + worker.terminate(); + done(); + }); + worker.postMessage("go"); + }); + it("calls a node:worker_threads once listener once when an earlier listener emits again", function () { var wt = require("node:worker_threads"); var worker = new wt.Worker("~/tests/eventLoopEchoWorker.js"); diff --git a/test-app/runtime/src/main/cpp/ArgConverter.h b/test-app/runtime/src/main/cpp/ArgConverter.h index 263a6ba3d..e8d941592 100644 --- a/test-app/runtime/src/main/cpp/ArgConverter.h +++ b/test-app/runtime/src/main/cpp/ArgConverter.h @@ -60,7 +60,7 @@ class ArgConverter { } else { auto isolate = v8::Isolate::GetCurrent(); v8::String::Utf8Value str(isolate, s); - return {*str}; + return {*str, static_cast(str.length())}; } } diff --git a/test-app/runtime/src/main/cpp/CallbackHandlers.cpp b/test-app/runtime/src/main/cpp/CallbackHandlers.cpp index f7dae3c84..7fd5d3d7f 100644 --- a/test-app/runtime/src/main/cpp/CallbackHandlers.cpp +++ b/test-app/runtime/src/main/cpp/CallbackHandlers.cpp @@ -1842,6 +1842,9 @@ static void ExtractTryCatchInfo(Isolate *isolate, Local context, TryCat } } + // `stack` may be an accessor. One that throws only costs the stack: caught + // here, its exception cannot replace the one `tc` holds. + TryCatch stackTc(isolate); Local outStackTrace = tc.StackTrace(context).FromMaybe(Local()); if (!outStackTrace.IsEmpty()) { Local stackTraceStr = diff --git a/test-app/runtime/src/main/cpp/WorkerEvents.cpp b/test-app/runtime/src/main/cpp/WorkerEvents.cpp index 667328494..2b5792e36 100644 --- a/test-app/runtime/src/main/cpp/WorkerEvents.cpp +++ b/test-app/runtime/src/main/cpp/WorkerEvents.cpp @@ -101,8 +101,8 @@ void WorkerEvents::EmitMessage(Isolate* isolate, Local receiver, MaybeLocal WorkerEvents::EmitError(Isolate* isolate, Local receiver, const std::string& message, const std::string& source, const std::string& stackTrace, int lineNumber, - const std::string& errorName, - const std::string& errorMessage) { + Local errorName, + Local errorMessage) { auto* state = RuntimeState::For(isolate); if (state == nullptr || state->emitError.IsEmpty()) { return MaybeLocal(); @@ -117,8 +117,8 @@ MaybeLocal WorkerEvents::EmitError(Isolate* isolate, Local receiv ArgConverter::ConvertToV8String(isolate, source), Number::New(isolate, lineNumber), ArgConverter::ConvertToV8String(isolate, stackTrace), - ArgConverter::ConvertToV8String(isolate, errorName), - ArgConverter::ConvertToV8String(isolate, errorMessage)}; + errorName, + errorMessage}; return state->emitError.Get(isolate)->Call(context, receiver, 6, args); } diff --git a/test-app/runtime/src/main/cpp/WorkerEvents.h b/test-app/runtime/src/main/cpp/WorkerEvents.h index 8df375280..8404370d6 100644 --- a/test-app/runtime/src/main/cpp/WorkerEvents.h +++ b/test-app/runtime/src/main/cpp/WorkerEvents.h @@ -53,8 +53,8 @@ class WorkerEvents { const std::string& message, const std::string& source, const std::string& stackTrace, int lineNumber, - const std::string& errorName, - const std::string& errorMessage); + v8::Local errorName, + v8::Local errorMessage); }; } // namespace tns diff --git a/test-app/runtime/src/main/cpp/WorkerWrapper.cpp b/test-app/runtime/src/main/cpp/WorkerWrapper.cpp index 90f3e72c0..ab4d74727 100644 --- a/test-app/runtime/src/main/cpp/WorkerWrapper.cpp +++ b/test-app/runtime/src/main/cpp/WorkerWrapper.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include "ArgConverter.h" @@ -33,36 +34,57 @@ namespace tns { namespace { /* - * The `name` and `message` the parent rebuilds the worker's error from, read - * off the value the worker threw. An object's `name` and `message` are taken - * when they are strings, so an Error or a DOMException keeps both; anything - * else becomes an Error whose message is the value's string form. Either - * property may be a getter, and one that throws leaves the default in place. + * The `name` and `message` the parent rebuilds the worker's error from. They + * stay UTF-16 on the way, so both arrive exactly as thrown, embedded NULs and + * unpaired surrogates included. */ -void DescribeThrownValue(Isolate* isolate, Local context, Local thrown, - std::string& name, std::string& message) { +struct ThrownErrorText { + std::u16string name = u"Error"; + std::u16string message; +}; + +std::u16string ToUtf16(Isolate* isolate, Local value) { + std::u16string result(value->Length(), u'\0'); + value->WriteV2(isolate, 0, value->Length(), reinterpret_cast(result.data())); + return result; +} + +Local FromUtf16(Isolate* isolate, const std::u16string& value) { + return String::NewFromTwoByte(isolate, reinterpret_cast(value.data()), + NewStringType::kNormal, static_cast(value.size())) + .ToLocalChecked(); +} + +/* + * Reads the text off the value the worker threw. An object's `name` and + * `message` are taken when they are strings, so an Error or a DOMException + * keeps both; anything else becomes an Error whose message is the value's + * string form. Either property may be a getter, and one that throws leaves the + * default in place. + */ +ThrownErrorText DescribeThrownValue(Isolate* isolate, Local context, Local thrown) { HandleScope handleScope(isolate); TryCatch tc(isolate); - name = "Error"; - message.clear(); + ThrownErrorText text; Local detail; if (thrown->ToDetailString(context).ToLocal(&detail)) { - message = ArgConverter::ConvertToString(detail); + text.message = ToUtf16(isolate, detail); } if (!thrown->IsObject() || thrown->IsFunction()) { - return; + return text; } auto object = thrown.As(); Local value; if (object->Get(context, ArgConverter::ConvertToV8String(isolate, "name")).ToLocal(&value) && value->IsString()) { - name = ArgConverter::ConvertToString(value.As()); + text.name = ToUtf16(isolate, value.As()); } if (object->Get(context, ArgConverter::ConvertToV8String(isolate, "message")) .ToLocal(&value) && value->IsString()) { - message = ArgConverter::ConvertToString(value.As()); + text.message = ToUtf16(isolate, value.As()); } + return text; } /* @@ -416,12 +438,10 @@ void WorkerWrapper::PassUncaughtExceptionFromWorkerToParent(const std::string& m // Read here, on the worker's isolate. A report with no thrown value, such // as the heap-limit one made from inside a GC, touches no v8 handle. - std::string errorName = "Error"; - std::string errorMessage = message; + std::optional thrownText; if (!thrown.IsEmpty()) { Isolate* workerIsolate = Isolate::GetCurrent(); - DescribeThrownValue(workerIsolate, workerIsolate->GetCurrentContext(), thrown, errorName, - errorMessage); + thrownText = DescribeThrownValue(workerIsolate, workerIsolate->GetCurrentContext(), thrown); } int workerId = workerId_; @@ -429,13 +449,21 @@ void WorkerWrapper::PassUncaughtExceptionFromWorkerToParent(const std::string& m Isolate* parentIsolate = parentIsolate_; parentTasks->PostInternal([workerId, message, filename, stackTrace, lineno, threadName, - parentIsolate, errorName, errorMessage]() { + parentIsolate, thrownText]() { v8::Locker locker(parentIsolate); Isolate::Scope isolate_scope(parentIsolate); HandleScope handle_scope(parentIsolate); auto context = Runtime::GetRuntime(parentIsolate)->GetContext(); Context::Scope context_scope(context); + // Without a thrown value, the error is an Error carrying the report's + // message. + Local errorName = thrownText + ? FromUtf16(parentIsolate, thrownText->name) + : ArgConverter::ConvertToV8String(parentIsolate, "Error"); + Local errorMessage = thrownText + ? FromUtf16(parentIsolate, thrownText->message) + : ArgConverter::ConvertToV8String(parentIsolate, message); WorkerWrapper::FireErrorOnParentWorkerObject(workerId, message, stackTrace, filename, lineno, threadName, errorName, errorMessage); @@ -446,8 +474,8 @@ void WorkerWrapper::FireErrorOnParentWorkerObject(int workerId, const std::strin const std::string& stackTrace, const std::string& filename, int lineno, const std::string& threadName, - const std::string& errorName, - const std::string& errorMessage) { + Local errorName, + Local errorMessage) { auto wrapper = WorkerWrapper::GetById(workerId); if (wrapper == nullptr) { DEBUG_WRITE("MAIN: no worker instance was found with workerId=%d.", workerId); diff --git a/test-app/runtime/src/main/cpp/WorkerWrapper.h b/test-app/runtime/src/main/cpp/WorkerWrapper.h index d5200e371..3068d69d2 100644 --- a/test-app/runtime/src/main/cpp/WorkerWrapper.h +++ b/test-app/runtime/src/main/cpp/WorkerWrapper.h @@ -176,8 +176,8 @@ class WorkerWrapper : public std::enable_shared_from_this { const std::string& stackTrace, const std::string& filename, int lineno, const std::string& threadName, - const std::string& errorName, - const std::string& errorMessage); + v8::Local errorName, + v8::Local errorMessage); v8::Isolate* parentIsolate_; // The parent runtime's task queue; weak so a child outliving its parent