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 Error Propagation During Chat Streaming #285

Open
gilljon opened this issue Nov 6, 2024 · 6 comments
Open

Fix Error Propagation During Chat Streaming #285

gilljon opened this issue Nov 6, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@gilljon
Copy link

gilljon commented Nov 6, 2024

If stream = True, we are noticing that a 400 returns (in the server logs):

StreamError("Invalid status code: 400 Bad Request")

On the client side (Python consumption):

httpx.RemoteProtocolError: peer closed connection without sending complete message body (incomplete chunked read)

If stream = False, then we still get the 400, but also more rich information:

Traceback (most recent call last):
  File "test.py", line 5, in <module>
    out = client.chat.completions.create(
          ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "python3.12/site-packages/openai/_utils/_utils.py", line 275, in wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "python3.12/site-packages/openai/resources/chat/completions.py", line 829, in create
    return self._post(
           ^^^^^^^^^^^
  File "python3.12/site-packages/openai/_base_client.py", line 1277, in post
    return cast(ResponseT, self.request(cast_to, opts, stream=stream, stream_cls=stream_cls))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "python3.12/site-packages/openai/_base_client.py", line 954, in request
    return self._request(
           ^^^^^^^^^^^^^^
  File "python3.12/site-packages/openai/_base_client.py", line 1058, in _request
    raise self._make_status_error_from_response(err.response) from None
openai.BadRequestError: After the optional system message, conversation roles must alternate user/assistant/user/assistant/...

What would be great is that we are able to retrieve the enriched error information even if streaming is true. That is the current behavior with the Python client.

@64bit 64bit added the enhancement New feature or request label Nov 12, 2024
@SeseMueller
Copy link

Chasing it down a bit, the actual response is contained in the error, so a fix shouldn't be that hard.

Basically, in the file error.rs in the reqwest-eventsource-0.6.0, the error type that is later used is defined. It is on line 43: InvalidStatusCode and contains the StatusCode and the Response. However, it is annotated with the error macro from the thiserror crate (See line 42) and thus only outputs the StatusCode, the Response is discarded.

That is, if the error code defined by the macro is used.

In src/client.rs on line 441, async-openai (0.25.0, sorry) simply calls e.to_string() on the error it might have gotten from the Eventsource, discarding all other information contained in e, such as the Response.

Changing this to something that properly captures the Response or just adds it to the Error String will drastically improve stream debugging capabilities, as was talked about in this issue.

I might work on it later.

SeseMueller pushed a commit to SeseMueller/async-openai that referenced this issue Feb 2, 2025
@SeseMueller
Copy link

I added a simple block to, if an invalid status code or content type is encountered, a custom error String is used instead that also contains the full Response text.

I didn't understand the testing suite or whether this is covered by the tests; sorry about that.

I'll quickly test it in my local environment and report back.

@SeseMueller
Copy link

SeseMueller commented Feb 2, 2025

It does work, I got it to output a more useful error when calling o3-mini.
Comparison:

Before:

StreamError("Invalid status code: 400 Bad Request")

Now:

StreamError("Invalid status code: 400 Bad Request\n{\n  \"error\": {\n    \"message\": \"Unsupported parameter: 'parallel_tool_calls' is not supported with this model.\",\n    \"type\": \"invalid_request_error\",\n    \"param\": \"parallel_tool_calls\",\n    \"code\": \"unsupported_parameter\"\n  }\n}")

Note that the error is quite ugly, but is much more informative.
I'm not sure how the errors should reasonably be printed in a pretty way that still contains all useful information.

Edit:
This actually helped a lot in working with the new reasoning models, as they have a few parameters they don't want set. The error messages just directly contained the information which parameters I needed to turn off.

@64bit
Copy link
Owner

64bit commented Feb 2, 2025

Thanks for debugging and sharing, it seems like underlying response.text().await can be deserialized into WrappedError and eventually to OpenAIError::ApiError variant to return. When error deserialization of response.text().await to WrapedError fails - fallback to OpenAIError::StreamError(response.text().await.<unwrap-with-default>).

@gilljon
Copy link
Author

gilljon commented Feb 5, 2025

Could someone share a code snippet of what exactly needs to change in order to support better error handling?

@SeseMueller
Copy link

SeseMueller commented Feb 5, 2025

Could someone share a code snippet of what exactly needs to change in order to support better error handling?

I did a minimal solution on my fork: SeseMueller@4b82e9a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants