Skip to content

Commit e110ebb

Browse files
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.
1 parent 42a8bcf commit e110ebb

3 files changed

Lines changed: 58 additions & 6 deletions

File tree

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
var parentPort = require("node:worker_threads").parentPort;
2+
parentPort.on("message", function () {
3+
throw new Error("thrown by a parentPort listener");
4+
});

‎test-app/app/src/main/assets/app/tests/testMessaging.js‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,48 @@ describe("Messaging runtime edges", function () {
255255
}
256256
};
257257
});
258+
259+
it("lets a node:worker_threads 'error' listener consume the error", function (done) {
260+
var wt = require("node:worker_threads");
261+
var globalErrors = [];
262+
var listener = function (event) {
263+
globalErrors.push(event.message);
264+
event.preventDefault();
265+
};
266+
addEventListener("error", listener);
267+
var worker = new wt.Worker("~/tests/messaging/throwingWorker.js");
268+
worker.on("error", function (error) {
269+
setTimeout(function () {
270+
removeEventListener("error", listener);
271+
expect(error.message).toContain("boom from worker");
272+
expect(globalErrors).toEqual([]);
273+
worker.terminate();
274+
done();
275+
}, SETTLE);
276+
});
277+
});
278+
279+
it("routes a throw from a parentPort listener to the parent's 'error' listeners", function (done) {
280+
var wt = require("node:worker_threads");
281+
var worker = new wt.Worker("~/tests/messaging/parentPortThrowingWorker.js");
282+
var messages = [];
283+
var finish = function () {
284+
expect(messages.length).toBe(1);
285+
expect(messages[0]).toContain("thrown by a parentPort listener");
286+
worker.terminate();
287+
done();
288+
};
289+
// Nothing else settles the spec when the error never arrives.
290+
var guard = setTimeout(finish, 10000);
291+
worker.on("error", function (error) {
292+
messages.push(error.message);
293+
if (messages.length === 1) {
294+
clearTimeout(guard);
295+
setTimeout(finish, SETTLE);
296+
}
297+
});
298+
worker.postMessage("go");
299+
});
258300
});
259301

260302
describe("AbortSignal handler attribute accounting", function () {

‎test-app/runtime/src/main/cpp/js/node-worker-threads.js‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ const { BroadcastChannel } = require("internal/broadcast-channel");
4444
const {
4545
EventTarget,
4646
defineEventHandler,
47+
dispatchEventRethrowing,
4748
globalEventTarget,
4849
} = require("internal/events");
4950

@@ -63,7 +64,6 @@ const globalPostMessage = g.postMessage;
6364

6465
const addEventListener = EventTarget.prototype.addEventListener;
6566
const removeEventListener = EventTarget.prototype.removeEventListener;
66-
const dispatchEvent = EventTarget.prototype.dispatchEvent;
6767

6868
// Runs `fn` after the caller returns. Node reports 'online' and 'exit' from
6969
// the thread's own lifecycle; the runtime's Worker has no equivalent signal,
@@ -123,10 +123,11 @@ class WorkerEmitter {
123123
return this.removeListener(type, listener);
124124
}
125125

126+
// Whether a listener was registered, as Node's EventEmitter reports it.
126127
emit(type, arg) {
127128
const list = this.#listeners[type];
128-
if (list === undefined) {
129-
return;
129+
if (list === undefined || list.length === 0) {
130+
return false;
130131
}
131132
const snapshot = ArrayPrototypeSlice(list);
132133
for (let i = 0; i < snapshot.length; i++) {
@@ -139,6 +140,7 @@ class WorkerEmitter {
139140
}
140141
FunctionPrototypeCall(entry.listener, this, arg);
141142
}
143+
return true;
142144
}
143145
}
144146

@@ -179,8 +181,10 @@ class Worker extends WorkerEmitter {
179181
worker.onmessageerror = function (event) {
180182
self.emit("messageerror", event.data);
181183
};
184+
// A truthy return cancels the error, so one an 'error' listener took is
185+
// not reported to the parent's global scope as well.
182186
worker.onerror = function (error) {
183-
self.emit("error", error);
187+
return self.emit("error", error);
184188
};
185189
soon(function () {
186190
self.emit("online", undefined);
@@ -312,9 +316,11 @@ ObjectDefineProperty(ParentPort.prototype, SymbolToStringTag, {
312316
let parentPort = null;
313317
if (!isMainThread) {
314318
parentPort = new ParentPort();
319+
// Rethrowing, so a listener that throws reaches the worker's error chain
320+
// (the scope's onerror, then the parent's Worker) the way a throwing
321+
// onmessage on the global scope does.
315322
const relay = function (event) {
316-
FunctionPrototypeCall(
317-
dispatchEvent,
323+
dispatchEventRethrowing(
318324
parentPort,
319325
new (getMessageEvent())(event.type, { data: event.data, ports: event.ports })
320326
);

0 commit comments

Comments
 (0)