Visitar URL original
📝 Clarify comment about SSE event terminator by YuriiMotov · Pull Request #16354 · fastapi/fastapi · GitHub
Skip to content

📝 Clarify comment about SSE event terminator - #16354

Open
YuriiMotov wants to merge 3 commits into
masterfrom
clarify-sse-event-terminator
Open

YuriiMotov wants to merge 3 commits into
masterfrom
clarify-sse-event-terminator

Conversation

@YuriiMotov

@YuriiMotov YuriiMotov commented Sep 14, 2026 •

Copy link
Copy Markdown
Member

Closes: #16265

Description

The docstring of format_sse_event mistakenly states that each event end up with two end-of-line characters:

The result always ends with `\\n\\n` (the event terminator).

In fact, according to the specification, event can be empty (no comments and no fields) and then it will be just one empty line.
So, two end-of-line characters in the end is only true for non-empty events. First end-of-line character is actually a part of field or comment and second (empty line) is an event terminator.

AI Disclaimer

I used AI (Codex) to investigate, but it was mostly misleading until I learned how to read the notation in the spec and figured out the solution by myself and explained my thoughts. Then Codex approved my approach.

Checklist

  • This PR links to a GitHub Discussion for the proposed code change.
  • I added tests for the change.
  • The new or updated tests fail on the main branch and pass on this PR.
  • Coverage stays at 100%.
  • The documentation explains the change if needed.

@codspeed

codspeed Bot commented Sep 14, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 24 untouched benchmarks


Comparing clarify-sse-event-terminator (f740087) with master (e74b6e0)1

Open in CodSpeed

Footnotes

  1. No successful run was found on master (94918c1) during the generation of this report, so e74b6e0 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

Shriprasad-P

This comment was marked as spam.

@LeonardoSanBenitez

This comment was marked as spam.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation about how to use FastAPI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants