Skip to content

Parse event stream fields without a colon - #53

Merged
clue merged 2 commits into
clue:1.xfrom
bensynapse:fix-colonless-fields
Oct 7, 2026
Merged

clue merged 2 commits into
clue:1.xfrom
bensynapse:fix-colonless-fields

Conversation

@bensynapse

Copy link
Copy Markdown
Contributor

I run Live Tennis API.

A bare id line currently leaves the previous event ID set. The next reconnect then sends that stale Last-Event-ID header.

The SSE parsing rules define an empty value for fields without a colon.

The fix clears bare id fields and preserves empty lines from bare data fields. A bare retry remains ignored.

The tests check parser output, decoder state and the reconnect request headers. Five regression cases fail against the original parser.

PHP 8.4: vendor/bin/phpunit --coverage-text --coverage-clover=clover.xml passes all 121 tests and 298 assertions. The workflow's coverage check passes with 100% statement coverage.

PHP 7.2: vendor/bin/phpunit -c phpunit.xml.legacy also passes all 121 tests and 298 assertions.

@bensynapse

Copy link
Copy Markdown
Contributor Author

I fixed the PHP 5.6 failure in 80ba24b. Empty field values now stay strings, so event: keeps the default message type.
The existing regression reproduces the error before this fix. All 121 tests pass with 100% coverage on PHP 5.6 and PHP 8.4.

@clue clue added bug Something isn't working new feature New feature or request labels Oct 7, 2026
@clue clue added this to the v1.5.0 milestone Oct 7, 2026

@clue clue left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@bensynapse Thanks for looking into this and fixing the legacy builds, changes LGTM! Matches the spec and a local spike I had for this back in 2022, so great to see this land with proper tests :shipit: Keep it coming 👍

@clue
clue merged commit 3b6f956 into clue:1.x Oct 7, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working new feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants