@davidsh @Priya91

I think all that people are asking for is that the status code that is embedded in the HttpRequestException message in EnsureSuccessStatusCode be made available to catchers of the thrown exception without having to parse the message string.

Maybe that means extending HttpRequestException with a nullable int or nullable HttpStatusCode property. Maybe that means a whole other derived exception type. Maybe that means using the Exception.Data property.

There’s a few options, and I don’t think folks will be too picky. All they really want is a way to get the response status code without parsing the exception message.

Personally, I’d prefer a StatusCode property on the exception to make catch-when filtering nice and clean:

try 
{
  // make call
}
catch (HttpRequestException ex) when (e.StatusCode == 404)
{
  // do something
}
catch (HttpRequestException ex) when (e.StatusCode == 503)
{
  // do something
}

In today’s HttpClient API, HttpRequestException is not thrown by the HttpClient APIs except for networking errors (like can’t connect etc).

However, the StatusCode can still be obtained easily from the original HttpResponseMessage returned by the HttpClient APIs prior to any call of EnsureSuccessStatusCode.

If you are interested in this feature, DESPITE the fact that HttpClient provides a reasonable way to get to StatusCode (even without exceptions being thrown)…

@davidsh @karelz This is so very frustrating. You keep saying things to this effect, but as I’ve already pointed out in this very thread, it’s simply not the case. HttpClient already provides a bunch of exception-based APIs which throw when they encounter HTTP errors and, in those cases, the status code is not available.

Clarify your use case - why is using (slower) exception handling better/easier for your code? Is it just because of migration from HttpWebRequest, or is more involved?

I don’t understand the fascination with performance. For me, the use case is simplicity and readability. I’d say that over 90% of the time when I use HttpClient I want to make some simple GET requests. I have no desire to deal with request/response messages. The exception-based GetStringAsync (and friends) is ideal for writing succinct readable code, but the instant I need to special-case a HTTP error, I have to reinvent code that already exists in the framework.

The performance overhead of an exception being thrown simply doesn’t enter into the equation.

i cannot believe there is such resistance to exposing the http status code to the calling code. the old WebClient api provided a mechanism for retrieving this. it’s amazing to me that the recommended way to find out what caused the exception it to parse the error string - this is insane.

@davidsh

Adding a new exception type would be a large change and would break existing API behavior.

How would that be a breaking change? The breaking change rules explicitly mention that throwing a more derived exception is allowed.

This assumes that you’re the direct consumer of the HttpResponseMessage and any details that it provides.

If you are building a library, for example, then:

• You don’t need to expose the underlying HttpResponseMessage, probably shouldn’t, and sometimes can’t.
• For a good consumer API you would want the return object to follow the 99% use-case, and can use exceptions for rare failures. If you create a return object that encompasses all scenarios, many consumers will ignore the failure cases anyway.

In all cases, though:

• For rare cases of a bad response, such as a 500/503 when the services are down, or 504 gateway timeout, the performance hit from exceptions are often not a concern, particularly given that we’re talking milliseconds or less, and a web request is often hundreds of milliseconds and varies based on network performance, hardware, the phase of the moon, etc.
• For a bad response, it is still often useful for consumers to handle the status code.
• A lot of codebases are still migrating from HttpWebRequest or WebClient and it is simpler to continue the behaviour of “success = ok, failure = throw” than re-architect applications, including breaking changes in libraries to build out an entirely new exception-less API design.
• I also don’t buy the exception-performance argument considering that HttpRequestException is also thrown in cases where there is no valid HTTP response (DNS failure, etc.), or TaskCanceledException for timeouts. The system isn’t entirely exception-less, so drawing a line at “once we have a parsed HTTP response, don’t use exceptions because they are slow” seems odd and arbitrary.

Adding a new exception type would be a large change and would break existing API behavior. As I mentioned above, if you need an exception to be thrown for a valid, but unsuccessful, HTTP status code, we recommend using the HttpClient.EnsureSuccessStatusCode() method.

@yaakov-h it is on our short list of top backlog asks for .NET Core 3.0 (the milestone reflects that). It has a decent chance to be done in 3.0 (no guarantee though).

@yaakov-h This issue is a dupe of dotnet/corefx#24253 , how is this issue different from that one?

i don’t understand this. You’re basically saying that since EnsureSuccessStatusCode doesn’t return the failure code, it’s useless and we should re-implement it ourselves?

EnsureSuccessStatusCode is just a convenience method that checks the HttpResponseMessage and then throws an exception. But it doesn’t have to be used. Many times, not throwing exceptions results in faster performance. That was an important consideration when HttpClient was designed. HttpWebRequest threw exceptions all the time even when a valid HTTP response was received.

public bool IsSuccessStatusCode
{
    get { return ((int)_statusCode >= 200) && ((int)_statusCode <= 299); 
}

public HttpResponseMessage EnsureSuccessStatusCode()
{
    if (!IsSuccessStatusCode)
    {
        throw new HttpRequestException(string.Format(
            System.Globalization.CultureInfo.InvariantCulture,
            SR.net_http_message_not_success_statuscode,
            (int)_statusCode,
            ReasonPhrase));
    }

    return this;
}

So, the best-practice pattern we recommend when using HttpClient is not to use EnsureSucessStatusCode at all. That API was added just to mimic HttpWebRequest behaviors. Using this API generates exceptions which slows down code. If you just need to check the status code of an HttpResponseMessage, it would be better to use the single ‘if’ check and then do your business logic accordingly. And you can do that with the public IsSuccessStatusCode method. This avoid any extra exceptions being thrown.

dotnet/corefx#24253 proposed adding rarely-used fields to an existing exception type.

This issue proposes adding a new exception type for this specific purpose.

HttpRequestException.StatusCode property

Rationale and Usage

A new nullable HttpRequestException.StatusCode property exposes a response’s status code if it has been already received, otherwise it’s set to null. This will enable a direct access to the status code on exceptions thrown from HttpClient’s simple GET methods and HttpResponseMessage.EnsureSuccessStatusCode(). New HttpRequestException constructors taking a status code will initialize that property.

try
{
    string response = httpClient.GetStringAsync(requestUri);
    ...
}
catch(HttpRequestException e)
{
    if (e.StatusCode == HttpStatusCode.MovedPermanently)
    {
       ...
    }
}

Proposed API

public class HttpRequestException : Exception
{
        ...
+       public HttpStatusCode? StatusCode { get; }
        
        public HttpRequestException(string message)
        {...}

+       public HttpRequestException(string message, HttpStatusCode? statusCode)
+       {...}

        public HttpRequestException(string message, Exception inner)
        {...}

+       public HttpRequestException(string message, Exception inner, HttpStatusCode? statusCode)
+       {...}
}

@yaakov-h Do you agree with this proposal? If yes, I will update the description.

Video

• Looks good
• At the minimum we’ll need to remove the nullable annotation fromt he first constructor, otherwise passing null as the second argument would be a source breaking change
• However, we concluded we can just remove it.

public class HttpRequestException : Exception
{
    // Existing:
    // public HttpRequestException(string message);
    // public HttpRequestException(string message, Exception inner);

    public HttpRequestException(string message, Exception inner, HttpStatusCode? statusCode);
    public HttpStatusCode? StatusCode { get; }
}

Yes, you are right HttpStatusCode would be better. I will update the comment and description.

Having an HttpResponseMessage in an exception means that it will keep connections open etc. So, then, do we add an Dispose() method to the HttpRequestException? It isn’t IDisposable at all. And having a pattern where complex types are inside exceptions is very confusing.

Yes, that was my concern as well.

However, the StatusCode can still be obtained easily from the original HttpResponseMessage returned by the HttpClient APIs prior to any call of EnsureSuccessStatusCode.

i don’t understand this. You’re basically saying that since EnsureSuccessStatusCode doesn’t return the failure code, it’s useless and we should re-implement it ourselves?

And then you use that ‘argument’ (?) as the basis for rejecting this request?

I know this is just opinions here, but that reasoning seems specious at best.

Given @davidsh’s summary and insights, I feel more that it is weird to mimic this old style HttpWebRequest-way of surfacing StatusCode to callers from HttpClient via exceptions. I am actually leaning more towards closing it as By Design / Won’t Fix.

If you are interested in this feature, DESPITE the fact that HttpClient provides a reasonable way to get to StatusCode (even without exceptions being thrown), can you please:

1. Upvote top post (currently there are 0 upvotes)
2. Clarify your use case - why is using (slower) exception handling better/easier for your code? Is it just because of migration from HttpWebRequest, or is more involved?

Thanks!

(closed by accident - the buttons are too close to each other 😦, sorry)

the old WebClient api provided a mechanism for retrieving this.

The prior Web APIs like HttpWebRequest would always throw a WebException for any non-successful (non 2xx) status code. And it would throw WebException as well for other network errors. So, it was important and useful that the WebException had HTTP response information (such as StatusCode) in the exception. That was because there may or may not be an actual HTTP protocol response even though the WebException is thrown.

In today’s HttpClient API, HttpRequestException is not thrown by the HttpClient APIs except for networking errors (like can’t connect etc). All HTTP responses containing any StatusCode will be returned. This includes success status codes like 200 or non-success status codes like 500.

The discussion is this issue derives from the use of the EnsureSuccessStatusCode API which can be called subsequently after the regular HttpClient APIs. Calling that API will thrown an HttpRequestException if the StatusCode contained in the HttpResponseMessage is a non-success status code.

However, the StatusCode can still be obtained easily from the original HttpResponseMessage returned by the HttpClient APIs prior to any call of EnsureSuccessStatusCode. So, the use pattern of HttpClient APIs is very different from HttpWebRequest APIs.

This is why it doesn’t seem as important to add a StatusCode field to the HttpRequestException because in the majority of cases, there won’t be any StatusCode at all (it would have to be a nullable field) since the HttpRequestException thrown by HttpClient APIs typically is due to networking errors where no HTTP response message is ever returned.

So, the argument here is whether or not it is a good API design to add a field to the HttpRequestException object where most of the time it will be null.

@davidsh to see comment above since it seems that several folks are interested in this. I agree that a derived exception type is rarely considered breaking, but I don’t have context on the issue itself.

I’m working on an API proposal that will extend HttpRequestException to provide for this StatusCode property as well as another property to handle storing platform-independent error information (similar to WebException.Status property). See dotnet/corefx#19185

I’d like to see this too - preferably as a nullable property on the existing exception type.

The GetStringAsync, GetByteArrayAsync and GetStreamAsync helper methods on HttpClient currently don’t provide any way to determine why a request failed, and this proposal (whichever form it takes) would solve the common scenarios for me.