Skip to content

Return error in case of runner crash #69

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

Merged
merged 3 commits into from
Jun 18, 2025
Merged

Return error in case of runner crash #69

merged 3 commits into from
Jun 18, 2025

Conversation

p1-0tr
Copy link
Member

@p1-0tr p1-0tr commented Jun 5, 2025

In case the runner crashes it would be nice to return an error to the
user. So, add an error handler on the proxy and use it to try to figure
out if the runner crashed and format the response error accordingly.

Based on: #68

@p1-0tr p1-0tr requested review from xenoscopic and doringeman June 5, 2025 12:05
@@ -134,6 +135,28 @@ func run(
proxyLog: proxyLog,
}

proxy.ErrorHandler = func(w http.ResponseWriter, req *http.Request, err error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is going to be sent back to OpenAI clients, I would try to structure the error responses in a format they can parse. Documentation is a little sparse, but Google says the error format looks like:

{
  "error": {
    "message": "Invalid 'messages[1].content': string too long. Expected a string with maximum length 1048576, but got a string with length 1540820 instead.",
    "type": "invalid_request_error",
    "param": "messages[1].content",
    "code": "string_above_max_length"
  }
}

The only docs I can find for the API are https://platform.openai.com/docs/api-reference/responses-streaming/error

In case the runner crashes it would be nice to return an error to the
user. So, add an error handler on the proxy and use it to try to figure
out if the runner crashed and format the response error accordingly.

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@docker.com>
@p1-0tr p1-0tr force-pushed the ps-fwd-runner-error branch from fe4ca85 to aaacee4 Compare June 17, 2025 12:56
@p1-0tr p1-0tr marked this pull request as ready for review June 17, 2025 12:56
@p1-0tr p1-0tr requested a review from xenoscopic June 17, 2025 12:56
Copy link
Contributor

@doringeman doringeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can I easily test this to see the error?

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@docker.com>
Copy link
Contributor

@doringeman doringeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Co-authored-by: Dorin-Andrei Geman <dorin.geman@docker.com>
@p1-0tr p1-0tr merged commit e57dcc0 into main Jun 18, 2025
4 checks passed
@p1-0tr p1-0tr deleted the ps-fwd-runner-error branch June 18, 2025 06:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants