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

UnexpectedResponse should grab response body #194

Open
adamchalmers opened this issue May 26, 2023 · 0 comments
Open

UnexpectedResponse should grab response body #194

adamchalmers opened this issue May 26, 2023 · 0 comments

Comments

@adamchalmers
Copy link
Contributor

When the KittyCAD API returns a HTTP 4xx or 5xx error to the KittyCAD rust client, it triggers the Error::UnexpectedResponse variant.

Generally our unit tests show the debug output of errors if they occur.

Unfortunately neither the debug nor display implementations for UnexpectedResponse are very helpful, because it doesn't show users the response body. Why not? Well, the UnexpectedResponse variant has one field, reqwest::Response. But this variant doesn't actually show you the response body, because reading the response body consumes the response. This means if you wanted to put the response body in the Display impl, it would consume the error object, so you'd only be able to read the error once.

Solution: the UnexpectedResponse variant should not contain a reqwest::Response. Instead, when the variant is constructed, it should:

  1. Store useful data like headers and HTTP status
  2. read the response body as text
  3. Store the headers and response body in a field of the enum

This way the errors will be helpful, contain the response body, and don't consume data when read.

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

No branches or pull requests

1 participant