Added missing status code 429 Too Many Requests

[KeithHenry]

This enumeration is missing HTTP status code 429 Too Many Requests - this is used by many rate-limited services to indicate that the current allocation has been exceeded.

For example the British government's companies registry service does this: https://developer.companieshouse.gov.uk/api/docs/index/gettingStarted/rateLimiting.html

.NET should be able to handle and create responses with this status code.

[JonHanna]

If adding one status from RFC 6585, it seems reasonable to add the rest (428, 431 & 511) at the same time.

[KeithHenry]

It does, though only 429 is causing me a problem. Shall I add that commit?

[KeithHenry]

@JonHanna done, cheers for the suggestion :-)

[davidsh]

cc: @karelz

Thx for the PR. However, this is an API change and would require changes to the REF folder. In addition, there is an API review process that needs to be followed first.

We are also tracking adding new values to this enumeration here: #4382.

So, it would probably be best to close this PR and to use issue #4382 to propose the full set of additions to this enumeration.

[dnfclas]

All CLA requirements met.

[KeithHenry]

Cheers @davidsh, I've updated the copy in the ref folder.

Adding enums is an API change? If so it has to be one that can change quickly (well, relatively, RFC 6585 being from 2012 and #4382 from 2015), and while HTTP codes are a standard has it been decided that .NET's HttpClient can only consume services that absolutely stick to that standard?

To be honest I think this needs an API change. I mean that HttpClient should be able to handle any HTTP status code, even arbitrary ones made up by wilfully bad APIs, and an enum be something that helps develop for standard ones.

I just want to get this bug fixed (and it is a bug).

[karelz]

@KeithHenry Touching any public surface is API change. API review checks also name consistency with other BCL types, etc.

If so it has to be one that can change quickly (well, relatively, RFC 6585 being from 2012 and #4382 from 2015), and while HTTP codes are a standard has it been decided that .NET's HttpClient can only consume services that absolutely stick to that standard?
To be honest I think this needs an API change. I mean that HttpClient should be able to handle any HTTP status code, even arbitrary ones made up by wilfully bad APIs, and an enum be something that helps develop for standard ones.

HttpClient is built on top of system libraries - WinHTTP and libcurl. They do most of the handling and as such define most of the "standard". This enum does not limit functionality of HttpClient, it is a convenience.

I just want to get this bug fixed (and it is a bug).

Definition of bug vs. feature is often biased opinion. Nevertheless, we should run it by API review first. It may take a week or two as we are super-busy reviewing APIs critical for 2.1 at this moment (just to set expectations).

[KeithHenry]

@karelz am I missing something?

This enum does not limit functionality of HttpClient, it is a convenience.

This started because I have an external API that returns code 429 once a limit has been reached - either WinHTTP or cURL could handle that result, but HttpClient can't - there is no value that HttpResponseMessage.StatusCode can hold.

Is there some way to get the underlying code from HttpClient that I'm missing?

Touching any public surface is API change

I understand that, but this enum is not coming from a source internal to .NET - is this .NET's API to regulate?

It may take a week or two

#4382 is 3 years old. Is there a way to push for a review sooner or help it along?

[karelz]

@KeithHenry I think there is a set of misunderstandings here ...

This started because I have an external API that returns code 429 once a limit has been reached - either WinHTTP or cURL could handle that result, but HttpClient can't - there is no value that HttpResponseMessage.StatusCode can hold.

Enums can hold any value of the underlying type (int in this case). The enum is just about pretty easy-to-use names. Existence of an easy name is not going to make something magically work. Technically speaking, you can use the "missing" codes even today.
If there is something HttpClient can't handle, then it is a separate issue and should be discussed separately. This PR will not make it go away. Note also, that the underlying implementation of HttpClient is WinHttp/libcurl, so if they don't handle it automatically today, it would be good to find out why (maybe just some missing settings, or they can't handle it at all? - good thing to discuss in separate issue with more details like minimal repro, etc.).

I understand that, but this enum is not coming from a source internal to .NET - is this .NET's API to regulate?

I don't follow. Did you mean external source? Either way, names are something that is also API-reviewed. I don't expect it to take longer than few minutes, but it is something we should do.
What is obvious and clear to one person is often not so clear to others, or should take larger context in consideration (esp. naming, patterns, etc.) - that is the key value of API review.

#4382 is 3 years old. Is there a way to push for a review sooner or help it along?

Technically, the issue is closer to 2 years old. I agree that it would be nice to have it done already, given how simple it is.
However, given the discussion on the issue and the fact no one volunteered to put together solid full list of all missing codes with names, etc. for API review tells me that maybe it was not so desperately needed after all (given there is alternative way, preferred by some). Either way, those things are now unblocked and we can progress further.
If the API reviewers were not busy during Jan and Feb doing 3x more API reviews as usual, I would be ok to push for it harder (given it is rather old, simple and we have a seriously interested contributor). However, we are now finishing some important larger API reviews which are critical for .NET Core 2.1 - they need to get in first and I don't want to jeopardize them by nice-to-have convenience APIs. I don't see huge additional harm to delay few more extra days/weeks after 2+years (given the circumstances). It is not that we do not want it, it is just about timing. Thanks for understanding!

[davidsh]

@KeithHenry Thank you very much for your PR on this. The .NET Core team was able to close on the expanded set of HttpStatusCode values to add to the enum as part of a final API review #4382

A new PR #26727 was opened up that now supersedes this PR. So, I'll close this PR.

[lindexi]

See https://github.com/dotnet/runtime/blob/master/src/libraries/System.Net.Primitives/src/System/Net/HttpStatusCode.cs