Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
46 changes: 21 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,16 @@ function injectFakeEmitter(Class) {
};
}

// These extractors recover the raw value for .on() listeners from whatever
// event shape dispatchEvent delivered. They must match both the native
// MessagePort events (message/messageerror are MessageEvent carrying .data)
// and the events emit() constructs below. Everything else (including "error",
// which MessagePort never fires natively) rides CustomEvent.detail, which is
// what node's NodeEventTarget[kCreateEvent] produces for non-message types.
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 +149,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 +193,26 @@ function injectFakeEmitter(Class) {
return this;
}

function emit(event, ...args) {
// node's NodeEventTarget.prototype.emit(type, arg): carry a single raw arg,
// hand it as-is to node-style listeners, lazily wrap it as a typed event for
// addEventListener listeners, and return whether any listeners were
// registered. The extractors in functionForEventType undo the wrapping for
// .on() listeners, so the init dict here must round-trip through them.
// Known gap: listenerCount() only sees the .on()/.once() registry, so an
// addEventListener-only listener yields false here where node returns true
// (same pre-existing gap as listenerCount() itself).
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