Fix incorrectly parsing event fieldnames in yielding responses #2363
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes being requested
Remove requirement for event names to end with a dot if they're "response" or "transcript", and extend the notion of a "valid" event name to be either missing,
response
,transcript
, or an extended of either (response.
andtranscript.
), with separate logic to reduce code duplicity.Additional context & links
When parsing the SSE in a streaming request, the captured event (if any) is parsed to ensure it is valid/matches the expected event.
It seems there are three options for a "valid event" - a missing one, and either a
response
ortranscript
one. In the current implementation, the latter are forced to have a trailing dot (supposedly to offer more granular information on the event type, such asresponse.foo
), but this is out of scope for SSEs.In fact, a server yielding
event:response
gets the wrong path and ends up with a response chunk ofprocess_data(data={"data": data, "event": sse.event}, cast_to=cast_to, response=response)
, which is then incorrectly parsed on the client side.