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

Check HTTP status code in request_stream #1300

Closed
chubei opened this issue Sep 27, 2023 · 4 comments · Fixed by #1433
Closed

Check HTTP status code in request_stream #1300

chubei opened this issue Sep 27, 2023 · 4 comments · Fixed by #1433
Labels
bug Something isn't working client http issues with the client help wanted Not immediately prioritised, please help!

Comments

@chubei
Copy link
Contributor

chubei commented Sep 27, 2023

Would you like to work on this feature?

yes

What problem are you trying to solve?

The request_stream function doesn't call handle_api_errors as other functions. User can't easily tell whether the returned HTTP body is for an error response or successful response.

Describe the solution you'd like

In request_stream, check the response code. If it's an error, fetch the whole HTTP body, parse the error and return Err.

Describe alternatives you've considered

Include the headers in the returned Ok variant of request_stream.

Documentation, Adoption, Migration Strategy

No response

Target crate for feature

kube-client

@clux
Copy link
Member

clux commented Sep 28, 2023

Thanks for this. Yeah, this looks wrong to me as well.

Note that the Api::log_stream is a relatively new in its current form; it was updated in #1235 to use use AsyncRead.

We should at the very least get the status (as you say) and bubble up an error here, but i'm unsure if we can just take the body and try to parse it as an ErrorResponse as we do in general in the handle_api_errors fn. Experimentation here would be very much welcome. I'm a bit busy the next coming weeks, so have to leave this open.

@clux clux added bug Something isn't working help wanted Not immediately prioritised, please help! client http issues with the client labels Sep 28, 2023
@chubei
Copy link
Contributor Author

chubei commented Sep 28, 2023

I can work on this if you find the solution acceptable.

@clux
Copy link
Member

clux commented Sep 28, 2023

Yeah, it seems reasonable to me. Thanks a lot!

@clux clux linked a pull request Mar 22, 2024 that will close this issue
@clux
Copy link
Member

clux commented Mar 22, 2024

Fixed in #1433

@clux clux closed this as completed Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working client http issues with the client help wanted Not immediately prioritised, please help!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants