Usage of EnsureSuccessStatusCode and handling of HttpRequestException it throws

Question

The idiomatic usage of EnsureSuccessStatusCode is to concisely verify success of a request, when you don't want to handle failure cases in any specific way. This is especially useful when you want to quickly prototype a client.

When you decide you want to handle failure cases in a specific way, do not do the following.

var response = await client.GetAsync(...);
try
{
    response.EnsureSuccessStatusCode();
    // Handle success
}
catch (HttpRequestException)
{
    // Handle failure
}

This throws an exception just to immediately catch it, which doesn't make any sense. The IsSuccessStatusCode property of HttpResponseMessage is there for this purpose. Do the following instead.

var response = await client.GetAsync(...);
if (response.IsSuccessStatusCode)
{
    // Handle success
}
else
{
    // Handle failure
}

I don't like EnsureSuccessStatusCode as it doesn't return anything meaninful. That is why I've created my own extension:

public static class HttpResponseMessageExtensions
{
    public static async Task EnsureSuccessStatusCodeAsync(this HttpResponseMessage response)
    {
        if (response.IsSuccessStatusCode)
        {
            return;
        }

        var content = await response.Content.ReadAsStringAsync();

        if (response.Content != null)
            response.Content.Dispose();

        throw new SimpleHttpResponseException(response.StatusCode, content);
    }
}

public class SimpleHttpResponseException : Exception
{
    public HttpStatusCode StatusCode { get; private set; }

    public SimpleHttpResponseException(HttpStatusCode statusCode, string content) : base(content)
    {
        StatusCode = statusCode;
    }
}

source code for Microsoft's EnsureSuccessStatusCode can be found here

Synchronous version based on SO link :

public static void EnsureSuccessStatusCode(this HttpResponseMessage response)
{
    if (response.IsSuccessStatusCode)
    {
        return;
    }

    var content = response.Content.ReadAsStringAsync().GetAwaiter().GetResult();

    if (response.Content != null)
        response.Content.Dispose();

    throw new SimpleHttpResponseException(response.StatusCode, content);
}

What I don't like about IsSuccessStatusCode is that it is not "nicely" reusable. For example you can use library like polly to repeat a request in case of network issue. In that case you need your code to raise exception so that polly or some other library can handle it...

I use EnsureSuccessStatusCode when I don't want to handle the Exception on the same method.

public async Task DoSomethingAsync(User user)
{
    try
    {
        ...
        var userId = await GetUserIdAsync(user)
        ...
    }
    catch(Exception e)
    {
        throw;
    }
}

public async Task GetUserIdAsync(User user)
{
    using(var client = new HttpClient())
    {
        ...
        response = await client.PostAsync(_url, context);

        response.EnsureSuccesStatusCode();
        ...
    }
}

The Exception thrown on GetUserIdAsync will be handled on DoSomethingAsync.

Below is my proposed solution. The only flaw is that since the ASP.NET Core framework resource manager is internal to the framework, I cannot directly re-use Microsoft's internationalized message strings, so I'm just using the verbatim English message literal here.

Pros

• Logs the content for an 5xx server error
  • Sometimes, a server error is actually a client error in disguise, such as a client using a deprecated endpoint that finally got shut off.
• Makes it easier to uncover errors when writing integration tests using ConfigureTestContainer<T>

Cons

• Inefficient.
  • If you read the response content, and the content is very long, you will slow the client down. For some clients, with soft real-time response requirements, this jitter may be unacceptable.
• Incorrect responsibility for error logging and monitoring.
  • If this is a 5xx server error, why does the client care, since the client did nothing wrong? Just call response.EnsureSuccessStatusCode(); and let the server deal with it.
  • Why not just check the server error logs when there is an Internal Server Error?
• Requires reading the Content property prior to checking the status. There may be situations where this is not desirable, one of which is inefficiency.

Usage

using (var requestMessage = new HttpRequestMessage(HttpMethod.Post, "controller/action"))
{
  using (var response = await HttpClient.SendAsync(requestMessage))
  {
    var content = await response.Content.ReadAsStringAsync();
    response.EnsureSuccessStatusCode2(content);
    var result = JsonConvert.DeserializeObject<ResponseClass>(content);
  }
}

API

    public static class HttpResponseMessageExtensions
    {
        public static void EnsureSuccessStatusCode2(this HttpResponseMessage message, string content = null)
        {
            if (message.IsSuccessStatusCode)
                return;
            var contentMessage = string.IsNullOrWhiteSpace(content) ? string.Empty : $"Content: {content}";
            throw new HttpRequestException(string.Format(
                System.Globalization.CultureInfo.InvariantCulture,
                "Response status code does not indicate success: {0} ({1}).{2}",
                (int)message.StatusCode,
                message.ReasonPhrase,
                contentMessage)
                );
        }
    }