Throw exceptions for failure status codes with more data about the response in them. #45
Conversation
Some APIs return useful messages in 401 responses, etc, but HttpResponseMessage.EnsureStatusCode() throws away the response, and only includes the status code in the message of the exception it throws. This throws a custom ApiException with enough information in it to deal with the error properly.
Had to modify the PostToRequestBin test too, but I'm not sure what the point of that is since it will always fail. Maybe we should just remove it?
| if (response.Content == null) return exception; | ||
|
|
||
| exception.ContentHeaders = response.Content.Headers; | ||
| exception.Content = await response.Content.ReadAsStringAsync(); |
There was a problem hiding this comment.
Content can be null. Also, what happens if reading the content fails? We should still return an ApiException with the status code information
There was a problem hiding this comment.
Content being null is checked up on line 551 (before trying to read the headers). But yeah, this will 💥 if it fails to read the content.
Do you reckon I should just wrap this line in a try/catch with an empty catch? It feels a bit dirty, but we're already handling an exception at this point.
There was a problem hiding this comment.
@bennor Yep, that's what I would do - the failure content is just bonus points, we can tolerate if it fails
|
Hey, no hurry on this obviously, but just in case you missed the notification - I pushed some updates about 6 days ago. |
| public HttpContentHeaders ContentHeaders { get; private set; } | ||
| 8000 | public string Content { get; private set; } | |
| public bool HasContent | ||
| { |
|
Cool, I'm gonna fix up these small things and merge. Thanks @bennor! |
|
No problem. Sorry about that style stuff. I tried to set up Resharper to match but a few of the finer points are beyond what Resharper can do for me and it's a fair but different to the style I use elsewhere. I'll make sure I double check it all next time. 😸 |
|
@bennor No worries, I fixed them up on merge. Also, this is why I don't use R#, because some of its style decisions are hard-coded. Also the death-by-1000-paper-cuts perf issues |
Some APIs return useful messages and other data in 401 responses, etc, but
HttpResponseMessage.EnsureStatusCode()eats the response, and only includes the status code in the message of the exception it throws.This PR adds (and throws) a custom exception type with enough information in it to deal with the error properly in calling code.
I was able to achieve this in my own project using a custom message handler that intercepts the response and throws a custom exception, but it feels like a lot of work and a bit of a hack to have to do that.