Correct use of EnsureSuccessStatusCode and IsSuccessStatusCode

Question

I'm calling my Web API using an HttpClient and I see that there is an EnsureSuccessStatusCode method and an IsSuccessStatusCode property. Which one is appropriate?

I read this article and I have a few additional questions:

Usage of EnsureSuccessStatusCode and handling of HttpRequestException it throws

The issue I'm having is that if I send a GET request where I pass the ID of the object I want to retrieve, there are basically two outcomes:

1. I get the object back with a status 200
2. I could get null back because there was no match, however this results in a status of 404.

If I call EnsureSuccessStatusCode() a status 404 would result in an exception being thrown. That isn't ideal because when I was testing my code I kept getting a 404 error which at first made me think that the API URL was incorrect, but in reality there was no matching object with the supplied Id. In this case, I'd rather return a null object instead of throwing an exception.

So instead I tried checking the value of the IsSuccessfulStatusCode property. That seemed like a better choice and I can return a null object when this property is false, however, there are many status codes that can result in this property having a false value. 404 is one of them, but there are several other status codes, such as 400 Bad Request, 405 Method Not Allowed, etc. I'd like to log an exception for all unsuccessful error codes except for the 404's and I'm wondering if there is a better way to do this than inspecting the ResponseCode value of the response and then throwing an exception which gets caught by my catch block, which is where the logging takes place.

Here's the code for my GET method:

public static Customer GetCustomerByID(int id)
{
    try
        {
            using (var client = GetConfiguredClient())
            {
                Customer customer = null;
                var requestUri = $"Customers/{id}";

                using (var response = client.GetAsync(requestUri).Result)
                {
                    if (response.IsSuccessStatusCode)
                        customer = response.Content.ReadAsAsync<Customer>().Result;
                }

                return customer;
            }
        }
        catch (Exception ex)
        {
          ex.Data.Add(nameof(id), id);

          LogException(ex);

          throw;
        }
    }

This code will just return a null Customer if a non-successful status code is returned and nothing gets logged.

What is the best way to handle this situation?

Answer 1

The accepted answer employs "branching by exception", which is considered an anti-pattern by some. Here is how to use EnsureSuccessStatusCode and IsSuccessStatusCode in a way that uses exceptions only for errors that are unexpected or cannot or should not be handled "locally":

1. If you want to handle specific error responses, handle them directly using if-statements.
2. If you want to treat all (remaining) error responses as unexpected errors, use EnsureSuccessStatusCode and do not catch the exception, but assume it will be handled by a catch handler that can actually do something about it (such as higher level application logic, or a generic top level error handler).
3. If you want to do something for all (remaining) error responses (such as logging), but then proceed normally, or if you want to throw your own type of exception, use IsSuccessStatusCode.

This approach gives you all the advantages of exceptions while minimizing the disadvantages (such as breaking in the debugger on a completely normal event that you might not be interested in, or riddling your code with catch blocks, which are harder to read and write than if-statements).

Example:

using (var response = client.GetAsync(requestUri).Result)
{
  if (response.StatusCode == System.Net.HttpStatusCode.Unauthorized)
  {
    // TODO: Special handling for "401 Unauthorized" goes here
  }
  else
  {
    // All other unsuccessful error codes throw
    response.EnsureSuccessStatusCode();

    // TODO: Handling of successful response goes here
  }
}

... or if you want to read the error response or do logging, etc.:

using (var response = client.GetAsync(requestUri).Result)
{
  if (response.StatusCode == System.Net.HttpStatusCode.Unauthorized)
  {
    // TODO: Special handling for "401 Unauthorized" goes here
  }
  else if (!response.IsSuccessStatusCode)
  {
    // TODO: Read error response, logging, throw custom exception, etc., goes here

    // TODO: Keep this if you still want to throw the standard exception.
    // TODO: Otherwise, remove this.
    response.EnsureSuccessStatusCode();
  }
  else
  {
    // TODO: Handling of successful response goes here
  }
}

Answer 2

Because of this:

So instead I tried checking the value of the IsSuccessfulStatusCode property. That seemed like a better choice and I can return a null object when this property is false, however, there are many status codes that can result in this property having a false value. 404 is one of them, but there are several other status codes, such as 400 Bad Request, 405 Method Not Allowed, etc. I'd like to log an exception for all unsuccessful error codes except for the 404's and I'm wondering if there is a better way to do this than inspecting the ResponseCode value of the response and then throwing an exception which gets caught by my catch block, which is where the logging takes place.

I would use the EnsureSuccessStatusCode method and then modify the catch block like below:

public static Customer GetCustomerByID(int id)
{
    try
    {
        using (var client = GetConfiguredClient())
        {
            var requestUri = $"Customers/{id}";
            Customer customer;

            using (var response = client.GetAsync(requestUri).Result)
            {
                try 
                {
                    response.EnsureSuccessStatusCode();
                    // If we reach here it means we can get the customer data.
                    customer = response.Content.ReadAsAsync<Customer>().Result;
                }
                catch(HttpRequestException)
                {
                    if(response.StatusCode == HttpStatusCode.NotFound) // 404
                    {
                        customer = null;
                    }
                    else
                    {
                        throw;
                    }
                }
            }

            return customer;
        }
    }
    catch (Exception ex)
    {
        ex.Data.Add(nameof(id), id);

        LogException(ex);

        throw;
    }
}

Answer 3

According to this documentation: https://learn.microsoft.com/en-us/windows/uwp/networking/httpclient can be ok following sollution:

Uri requestUri = new Uri("http://www.contoso.com");

//Send the GET request asynchronously and retrieve the response as a string.
Windows.Web.Http.HttpResponseMessage httpResponse = new 
Windows.Web.Http.HttpResponseMessage();
string httpResponseBody = "";

try
{
    //Send the GET request
    httpResponse = await httpClient.GetAsync(requestUri);
    httpResponse.EnsureSuccessStatusCode();
    httpResponseBody = await httpResponse.Content.ReadAsStringAsync();
}
catch (Exception ex)
{
    httpResponseBody = "Error: " + ex.HResult.ToString("X") + " Message: " + ex.Message;
}

BUT: httpResponse.EnsureSuccessStatusCode(); == status codes from 200 to 299

I'm using HttpStatusCode.OK = 200 = is SUB-PART of EnsureSuccessStatusCode and indicates that the request succeeded and the requested information is in the response.

HttpResponseMessage result = await this._httpClient.GetAsync("https://www.web1.com/getThis", cancellationToken);
if (result.StatusCode != HttpStatusCode.OK)
{
    return new HttpResponseMessage(result.StatusCode)
    {
        Content = new StringContent( "ERROR DESCRIPTION HERE")
    };
}
return result;  // I can return here HttpResponseMessage....