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

Exception on http status code 204 #13134

Open
stakach opened this issue Mar 1, 2023 · 6 comments
Open

Exception on http status code 204 #13134

stakach opened this issue Mar 1, 2023 · 6 comments

Comments

@stakach
Copy link
Contributor

stakach commented Mar 1, 2023

We're seeing an error on a 204 response from Microsoft. As you can see, MS Graph API returns an empty object as a response body on a 204 (image below)

Not sure what the HTTP standard says about 204 responses, however it seems a little heavy handed to raise an error in this case.
The application using the HTTP client can happily ignore the body if any was sent.

image

related to #2512

@straight-shoota
Copy link
Member

What's the Crystal error?

@stakach
Copy link
Contributor Author

stakach commented Mar 1, 2023

ArgumentError here
https://github.com/crystal-lang/crystal/blob/master/src/http/client/response.cr#L19-L21

@stakach
Copy link
Contributor Author

stakach commented Mar 1, 2023

Seems the behaviour is quite diverse: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/204#compatibility_notes

Apple Safari rejects any such data. Google Chrome and Microsoft Edge discard up to four invalid bytes preceding a valid response. Firefox tolerates in excess of a kilobyte of invalid data preceding a valid response.

and quite annoying MS is making basic mistakes with their APIs

@straight-shoota
Copy link
Member

Does the response in your case include a Content-Length header with nonzero value?
I think in that case it's very clear that the response can be just accepted, even though it's unexpected.

Without a Content-Length header it might be a bit more complicated, but probably not a big deal either.

@stakach
Copy link
Contributor Author

stakach commented Mar 1, 2023

All the browsers take the approach of ignoring the body data and making it unavailable to the user.

Given it's a success response it might make sense to follow Apples lead and just close the connection if any body data is provided - as the crystal client doesn't pipeline requests, there is no need to search for the next response.

If the crystal client treats responses, where no body is expected, with a body as valid, closes the connection and returns the headers, I think that would be improved behaviour.

@straight-shoota
Copy link
Member

Closing the connection definitely sounds like the easiest basic solution here. We can always improve it later with fancier stuff, if necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants