Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 13 additions & 25 deletions src/js/node/worker_threads.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,6 @@ function injectFakeEmitter(Class) {
return event.data;
}

function errorEventHandler(event: ErrorEvent) {
return event.error;
}

function customEventHandler(event) {
return event.detail;
}
Expand All @@ -134,14 +130,13 @@ function injectFakeEmitter(Class) {
};
}

// message/messageerror arrive as MessageEvent (.data), from native dispatch
// and from emit() below; everything else rides CustomEvent.detail (node's
// NodeEventTarget default). MessagePort has no native "error" event.
Comment thread
robobun marked this conversation as resolved.
Outdated
function functionForEventType(event, listener) {
switch (event) {
case "error":
case "message":
case "messageerror": {
return wrapped(errorEventHandler, listener);
}

case "message": {
return wrapped(messageEventHandler, listener);
}

Expand All @@ -151,14 +146,6 @@ function injectFakeEmitter(Class) {
}
}

function EventClass(eventName) {
if (eventName === "error" || eventName === "messageerror") {
return ErrorEvent;
}

return MessageEvent;
}

// EventTarget dedupes on (type, callback), so in node the FIRST registration of
// a listener wins outright -- including its once-ness -- and later adds of the
// same function are no-ops. Keying wrappers per listener reproduces that.
Expand Down Expand Up @@ -203,20 +190,21 @@ function injectFakeEmitter(Class) {
return this;
}

function emit(event, ...args) {
// node's NodeEventTarget.emit(type, arg): single arg, boolean return. The
// init dicts here must round-trip through functionForEventType's extractors.
// listenerCount() misses addEventListener-only listeners (pre-existing gap).
Comment thread
robobun marked this conversation as resolved.
Outdated
function emit(event, arg) {
const hadListeners = listenerCount.$call(this, event) > 0;
Comment thread
robobun marked this conversation as resolved.
switch (event) {
case "error":
case "messageerror":
case "message":
this.dispatchEvent(new (EventClass(event))(event, ...args));
case "messageerror":
this.dispatchEvent(new MessageEvent(event, { data: arg }));
break;
default:
// Non-standard events surface as CustomEvent (detail = first arg) to
// addEventListener and as the raw argument to .on(), matching node.
this.dispatchEvent(new CustomEvent(event, { detail: args[0] }));
this.dispatchEvent(new CustomEvent(event, { detail: arg }));
break;
}
return this;
return hadListeners;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
}

const kMaxListeners = Symbol("kMaxListeners");
Expand Down
86 changes: 86 additions & 0 deletions test/js/node/worker_threads/worker_threads.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1312,6 +1312,92 @@ test("MessagePort NodeEventTarget methods", () => {
port1.close();
});

// emit() previously did `new MessageEvent("message", payload)` / `new
// ErrorEvent("error", err)`, so the payload was (mis)read as an init dict and
// .on() listeners received null/undefined. It also returned `this`. Verified
// against node: .on() sees the raw arg by identity; addEventListener sees a
// MessageEvent (data = arg) for message/messageerror and a CustomEvent
// (detail = arg) otherwise; emit() returns a boolean.
test("MessagePort emit() delivers the raw payload and returns a boolean", () => {
// message: .on() gets the payload by identity, addEventListener gets a
// MessageEvent whose .data is the payload; emit() -> true.
{
const { port1: p } = new MessageChannel();
const payload = { id: 42 };
let onArg: unknown = "unset";
let aelData: unknown = "unset";
let aelCtor = "";
p.on("message", x => (onArg = x));
p.addEventListener("message", e => ((aelData = e.data), (aelCtor = e.constructor.name)));
const ret = p.emit("message", payload);
expect({ onArg, aelData, aelCtor, ret }).toEqual({
onArg: payload,
aelData: payload,
aelCtor: "MessageEvent",
ret: true,
});
expect(onArg).toBe(payload);
expect(aelData).toBe(payload);
p.close();
}

// error: .on() gets the Error by identity; addEventListener gets a
// CustomEvent whose .detail is the Error (node's NodeEventTarget default).
{
const { port1: p } = new MessageChannel();
const err = new Error("boom");
let onArg: unknown = "unset";
let aelDetail: unknown = "unset";
let aelCtor = "";
p.on("error", x => (onArg = x));
p.addEventListener("error", (e: any) => ((aelDetail = e.detail), (aelCtor = e.constructor.name)));
const ret = p.emit("error", err);
expect({ onArg, aelDetail, aelCtor, ret }).toEqual({
onArg: err,
aelDetail: err,
aelCtor: "CustomEvent",
ret: true,
});
expect(onArg).toBe(err);
p.close();
}

// messageerror: same shape as message (MessageEvent, .data = arg).
{
const { port1: p } = new MessageChannel();
const err = new Error("boom");
let onArg: unknown = "unset";
let aelData: unknown = "unset";
p.on("messageerror", x => (onArg = x));
p.addEventListener("messageerror", (e: any) => (aelData = e.data));
p.emit("messageerror", err);
expect(onArg).toBe(err);
expect(aelData).toBe(err);
p.close();
}

// No listeners: emit() -> false (not `this`).
{
const { port1: p } = new MessageChannel();
expect(p.emit("message", 1)).toBe(false);
expect(p.emit("custom", 1)).toBe(false);
p.close();
}

// Custom events (unchanged behaviour, guards against regression): .on() gets
// the arg, addEventListener gets CustomEvent with .detail = arg.
{
const { port1: p } = new MessageChannel();
let onArg: unknown = "unset";
let aelDetail: unknown = "unset";
p.on("foo", (x: unknown) => (onArg = x));
p.addEventListener("foo", (e: any) => (aelDetail = e.detail));
p.emit("foo", 123);
expect({ onArg, aelDetail }).toEqual({ onArg: 123, aelDetail: 123 });
p.close();
}
});

// jsRef() only gated on m_isDetached, so .ref()/onmessage= after the peer closed
// re-took an event-loop ref that nothing releases and the process hung. Node no-ops
// both. Spawned, because the symptom is "the process never exits".
Expand Down
Loading