From 0b82b45ce84b9022c3bc59f56c1ae7beea24f0e5 Mon Sep 17 00:00:00 2001 From: Brennan Butler <64561607+brennanbutler01@users.noreply.github.com> Date: Fri, 18 Sep 2026 16:52:51 +0700 Subject: [PATCH 1/2] fix: Abort rejected response requests Close the request before reporting terminal response validation errors so unread response bodies cannot keep connections active. Add real-server coverage for rejected status and content type, preserving error details and closed state. --- .changeset/failed-stream-cleanup.md | 5 ++++ src/EventSource.ts | 5 ++-- test/client.test.ts | 36 +++++++++++++++++++++++++++++ test/helpers/server.ts | 10 ++++++++ 4 files changed, 53 insertions(+), 3 deletions(-) create mode 100644 .changeset/failed-stream-cleanup.md diff --git a/.changeset/failed-stream-cleanup.md b/.changeset/failed-stream-cleanup.md new file mode 100644 index 0000000..e4d6b28 --- /dev/null +++ b/.changeset/failed-stream-cleanup.md @@ -0,0 +1,5 @@ +--- +'eventsource': patch +--- + +Abort the underlying request when a response fails EventSource validation. diff --git a/src/EventSource.ts b/src/EventSource.ts index 7588060..0602fa7 100644 --- a/src/EventSource.ts +++ b/src/EventSource.ts @@ -668,9 +668,8 @@ class EventSourceImpl extends EventTarget implements EventSource { #failConnection(message?: string, code?: number) { // [spec] …if the readyState attribute is set to a value other than CLOSED, // [spec] sets the readyState attribute to CLOSED… - if (this.#readyState !== this.CLOSED) { - this.#readyState = this.CLOSED - } + // Release the request before reporting failure, including responses with an unread body. + this.close() // [spec] …and fires an event named `error` at the `EventSource` object. // [spec] Once the user agent has failed the connection, it does not attempt to reconnect. diff --git a/test/client.test.ts b/test/client.test.ts index 7eb0fe8..cdf9d55 100644 --- a/test/client.test.ts +++ b/test/client.test.ts @@ -44,6 +44,42 @@ const xOriginRedirectTest = suite === 'happy-dom' ? test.fails : test */ const onHandlerTest = suite === 'workerd' ? test.fails : test +test.each([200, 403])( + 'aborts a rejected HTTP %i response before reporting failure', + async (status) => { + let signal: AbortSignal | undefined + let abortedOnError: boolean | undefined + const onError = getCallCounter({name: 'connection failure'}) + const es = new OurEventSource(`${serverUrl}/invalid-stream?status=${status}`, { + fetch(url, init) { + signal = init.signal + return request(url, init) + }, + }) + es.addEventListener('error', (event) => { + abortedOnError = signal?.aborted + onError.listener(event) + }) + + try { + await onError.waitForCallCount(1) + expect(es.readyState).toBe(OurEventSource.CLOSED) + expect(abortedOnError).toBe(true) + expect(signal?.aborted).toBe(true) + expect(onError.lastArg.code).toBe(status) + expect(onError.lastArg.message).toBe( + status === 200 + ? 'Invalid content type, expected "text/event-stream"' + : 'Non-200 status code (403)', + ) + es.close() + expect(onError.callCount).toBe(1) + } finally { + es.close() + } + }, +) + test('can connect, receive message, manually disconnect', async () => { const onMessage = getCallCounter({name: 'onMessage'}) const es = new OurEventSource(new URL(`${serverUrl}/`)) diff --git a/test/helpers/server.ts b/test/helpers/server.ts index cddddf3..6188cc5 100644 --- a/test/helpers/server.ts +++ b/test/helpers/server.ts @@ -61,6 +61,8 @@ export function handleRequest( return writeCounter(req, res) case '/mixed-ids': return writeMixedIds(req, res) + case '/invalid-stream': + return writeInvalidStream(req, res) case '/id-only': return writeIdOnly(req, res) case '/identified': @@ -145,6 +147,14 @@ async function writeCounter(req: IncomingMessage, res: ServerResponse) { res.end() } +function writeInvalidStream(req: IncomingMessage, res: ServerResponse) { + const status = + new URL(req.url || '/', 'http://localhost').searchParams.get('status') === '403' ? 403 : 200 + res.writeHead(status, {'Content-Type': status === 200 ? 'text/plain' : 'text/event-stream'}) + // Keep the response open so the client owns releasing the failed connection. + res.write('This response is not a usable event stream.') +} + /** * Writes two messages: one with an `id` field, then one without. Per the spec, the second * event's `lastEventId` must still be `'1'`: the last event ID buffer is only updated by an From 23535a0b1ecb85c9bf9d7e4692276b301addc20f Mon Sep 17 00:00:00 2001 From: Brennan Butler <64561607+brennanbutler01@users.noreply.github.com> Date: Fri, 18 Sep 2026 17:06:44 +0700 Subject: [PATCH 2/2] test: Keep response regressions separate Place rejected-response tests beside response validation tests and use a separate fixture location. This avoids conflicts with the independent buffered-message fix without changing test behavior. --- test/client.test.ts | 72 +++++++++++++++++++++--------------------- test/helpers/server.ts | 20 ++++++------ 2 files changed, 46 insertions(+), 46 deletions(-) diff --git a/test/client.test.ts b/test/client.test.ts index cdf9d55..cf64032 100644 --- a/test/client.test.ts +++ b/test/client.test.ts @@ -44,42 +44,6 @@ const xOriginRedirectTest = suite === 'happy-dom' ? test.fails : test */ const onHandlerTest = suite === 'workerd' ? test.fails : test -test.each([200, 403])( - 'aborts a rejected HTTP %i response before reporting failure', - async (status) => { - let signal: AbortSignal | undefined - let abortedOnError: boolean | undefined - const onError = getCallCounter({name: 'connection failure'}) - const es = new OurEventSource(`${serverUrl}/invalid-stream?status=${status}`, { - fetch(url, init) { - signal = init.signal - return request(url, init) - }, - }) - es.addEventListener('error', (event) => { - abortedOnError = signal?.aborted - onError.listener(event) - }) - - try { - await onError.waitForCallCount(1) - expect(es.readyState).toBe(OurEventSource.CLOSED) - expect(abortedOnError).toBe(true) - expect(signal?.aborted).toBe(true) - expect(onError.lastArg.code).toBe(status) - expect(onError.lastArg.message).toBe( - status === 200 - ? 'Invalid content type, expected "text/event-stream"' - : 'Non-200 status code (403)', - ) - es.close() - expect(onError.callCount).toBe(1) - } finally { - es.close() - } - }, -) - test('can connect, receive message, manually disconnect', async () => { const onMessage = getCallCounter({name: 'onMessage'}) const es = new OurEventSource(new URL(`${serverUrl}/`)) @@ -773,6 +737,42 @@ browserTest( }, ) +test.each([200, 403])( + 'aborts a rejected HTTP %i response before reporting failure', + async (status) => { + let signal: AbortSignal | undefined + let abortedOnError: boolean | undefined + const onError = getCallCounter({name: 'connection failure'}) + const es = new OurEventSource(`${serverUrl}/invalid-stream?status=${status}`, { + fetch(url, init) { + signal = init.signal + return request(url, init) + }, + }) + es.addEventListener('error', (event) => { + abortedOnError = signal?.aborted + onError.listener(event) + }) + + try { + await onError.waitForCallCount(1) + expect(es.readyState).toBe(OurEventSource.CLOSED) + expect(abortedOnError).toBe(true) + expect(signal?.aborted).toBe(true) + expect(onError.lastArg.code).toBe(status) + expect(onError.lastArg.message).toBe( + status === 200 + ? 'Invalid content type, expected "text/event-stream"' + : 'Non-200 status code (403)', + ) + es.close() + expect(onError.callCount).toBe(1) + } finally { + es.close() + } + }, +) + test('throws on `fetch()` that does not return web-stream', async () => { const url = `${serverUrl}/` diff --git a/test/helpers/server.ts b/test/helpers/server.ts index 6188cc5..a91ded8 100644 --- a/test/helpers/server.ts +++ b/test/helpers/server.ts @@ -61,8 +61,6 @@ export function handleRequest( return writeCounter(req, res) case '/mixed-ids': return writeMixedIds(req, res) - case '/invalid-stream': - return writeInvalidStream(req, res) case '/id-only': return writeIdOnly(req, res) case '/identified': @@ -73,6 +71,8 @@ export function handleRequest( return writeSlowConnect(req, res) case '/debug': return writeDebug(req, res) + case '/invalid-stream': + return writeInvalidStream(req, res) case '/set-cookie': return writeCookies(req, res) case '/authed': @@ -147,14 +147,6 @@ async function writeCounter(req: IncomingMessage, res: ServerResponse) { res.end() } -function writeInvalidStream(req: IncomingMessage, res: ServerResponse) { - const status = - new URL(req.url || '/', 'http://localhost').searchParams.get('status') === '403' ? 403 : 200 - res.writeHead(status, {'Content-Type': status === 200 ? 'text/plain' : 'text/event-stream'}) - // Keep the response open so the client owns releasing the failed connection. - res.write('This response is not a usable event stream.') -} - /** * Writes two messages: one with an `id` field, then one without. Per the spec, the second * event's `lastEventId` must still be `'1'`: the last event ID buffer is only updated by an @@ -573,6 +565,14 @@ function writeAuthed(req: IncomingMessage, res: ServerResponse) { res.end() } +function writeInvalidStream(req: IncomingMessage, res: ServerResponse) { + const status = + new URL(req.url || '/', 'http://localhost').searchParams.get('status') === '403' ? 403 : 200 + res.writeHead(status, {'Content-Type': status === 200 ? 'text/plain' : 'text/event-stream'}) + // Keep the response open so the client owns releasing the failed connection. + res.write('This response is not a usable event stream.') +} + function writeFallback(_req: IncomingMessage, res: ServerResponse) { res.writeHead(404, { 'Content-Type': 'text/plain',