Handle streaming errors and return to user proc, with result_type

[harlantwood]

Currently when streaming, errors are ignored and never returned to the user's code. This includes non-starter errors (eg wrong API key) as well as errors encountered in the course of usage (eg context length exceeded). These are discussed in #256 and #270 .

This PR (the one you are reading) offers an alternative solution:

• Errors are returned to the user's code as well as data blocks
• Every returned structure has a "result_type" injected into the top level of the JSON, one of:
  • "data"
  • "error"
  • "unknown"

Specs have been updated, but in short:

data: { "some": "json" } chunks are returned as data result_type
error: { "some": "json" } chunks are returned as error result_type
{"error": {"some": "json"} } chunks are returned as error result_type
{"some": "json"} chunks are returned as unknown result_type

Note that this is carefully crafted to ensure backward compatibility with existing code! For example, this sample code will work unchanged:

stream: proc do |chunk, _bytesize|
  print chunk.dig("choices", 0, "delta", "content")
end

.... it will just ignore the result_type field and look for chunks matching the expected shape.

Feedback or change requests welcome.

Thanks BTW for an awesome gem, we're super happy with it!

All Submissions:

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same update/change?
• Have you added an explanation of what your changes do and why you'd like us to include them?

[gregschwartz]

Would love to see this merged, I got stuck because I was missing errors for a bit. @alexrudall or others?

[harlantwood]

For @gregschwartz, or anyone else who wants to use this branch before merge, you can temporarily point your Gemfile to:

gem "ruby-openai", github: "harlantwood/ruby-openai", branch: "streaming-error-handling"

[codenamev]

I would like to see result as an object rather than an extended parsed JSON hash. The object could extend some of Hash's []/dig methods for backwards compatibility. Reason being, it would be nice to eventually add header info, or add other metadata that would be helpful when processing the chunks externally to this gem without polluting the response's contents.

[harlantwood]

Makes sense, but probably for another PR — trying to keep this one as tightly scoped as possible.

[PsiKai]

I don't think this implementation will catch all errors. I've been playing around with the solution and the regex doesn't work. Here is a sample using a hand-rolled HTTParty request with a bad API key:

irb(main):259:1* def handle_open_ai_response
irb(main):260:2*   proc do |response|
irb(main):261:2*     puts "IN PROC"
irb(main):262:2*     puts response
irb(main):263:2*     puts "PARSED RESPONSE"
irb(main):264:2*     puts response.scan(/^\s*(data|error): *(\{.+\})/i)
irb(main):265:2*     puts "END PARSED RESPONSE"
irb(main):266:1*   end
irb(main):267:0> end
:handle_open_ai_response
irb(main):268:0> HTTParty.post("https://api.openai.com/v1/chat/completions", headers:, body:, stream_body: true,  &handle_open_ai_response)
IN PROC
{
    "error": {
        "message": "You didn't provide an API key. You need to provide your API key in an Authorization header using Bearer auth (i.e. Authorization: Bearer YOUR_KEY), or as the password field (with blank username) if you're accessing the API from your browser and are prompted for a username and password. You can obtain an API key from https://platform.openai.com/account/api-keys.",
        "type": "invalid_request_error",
        "param": null,
        "code": null
    }
}
PARSED RESPONSE
END PARSED RESPONSE

Notice that the Regexp does not match the error with any information.

[harlantwood]

@alexrudall any thoughts on this one? Happy to make changes if you have requests.

[Timrael]

@harlantwood thank you for this PR!

[atesgoral]

I'm working on properly fixing these issues with #338 (and I'll follow up with a breaking change to raise actual exceptions)

[harlantwood]

@alexrudall any thoughts on this one vs #338 ? Happy to update this one and fix tests, but if you plan to merge #338 I won’t bother.

[alexrudall]

Big thanks for this @harlantwood -- going with #344 as simpler (and outsources some of the work to Shopify via event_stream_parser!) - but thanks a lot for taking the time to contribute this, it all helps find a solution.

[harlantwood]

Big thanks for this @harlantwood -- going with #344 as simpler (and outsources some of the work to Shopify via event_stream_parser!) - but thanks a lot for taking the time to contribute this, it all helps find a solution.

Sure thing! I think #344 is a great solution, if you’re willing to go with a breaking change.

Thanks for creating a great lib — we’ve been using it in production (streaming via hotwire) and it’s been super solid.