Skip to content

fix(core): default the SSE event type to message, omit it when encoding - #123

Merged
dinwwwh merged 6 commits into
mainfrom
claude/tender-mendel-tosiu2
Sep 29, 2026
Merged

dinwwwh merged 6 commits into
mainfrom
claude/tender-mendel-tosiu2

Conversation

@dinwwwh

@dinwwwh dinwwwh commented Sep 29, 2026 •

Copy link
Copy Markdown
Member

toAsyncIteratorObject in the fetch adapter, which the node and aws-lambda adapters also use, only handled messages whose event: field was message, error or close. A message with no event: field was dropped, even though the SSE spec gives it the default type message, and many third-party SSE servers send only data: lines. The default now lives in the core decoder, and the encoder relies on it to stop writing event: message.

Fixes

  • An SSE message without an event: field, or with an empty one, is now read as a message event instead of being skipped. A response from a server that never sends event: no longer resolves as an empty stream.

Notes for reviewers

  • decodeEventStreamMessage gives an unnamed message with data the event type message. An unnamed message without data (a comment, a keep-alive ping, or an id/retry-only message) is never dispatched per spec, so it decodes with no event type and readers keep skipping it. The fetch reader's switch is unchanged from main.
  • encodeEventStreamMessage no longer writes event: message for a message with data, so yield { a: 1 } sends data: {"a":1}. Without data it keeps the line, so the message is not mistaken for a comment. The peer adapter already sends data messages without an event type.
  • This changes the wire format in a way older readers can't handle. No released reader, up to and including 0.9.2, handles unnamed messages. A server with this change talking to an older client would have every data message dropped, and only close and error would get through. An old server talking to a new client is fine. The encoder change is its own commit (af91cce), so it can be split out and released later, after clients have the decoder fix.
  • Named messages without data are still dispatched, although browsers drop them. The library's own event: message (from yield undefined) and empty event: close depend on this. Unnamed messages without data must stay skipped, or every keep-alive ping would come through as undefined.

Testing

  • New decoder test for the default event type (unnamed, empty event:, named without data, comment/id/retry-only). New encoder test for omitting event: message. The round-trip test now covers both.
  • New fetch reader test with unnamed messages, which fails on the original code.
  • Tests that check the exact bytes sent in fetch, node, fastify and aws-lambda are updated to the new wire format. Reader tests still send an explicit event: message.
  • eslint, tsc -b and vitest run (1244 tests) pass locally. In the Bun suite, data-transfer passes. Five signal-and-cancel tests fail the same way without this change.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NVk6KTeqLJfBMM1aWsSHZq

`toAsyncIteratorObject` only handled messages whose `event:` field was
`message`, `error` or `close`, so a message with no `event:` field was
silently dropped. Per the SSE spec an absent or empty event type means
`message`, which is what most third-party SSE servers send. Resolving
such a response through the fetch, node or aws-lambda adapters lost
those messages, and a stream with no `event:` lines looked empty.

Unnamed messages are now dispatched as `message` events when they carry
data. Unnamed messages without data, such as comment-only keep-alives
or `id`/`retry`-only messages, are still skipped, as the spec requires.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NVk6KTeqLJfBMM1aWsSHZq
Move the SSE default event type from the fetch reader into
`decodeEventStreamMessage`, so every consumer of the core decoder gets it.

An unnamed message (no `event:` field, or an empty one) that carries data
now decodes with `event: 'message'`, as the spec requires. An unnamed
message without data (a comment, a keep-alive, or an `id`/`retry`-only
message) is never dispatched per spec, so it decodes with no event type.
Named messages keep their type with or without data, which the library's
own `event: message` and `event: close` messages without data rely on.

The fetch reader goes back to its original `switch`: unnamed messages
with data now arrive as `message`, and data-less ones still match no case.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NVk6KTeqLJfBMM1aWsSHZq
`encodeEventStreamMessage` no longer writes `event: message` for a
message with data, since `message` is the SSE default event type there.
The decoder fills it back in, so `decode(encode(m))` still round-trips.

A message without data keeps `event: message`: an unnamed message
without data would look like a comment or keep-alive and not be
dispatched.

This matches the peer adapter, which already sends data messages without
an event type.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NVk6KTeqLJfBMM1aWsSHZq
@pkg-pr-new

pkg-pr-new Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
@standard-server/aws-lambda

npm i https://pkg.pr.new/@standard-server/aws-lambda@123

@standard-server/core

npm i https://pkg.pr.new/@standard-server/core@123

@standard-server/fastify

npm i https://pkg.pr.new/@standard-server/fastify@123

@standard-server/fetch

npm i https://pkg.pr.new/@standard-server/fetch@123

@standard-server/node

npm i https://pkg.pr.new/@standard-server/node@123

@standard-server/peer

npm i https://pkg.pr.new/@standard-server/peer@123

@standard-server/shared

npm i https://pkg.pr.new/@standard-server/shared@123

commit: a269dda

@dinwwwh dinwwwh changed the title Omit default 'message' event type from encoded SSE messages fix(core): default the SSE event type to message, omit it when encoding Sep 29, 2026
@codecov

