Skip to content

fix: Abort rejected response requests - #363

Open
brennanbutler01 wants to merge 2 commits into
EventSource:mainfrom
brennanbutler01:fix/abort-rejected-response
Open

brennanbutler01 wants to merge 2 commits into
EventSource:mainfrom
brennanbutler01:fix/abort-rejected-response

Conversation

@brennanbutler01

@brennanbutler01 brennanbutler01 commented Sep 18, 2026

Copy link
Copy Markdown

Problem

A non-200 response or an invalid content type marks EventSource as closed without aborting its fetch. If the server keeps the response body open, the connection remains active. A later close() cannot release it because that method returns early for an already-closed instance.

Reproduced with a real local HTTP server on the installed 4.x release, published 5.1.1, and current main. Route terminal connection failures through close() before emitting the existing error event. This aborts the request and clears its controller while preserving the error code, message, and no-reconnect behavior. No public API changes.

Verification

  • Two regressions failed before the fix: HTTP 200 with an invalid content type, and HTTP 403 with an event-stream content type. Both now verify that the request is aborted before the error listener runs, the state is closed, and existing error details are preserved.
  • The test server deliberately leaves each rejected response body open.
  • Full Node suite: 46 passed, one existing browser-only skip.
  • Full Chromium and desktop WebKit suites: 47 passed each.
  • Full Bun 1.3.10 suite: 46 passed, one existing browser-only skip.
  • Full workerd suite: 39 passed, seven existing expected failures, and one existing skip. Its console warnings and network-error log also reproduce on untouched main.
  • TypeScript 7.0.2 source build, full test/configuration typecheck, and browser type-compatibility check pass. Oxlint, changed-file formatting, and git diff --check pass.
  • A separate reproduction using the built JavaScript export confirms that the server sees both rejected response connections close after the fix.
  • Test placement avoids conflicts with fix: Stop dispatching messages after close #362. The two branches merge cleanly; the combined Node suite passes 50 tests with one existing browser-only skip, and combined type checks and lint pass.

Verified on macOS with Node 24.13.0, Vitest 4.1.10, and Playwright 1.61.1. Firefox, iOS Safari, Deno, and happy-dom were not run. Local verification reused an isolated toolchain; dependency manifests and lockfiles are unchanged. Includes a patch changeset.

Prepared with OpenAI Codex assistance, including the reproduction, implementation, tests, and local review.

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.
Copilot AI lite review requested due to automatic review settings September 18, 2026 09:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants