Repository navigation
feat(fetch)!: always send close event, remove emptyCloseEventEnabled option - #125
Merged
Merged
Conversation
…` option `toEventStream` now always emits a `close` event when the iterator completes, including when it returns `undefined`, so clients can reliably tell a finished stream from a dropped connection.
@standard-server/aws-lambda
@standard-server/core
@standard-server/fastify
@standard-server/fetch
@standard-server/node
@standard-server/peer
@standard-server/shared
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This PR makes toEventStream always emit the terminal close event and removes the emptyCloseEventEnabled option.
- Always-on close event —
packages/fetch/src/event-stream.ts:187now unconditionally enqueues{ ...meta, event: result.done ? 'close' : 'message', data: stringifyJSON(data) }, dropping theemptyCloseEventEnabledgate and its default. Behavior under default options is unchanged; only callers that passedfalsegain a trailingevent: close. - Option removal —
emptyCloseEventEnabledis gone fromToEventStreamOptions(packages/fetch/src/event-stream.ts:116). Since@standard-server/nodere-exports this type (packages/node/src/event-stream.ts:18), the break is marked correctly as!. - Test cleanup — the
empty close eventdescribe block (5 tests) is removed. The survivingwith returnandwithout returncases (packages/fetch/src/event-stream.test.ts:218,:238) still pin the close event both with a return value and without, so coverage is retained.
No remaining references to emptyCloseEventEnabled exist anywhere in the repo, and packages/core/README.md:185 already documents that return emits a close event, so the docs get strictly more accurate. pnpm exec vitest run packages/fetch/src/event-stream.test.ts packages/node/src/event-stream.test.ts passes (26 tests).
DeepSeek Flash (default — pick a model for stronger reviews) ｜ 𝕏
Contributor
Merging this PR will not alter performance
Comparing 
Footnotes
|
dinwwwh
added a commit
that referenced
this pull request
Oct 18, 2026
…169) `toAsyncIteratorObject` now only finishes cleanly on a `close` event, or throws `ErrorEvent` on an `error` event. A stream that ends without either now rejects with `EventStreamDecoderError('Event Stream ended before complete')`. That is the same error a stream gets when it ends partway through a message. The rule also covers a missing (`null`) body. Since #125, `toEventStream` always sends a `close` event, so streams from standard-server senders are unaffected. This change lets the client actually use that signal: a dropped connection no longer looks like a finished stream. ## Why When the server's pipeline errors, the client already sees a network error. Many cut-offs still reach the client as a clean end of stream, though, and before this change they read as a finished stream. In a local probe with Node's `fetch` and `http2` client: - an HTTP/1.1 response ended early (`res.end()`) - a connection-close-delimited body (an HTTP/1.0 hop) whose connection drops - an HTTP/2 stream ended early, or its session torn down Proxy timeouts, serverless time limits and graceful shutdowns can end responses in these ways. ## Scope - Applies to `@standard-server/fetch`, `@standard-server/node` and `@standard-server/aws-lambda`, which share the fetch implementation. - `@standard-server/peer` is unchanged. It already rejects when its queue closes before a `close` event. - Stopping early with `return()` still resolves as done. ## Breaking change - Reading an event stream that ends without a `close` event now throws instead of resolving `{ done: true, value: undefined }`. - This affects SSE senders that never send `event: close`: generic SSE APIs, and standard-server senders that set the old `emptyCloseEventEnabled: false` option. - `toAsyncIteratorObject(null)` now throws on the first `next()`. This happens, for example, with a HEAD or 204 response labelled `text/event-stream`. ## Testing - Updated the tests that relied on a close-less stream finishing quietly. They now either send `event: close` or expect the error. - Added an empty-stream case and changed the null-stream case to expect the error. - `pnpm run check` (sherif, oxlint, oxfmt, tsc) and the full vitest suite pass locally. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01NYpqRELEp4zdwCYWeqRMAj --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
toEventStreamnow always sends acloseevent when the iterator completes, even if it returnsundefined(sent asevent: closewith no data). TheemptyCloseEventEnabledoption is removed, so clients can always tell a finished stream from a dropped connection.Breaking change
emptyCloseEventEnabledis now a type error in@standard-server/fetchand@standard-server/node, which share the same options type.falsenow receive a trailingevent: closethey didn't get before.Testing
with returnandwithout returntests still cover the close event, both with a return value and without one.