codecov Bot commented Sep 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codspeed

codspeed Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Merging this PR will improve performance by 11.11%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 2 improved benchmarks
✅ 24 untouched benchmarks
⏩ 108 skipped benchmarks1

Performance Changes

Benchmark BASE HEAD Efficiency
⚡ 100 small messages in 1KB chunks 957.8 µs 861.4 µs +11.2%
⚡ 100 small messages in one chunk 917.2 µs 826.1 µs +11.03%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.


Comparing claude/tender-mendel-tosiu2 (a269dda) with main (ddd0a09)

Open in CodSpeed

Footnotes

  1. 108 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@pullfrog pullfrog Bot 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.

Important

The encoder omission is only safe once the reader ships the matching decoder default. An older @standard-server client has no protocol version negotiation and will silently drop every unnamed data message a new server sends. Please make a deliberate call on the compatibility window before merging.

Reviewed changes

  • Encoder omits event: message when data is present — encodeEventStreamMessage() skips the event: line when the event type is message and data !== undefined, keeping it for data-less messages so they are not mistaken for comments (packages/core/src/event-stream/encoder.ts:79).
  • Decoder fills in the default event type — decodeEventStreamMessage() sets event: 'message' for any message with data but no (or an empty) event type, and leaves comment/id/retry-only messages without one (packages/core/src/event-stream/decoder.ts:81).
  • Type documentation — EventStreamMessage.event now documents message as the data-message default (packages/core/src/event-stream/types.ts:26).
  • Tests — encoder/decoder edge cases and round-trip coverage added; every adapter response/body test moved to the shorter wire form, plus a new toAsyncIteratorObject test for unnamed data: messages (packages/fetch/src/event-stream.test.ts:113).

⚠️ A new server is incompatible with already-published clients

The reader that consumes this wire format is packages/fetch/src/event-stream.ts:30, whose switch (value.event) has no default/undefined case, so it skips any decoded message that lacks a recognized event type. The decoder default added here only exists in the new code; any older @standard-server/core yields { data } with no event for the very messages the encoder now emits unnamed, and the reader loops past them — every message is dropped, not just one. Because the packages have no protocol version handshake, a mixed-version deployment (new server package, older fetch/core client) fails silently rather than loudly.

Technical details
# Backward-compatibility window for the omitted event type

## Affected sites
- `packages/core/src/event-stream/encoder.ts:79` — omits `event: message` for data messages.
- `packages/fetch/src/event-stream.ts:30` — unmodified reader; `switch` on `value.event` with no `case undefined`, so unnamed messages (from an older decoder) are re-read/skipped.
- `packages/core/src/event-stream/decoder.ts:81` — the new defaulting that only newer readers have.

## Required outcome
- A deliberate decision on the compatibility window, since there is no wire-protocol version negotiation. Options: emit `event: message` until readers can be assumed updated; ship the decoder default in a prior/separate release before the encoder omission; or treat the shorter wire form as an explicit breaking change (major bump / release note).

## Open questions for the human
- Is a mixed-version deployment (new server adapter, older `@standard-server/fetch`/`core` reader) a supported scenario? If not, a changelog/release note may be the whole fix.
- Do node/fastify/aws-lambda senders need to stay on the old form for one release cycle?

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using DeepSeek Flash (default — pick a model for stronger reviews) | 𝕏

Comment thread packages/core/src/event-stream/decoder.ts
Clarify in the decoder that an empty `data:` line dispatches with data
`''`. Per spec, each `data:` line appends its value plus a line feed, so
only a message with no `data:` lines at all has an empty data buffer and
is skipped. Add decoder tests for the empty `data:` line, with and
without an empty `event:` field.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NVk6KTeqLJfBMM1aWsSHZq
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NVk6KTeqLJfBMM1aWsSHZq

@pullfrog pullfrog Bot 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.

✅ No new issues found.

Reviewed changes

Reviewed the two commits since the prior Pullfrog review at af91cce — 2aa4e33 (docs plus decoder tests for an empty data: line) and 96a2ae87 (comment-only shortening). Neither changes runtime behavior.

  • Documented and tested the empty-data: decode contract — decodeEventStreamMessage now carries a comment explaining that an unnamed message with data (including an empty data: line) is a message event while a data-less one is left untyped; two new assertions cover data:\n\n and event:\ndata:\n\n decoding to { event: 'message', data: '' }. Both would fail if the default or the empty-value handling regresses (packages/core/src/event-stream/decoder.ts, decoder.test.ts).
  • Shortened explanatory comments — the decoder, encoder, types, and test comments were condensed with no semantic change (packages/core/src/event-stream/{decoder,encoder,types}.ts, *.test.ts).

Pullfrog  | View workflow run | Using DeepSeek Flash (default — pick a model for stronger reviews) | 𝕏

…tosiu2

# Conflicts:
#	packages/fetch/src/event-stream.test.ts
@dinwwwh
dinwwwh merged commit 0367a5d into main Sep 29, 2026
8 of 9 checks passed
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