runtime: [System.Net.Http.Json] `HttpRequestException` thrown by `HttpClient.GetFromJsonAsync` doesn't contain response content

HttpClient.GetFromJsonAsync will throw an HttpRequestException on non-success responses. This is fine. But the exception doesn’t include enough info to figure out what went wrong.

So when using HttpClient.GetFromJsonAsync we have to leave some commented out debug code to figure out why a request failed. This isn’t ideal.

// uncomment to read the error response
// var error = await httpClient.GetAsync("...");
// var msg = await error.Content.ReadAsStringAsync();

var response = await httpClient.GetFromJsonAsync<Foo>("...");

This basically means that we can’t use GetFromJsonAsync if we care about why a request failed.

A solution would be to include HttpResponseMessage.Content in HttpRequestException. Then we could act on it. Something like this:

try
{
    var response = await httpClient.GetFromJsonAsync<Foo>("...");
}
catch (HttpRequestException ex)
{
    var errorMsg = await ex.Content.ReadAsStringAsync();
    _logger.Error(errorMsg);
    throw;
}

About this issue

Original URL
State: closed
Created 3 years ago
Reactions: 1
Comments: 20 (9 by maintainers)

Most upvoted comments

On another note. Can you point me to an article, or the like, that explains the dangers of logging? Cause I had considered sending logs to providers like CloudWatch safe.

I can’t recommend any, but there’s an abundance of material if you search for “pii logs”.

eiriktsarpalis on Apr 22, 2021

In your example how is that any better

Hmm, it isn’t. I think this can be written up as tunnel vison on using the simplified API when in practice it should be avoided in most real world cases. Even if the response was added to the exception then having to deal with try-catch scopes is - well annoying 😃

On another note. Can you point me to an article, or the like, that explains the dangers of logging? Cause I had considered sending logs to providers like CloudWatch safe.

snebjorn on Apr 21, 2021

@eiriktsarpalis the github issue I linked above does actually talk about that specific idea, but decided against it because disposables and exceptions don’t mix very well.

@snebjorn In your example how is that any better than:

using var response = await httpClient.SendAsync("...");
if (response.StatusCode == HttpStatusCode.OK)
    return await response.Content.ReadFromJsonAsync<Foo>();

var errorMsg = await response.Content.ReadAsStringAsync();
_logger.Error(errorMsg); // still should be careful and not actually log this sort of thing...
throw new Exception(...); // or HttpRequestException or app-specific exception

Also the catch block you wrote above would need to check ex.ResponseMessage is not null, because HttpRequestException is used for other types of errors like network connectivity, DNS, etc, Even if you catch that exception it isn’t guaranteed that there will be a response to parse at all.

Or if you really want to use the GetFromJsonAsync you can still do that and log the response code:

try
{
    var response = await httpClient.GetFromJsonAsync<Foo>("...");
}
catch (HttpRequestException ex)
{
    _logger.Error("Failed to get Foo: {statusCode}", ex.StatusCode); // available currently, and doesn't leak potentially sensitive response content
    throw;
}

dferretti on Apr 20, 2021

Some previous discussion on a similar topic: https://github.com/dotnet/runtime/issues/23648#issuecomment-550251657

Original request was for adding StatusCode to the exception. Then it was considered to include the full HttpResponseMessage on the exception, but that was decided against because that would involve keeping a disposable on the exception. So StatusCode was eventually added as of .net5.0.

GetFromJsonAsync uses EnsureSuccessStatusCode, so you will have access to ex.StatusCode to make decisions on. But I feel like if you know you may potentially want to parse different error codes and their response bodies you can just SendAsync and use the response.Content.ReadFromJsonAsync extensions after checking the response.StatusCode.

dferretti on Apr 16, 2021

I deal with a vendor api at work that uses 403 status code for an assortment of errors with a small message explaining why(invalid credentials, unauthorized, rate limited, etc).

In my current code, I try to get the response, and if I get a 403, I decipher the message and display something more useful to the end user. (e.g. Unauthorized: please contact Administrator if you think this is a mistake. Rate limited: Please try again later.).

While I’d like to use the simpler API, since I can’t get the content, I cannot here.

mburbea on Mar 26, 2021