Better SSE parsing #332
Better SSE parsing #332
Conversation
atesgoral
force-pushed
the
robust-sse-parsing
branch
from
c1899ad
to
e330b43
Compare
| @@ -53,13 +57,13 @@ def to_json(string) | |||
| # @param user_proc [Proc] The inner proc to call for each JSON object in the chunk. | |||
| # @return [Proc] An outer proc that iterates over a raw stream, converting it to JSON. | |||
| def to_json_stream(user_proc:) | |||
There was a problem hiding this comment.
A better name for this would be to_completion_stream because once the completion JSON is parsed, it's no longer "JSON" -- it's a completion object.
There was a problem hiding this comment.
I didn't add a separate unit test for this new module because it's sufficiently tested by the http tests.
| stream.call(<<~CHUNK) | ||
| data: { "foo": "bar" } | ||
|
|
||
| # |
There was a problem hiding this comment.
This ensures the trailing \n\n are preserved.
| @@ -117,13 +121,14 @@ | |||
| expect(user_proc).to receive(:call).with(JSON.parse('{"foo": "bar"}')) | |||
| expect(user_proc).to receive(:call).with(JSON.parse('{"baz": "qud"}')) | |||
|
|
|||
| stream.call(<<-CHUNK) | |||
| stream.call(<<~CHUNK) | |||
There was a problem hiding this comment.
Should be unindenting these, so switched from - to ~ in these heredocs.
| data: { "foo": "bar" } | ||
| data: { BAD ]:-> JSON } |
There was a problem hiding this comment.
So, this was actually a totally new case that doesn't have to be handled in the scope of OpenAI responses because they don't seem to return anything like this. Normally, adjacent data field values should be concatenated with a \n -- it's a multi-line value. In this case, the above would be read as "{ "foo": "bar" }\n{ BAD ]:-> JSON }" and that is not what this test's intention is.
| stream.call(<<~CHUNK) | ||
| data: { "foo": "bar" } | ||
|
|
||
| error: { "message": "A bad thing has happened!" } |
There was a problem hiding this comment.
AFAIK, there's no such thing as an "error" field. But that is beyond the scope of this PR.
| end | ||
| end | ||
|
|
||
| context "when called with JSON split across chunks" do |
There was a problem hiding this comment.
This is the not-so-common case this PR intends to fix. Most people using this library to hit OpenAI directly will not encounter this. But I have seen it happen in the past, even when talking to OpenAI directly, when the network was congested, or there was some backpressure in my streaming. I can't put my finger on it right now.
|
This is awesome! Thanks for working on this @atesgoral |
|
Related issue: Helicone/helicone/issues/678 |
|
I'm working on a proper parser. I'll report back. |
|
Published |
The issue mentioned in #251 persists.
OpenAI reliably returns one completion JSON per SSE message. However, when intermediaries are placed between OpenAI and the client (e.g. a proxy), the combining and splitting of chunks during streaming/buffering (or even TCP fragmentation) can cause the client to receive chunks with completion fragments (invalid JSON).
This PR brings more spec-compliant SSE parsing. Instead of stripping and ignoring whitespace, processes a chunk by splitting it into blocks delimited by
"\n\n", and buffering any remaining block that is not a fulldatafield.More spec-compliant, but not really fully spec-compliant: There's still no handling of multi-line data spread across adjacent
datafields. But there's no need to make this a generic SSE parser since the only concern is hitting OpenAI.I've updated the http tests by making them emit proper SSE chunks into the stream. I added a test for the fragmented completion JSON case.
Note that this is still a bit of a quick hack and not the best way to parse SSE. An ideal parser would process the stream line by line with a state machine (instead of YOLO splitting at
\n\n).I also noticed two issues that I did not address in the scope of this PR:
errorfield in SSE. It's neither in the spec, nor in OpenAI's documentation. It would be nice to remove the handling of that, or mention in a code comment where it's coming from."[DONE]"value omitted by OpenAI, and also the very JSON fragmentation issue that is not very common, but still a possibility in certain settings.