Skip to content

fix(core): parse event-stream retry and id fields per the SSE spec - #114

Merged
dinwwwh merged 1 commit into
mainfrom
claude/frosty-montalcini-d69880
Sep 26, 2026
Merged

dinwwwh merged 1 commit into
mainfrom
claude/frosty-montalcini-d69880

Conversation

@dinwwwh

@dinwwwh dinwwwh commented Sep 26, 2026

Copy link
Copy Markdown
Member

The event-stream decoder now follows the WHATWG SSE rules for retry and id, matching browsers, undici's EventSource and eventsource-parser. retry: 010 is now read as 10 instead of being dropped, and an id containing U+0000 NULL is ignored (the previous id is kept) instead of being surfaced as event meta that fetch can't send back as Last-Event-ID.

Fixes

  • retry values made only of ASCII digits are accepted, including leading zeros; signs, decimals, exponents, hex, non-ASCII digits and extra spaces are still ignored.
  • A retry too large to parse to a finite number (309+ digits) is ignored instead of producing Infinity, so a decoded message never makes withEventMeta throw and end the stream.
  • id fields containing NULL are ignored by the decoder, and rejected by the encoder, withEventMeta and the peer validator, since every spec-compliant client drops them.

Notes for reviewers

  • The id error message is now "Event's id must not contain a carriage return, newline or NULL character".
  • Peer messages with a NULL id are now rejected by isPeerEventStreamMessage, the same way ids with CR/LF already were.
  • Very large finite retry values are kept as parsed and may lose precision past Number.MAX_SAFE_INTEGER.

Testing

  • New decoder, encoder, withEventMeta and peer validator cases; each fails against the previous code.
  • Full suite passes (1240 tests), 100% coverage on touched files, eslint and tsc clean.

The decoder now accepts any all-ASCII-digit retry (so `retry: 010` is 10,
matching browsers, undici and eventsource-parser) and ignores an id that
contains U+0000 NULL, keeping the previous id. Values that parse to
Infinity are ignored so withEventMeta never rejects decoded output.

The encoder, withEventMeta and the peer validator now also reject ids
containing NULL: every spec-compliant client drops such ids, and a NULL
in a Last-Event-ID header makes fetch throw.
@pkg-pr-new

pkg-pr-new Bot commented Sep 26, 2026

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

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

@standard-server/core

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

@standard-server/fastify

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

@standard-server/fetch

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

@standard-server/node

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

@standard-server/peer

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

@standard-server/shared

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

commit: ef56e24

@codecov

codecov Bot commented Sep 26, 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 26, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 26 untouched benchmarks
⏩ 108 skipped benchmarks1


Comparing claude/frosty-montalcini-d69880 (ef56e24) with main (a545524)

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.

✅ No new issues found.

Reviewed changes

  • retry decoding (decoder.ts) — Number.parseInt is now gated by /^\d+$/ and isEventStreamMessageRetry, so leading zeros parse (retry: 010 → 10) and 400-digit values that become Infinity are ignored instead of producing a value withEventMeta rejects.
  • id decoding (decoder.ts) — an id containing U+0000 NULL is ignored and the prior id is retained, matching WHATWG §9.2.6.
  • id encoding/validation (encoder.ts, types.ts, peer/src/validators.ts) — isEventStreamMessageId now rejects \r, \n, and \0; error text and peer validation updated accordingly, with comments still permitting NULL.
  • Tests — decoder, encoder, withEventMeta, and peer validator cases added; each fails against the pre-PR code.

Verified against the WHATWG HTML spec §9.2.6: retry must consist of only ASCII digits (and /^\d+$/ is ASCII-only in JS), and an id containing U+0000 NULL must be ignored while the Last-Event-ID value space excludes NUL/LF/CR. The implementation and its tests line up with both. The only tradeoff, already documented in the PR description, is precision loss for astronomically large finite retry values — not a practical concern.

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

@dinwwwh
dinwwwh merged commit 193755f into main Sep 26, 2026
11 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.

1 participant