Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: make the SSE parser properly ignore non-data chunks #487

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hourianto
Copy link

@hourianto hourianto commented Mar 12, 2025

As per https://developer.mozilla.org/en-US/docs/Web/API/Server-sent_events/Using_server-sent_events#event_stream_format and https://developer.mozilla.org/en-US/docs/Web/API/Server-sent_events/Using_server-sent_events#examples, the actual SSE stream can have so-called "comments" that do not have a field name and just have whatever comment after the colon. Any proper SSE parser should ignore those and not try to parse them.

In the case of python-genai, this is an issue when using a different base_url in http_options for a separate proxy to the Gemini API which might add extra things in the SSE stream (for example, keepalive like : this is a keep-alive). As far as I know this should be a very safe change because the official API only cares about the "data" chunks in any way (if there were any other chunks with prefixes, the json parsing would fail anyway).

Copy link

google-cla bot commented Mar 12, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@hourianto hourianto changed the title Make the SSE parser properly ignore non-data chunks fix: make the SSE parser properly ignore non-data chunks Mar 12, 2025
@hourianto
Copy link
Author

Sorry for the spam in the PR, I just wanted to have a single clean commit based on the latest main

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