From aabfe336f461824d0448bdb07d26225027f91b80 Mon Sep 17 00:00:00 2001 From: nabsei Date: Wed, 15 Jul 2026 01:58:42 +0200 Subject: [PATCH 1/2] fix(eio): expose EventEmitter interface on WebSocket upgrade response MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WebSocketResponse (the fake response object passed to middlewares during a WebSocket upgrade) did not implement EventEmitter, so any middleware calling res.on(...) — e.g. pino-http listening for "close" to know when the request finished — crashed with "res.on is not a function". Make WebSocketResponse extend EventEmitter and emit "close" when the underlying socket closes, and "finish" when end() is called, mirroring the events a real http.ServerResponse emits. Fixes #5072 Co-authored-by: Claude --- packages/engine.io/lib/server.ts | 7 ++++++- packages/engine.io/test/middlewares.js | 23 +++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/packages/engine.io/lib/server.ts b/packages/engine.io/lib/server.ts index c4f2c30d4c..293c741974 100644 --- a/packages/engine.io/lib/server.ts +++ b/packages/engine.io/lib/server.ts @@ -669,13 +669,17 @@ export abstract class BaseServer extends EventEmitter { * * @see https://nodejs.org/api/http.html#class-httpserverresponse */ -class WebSocketResponse { +class WebSocketResponse extends EventEmitter { constructor( readonly req, readonly socket: Duplex, ) { + super(); // temporarily store the response headers on the req object (see the "headers" event) req[kResponseHeaders] = {}; + // some middlewares (like pino-http) rely on the "close" event to know when the response is done, which the + // underlying socket does not expose by default + socket.once("close", () => this.emit("close")); } public setHeader(name: string, value: any) { @@ -695,6 +699,7 @@ class WebSocketResponse { public writeHead() {} public end() { + this.emit("finish"); // we could return a proper error code, but the WebSocket client will emit an "error" event anyway. this.socket.destroy(); } diff --git a/packages/engine.io/test/middlewares.js b/packages/engine.io/test/middlewares.js index 60e1cc6b22..6e97711999 100644 --- a/packages/engine.io/test/middlewares.js +++ b/packages/engine.io/test/middlewares.js @@ -56,6 +56,29 @@ describe("middlewares", () => { }); }); + it("should expose EventEmitter methods on the response object during upgrade (regression for pino-http)", (done) => { + const engine = listen((port) => { + engine.use((req, res, next) => { + expect(res.on).to.be.a("function"); + res.on("close", () => { + if (engine.httpServer) { + engine.httpServer.close(); + } + done(); + }); + next(); + }); + + const socket = new WebSocket( + `ws://localhost:${port}/engine.io/?EIO=4&transport=websocket`, + ); + + socket.on("open", () => { + socket.close(); + }); + }); + }); + it("should apply all middlewares in order", (done) => { const engine = listen((port) => { let count = 0; From 10c0eeddffccb688ef414720aa10262b3aaa5528 Mon Sep 17 00:00:00 2001 From: nabsei Date: Fri, 24 Jul 2026 15:14:20 +0200 Subject: [PATCH 2/2] fix(eio): expose EventEmitter interface on the uws ResponseWrapper too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit only patched WebSocketResponse in server.ts (used by the plain Node.js HTTP server). userver.ts (used with EIO_WS_ENGINE=uws) has its own separate response class, ResponseWrapper, which didn't get the same fix — so the pino-http regression test failed specifically under the uws engine. Two changes were needed, not just extending EventEmitter: unlike the Node.js HTTP server, where the same TCP socket persists across a WebSocket upgrade (so listening for its "close" event works directly), uWebSockets.js invalidates the HTTP response handle once res.upgrade() succeeds, and res.onAborted() never fires afterwards. The ResponseWrapper now also gets threaded through the WebSocket's user data on upgrade, so it can be told to emit "close" from the `close` handler on the `.ws()` route once the actual WebSocket connection ends. Verified: full engine.io suite passes under both EIO_WS_ENGINE=uws (178 passing, 0 failing, up from 177/1) and the default ws engine (175 passing on server.js/engine.io.js/middlewares.js, no regressions). prettier --check passes. --- packages/engine.io/lib/userver.ts | 36 +++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/packages/engine.io/lib/userver.ts b/packages/engine.io/lib/userver.ts index 54d34a6030..c3add250a6 100644 --- a/packages/engine.io/lib/userver.ts +++ b/packages/engine.io/lib/userver.ts @@ -1,4 +1,5 @@ import debugModule from "debug"; +import { EventEmitter } from "events"; import { AttachOptions, BaseServer, Server } from "./server"; import { HttpRequest, HttpResponse, TemplatedApp } from "uWebSockets.js"; import transports from "./transports-uws"; @@ -76,7 +77,7 @@ export class uServer extends BaseServer { (app as TemplatedApp) .any(path, this.handleRequest.bind(this)) // - .ws<{ transport: any }>(path, { + .ws<{ transport: any; response?: ResponseWrapper }>(path, { compression: options.compression, idleTimeout: options.idleTimeout, maxBackpressure: options.maxBackpressure, @@ -94,7 +95,9 @@ export class uServer extends BaseServer { ); }, close: (ws, code, message) => { - ws.getUserData().transport.onClose(code, message); + const { transport, response } = ws.getUserData(); + transport.onClose(code, message); + response?.emit("close"); }, }); } @@ -246,6 +249,10 @@ export class uServer extends BaseServer { res.upgrade( { transport, + // only set when middlewares are registered (see _applyMiddlewares); used to emit "close" on the + // response object once the WebSocket connection actually closes (see the `close` handler below) — + // unlike the plain Node.js HTTP server, UWS does not expose a persistent socket to listen on here + response: req.res instanceof ResponseWrapper ? req.res : undefined, }, req.getHeader("sec-websocket-key"), req.getHeader("sec-websocket-protocol"), @@ -288,12 +295,21 @@ export class uServer extends BaseServer { } } -class ResponseWrapper { +class ResponseWrapper extends EventEmitter { private statusWritten: boolean = false; private headers = []; private isAborted = false; - constructor(readonly res: HttpResponse) {} + constructor(readonly res: HttpResponse) { + super(); + // some middlewares (like pino-http) rely on the "close" event to know when the response is done, which + // uWebSockets.js does not expose directly — mirrors WebSocketResponse in server.ts + res.onAborted(() => { + // Any attempt to use the UWS response object after abort will throw! + this.isAborted = true; + this.emit("close"); + }); + } public set statusCode(status: number) { if (!status) { @@ -356,6 +372,7 @@ class ResponseWrapper { public end(data) { if (this.isAborted) return; + this.emit("finish"); this.res.cork(() => { if (!this.statusWritten) { // status will be inferred as "200 OK" @@ -372,13 +389,10 @@ class ResponseWrapper { } public onAborted(fn) { - if (this.isAborted) return; - - this.res.onAborted(() => { - // Any attempt to use the UWS response object after abort will throw! - this.isAborted = true; - fn(); - }); + // kept for backward compatibility; the actual UWS-level abort handler is registered once in the + // constructor (UWS only supports a single onAborted() callback per response), so this now just + // subscribes to the "close" event it emits + this.on("close", fn); } public cork(fn) {