Replacement for EnsureSuccessStatusCode

Question

I like the simplicity of EnsureSuccessStatusCode for HttpResponseMessage after a call using HttpClient. But it does not really provide great information. This is my replacement for it.

I am looking to see if I have missed useful information that is in the HttpResponseMessage, if there is a better exception type to throw or if I could generally improve it.

Some of the tricky parts was constructing a viable logging message and an exception message when logging needs it in its component parts and logging wants it all together (and some parts are optional).

public static class HttpResponseHelper
{
    public static async Task CheckResponseAndThrowIfIsError(this HttpResponseMessage response, 
           string? errorMessage = null, ILogger? logger = null, bool includeResponseContent = true)
    {
        if (response.IsSuccessStatusCode == false)
        {
            var statusCode = response.StatusCode.ToString();

            string fullErrorMessage = "";
            if (!string.IsNullOrEmpty(errorMessage))
            {
                fullErrorMessage = $"{errorMessage}\r\n";
            }
            else
            {
                // Put in something generic so that the log as a value instead of a blank line.
                errorMessage = "Call Failed";
            }

            fullErrorMessage += $"Call Failed with Status Code: {statusCode}";
            
            var errorMessageFromCall = "";
            if (includeResponseContent)
            {
                errorMessageFromCall = await response.Content.ReadAsStringAsync();
                fullErrorMessage += $"\r\n{errorMessageFromCall}";
            }

            logger?.LogError("{errorMessage}\r\nCall Failed with Status Code: {statusCode}\r\n{errorMessageFromCall}",
                errorMessage, statusCode, errorMessageFromCall);

            throw new ApplicationException(fullErrorMessage);
        }
    }
}

Its use would look like this:

var response = await httpClient.SendAsync(message);
await response.CheckResponseAndThrowIfIsError("My Error Message Here", _logger);

Answer 1

Here are my observations:

if (response.IsSuccessStatusCode == false)

• I would rather suggest to have an early exit by inverting the if condition
  • Having a single giant guard condition is less expressive and maintainable IMHO

string fullErrorMessage = "";
if (!string.IsNullOrEmpty(errorMessage))
{
    fullErrorMessage = $"{errorMessage}\r\n";
}
else
{
    // Put in something generic so that the log as a value instead of a blank line.
    errorMessage = "Call Failed";
}

• The fullErrorMessage can be constructed in a single step whenever you throw the exception so, I would advice against build it incrementally
• The else block is basically a fallback value if the errorMessage parameter is blank
  • I would suggest a simple ?: structure instead

var errorMessageFromCall = "";
if (includeResponseContent)
{
    errorMessageFromCall = await response.Content.ReadAsStringAsync();
    fullErrorMessage += $"\r\n{errorMessageFromCall}";
}

• Yet again the errorMessageFromCall is either a blank string or the full response body
  • So, a simple ?: structure would be enough here as well
  • I would suggest to truncate the string to avoid logging failure due to too large content

logger?.LogError("{errorMessage}\r\nCall Failed with Status Code: {statusCode}\r\n{errorMessageFromCall}",
    errorMessage, statusCode, errorMessageFromCall);

• I would suggest to use constant instead of hard coding \r\n many times
  • By using the line break constant you would rely on string interpolation
  • In that case you would need to escape the { and } characters by doubling them

throw new ApplicationException(fullErrorMessage)

• As I stated earlier I would rather construct here the fullErrorMessage in a single step
• As it was mentioned by others as well using an HttpRequestException would fill more natural here

---

So, here is my revised version of your method:

private const string LineBreak = "\r\n";
private const int MaxBodyLength = 5_000;
...


if (response.IsSuccessStatusCode)
    return;

errorMessage = !string.IsNullOrEmpty(errorMessage) ? errorMessage : "Call Failed";

string errorMessageFromCall = includeResponseContent ? await response.Content.ReadAsStringAsync() : string.Empty;
errorMessageFromCall = errorMessageFromCall.Length >= MaxBodyLength
    ? errorMessageFromCall[..(MaxBodyLength - 3)] + "..."
    : errorMessageFromCall;

logger?.LogError($"{{errorMessage}}{LineBreak}Call Failed with Status Code: {{statusCode}}{LineBreak}{{errorMessageFromCall}}",
    errorMessage, response.StatusCode, errorMessageFromCall);

throw new HttpRequestException($"{errorMessage}{LineBreak}Call Failed with Status Code: {response.StatusCode}{LineBreak}{errorMessageFromCall}");