-
-
Notifications
You must be signed in to change notification settings - Fork 361
Don't swallow model errors in streaming mode #270
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
Conversation
line.delete_prefix!("data: ") | ||
user_proc.call(JSON.parse(line)) | ||
end | ||
elsif chunk.include?('"error": ') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't know 100% how it works, but can it just return me some JSON examples with this string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
raw chunk: {
"error": {
"message": "The model `gpt-3.5-turbo!!!!` does not exist",
"type": "invalid_request_error",
"param": null,
"code": null
}
}
FYI I have an open PR to solve the same issue, using a different approach: #275 |
Merge to current release
rescue JSON::ParserError | ||
# Ignore invalid JSON. | ||
proc do |chunk, overall_received_bytes, env| | ||
Rails.logger.debug("raw chunk: #{chunk}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove references to Rails
. It's not a dependency and will cause issues for anyone using this outside of Rails.
I've created a new issue (#320) to track adding a logger, and this can be added there.
I'm working on properly fixing these issues with #338 (and I'll follow up with a breaking change to raise actual exceptions) |
All Submissions:
I lost a lot of time and suffered needless premature aging until putting this change into place on my project. A bug was causing context overflows and the current release of the library swallows those errors.
Credit to @lingceng for the solution in #256