Return 429 Too Many Requests for gRPC error code ResourceExhausted from Trillian

[roger2hk]

The existing HTTP status code StatusForbidden does not reflect the actual gRPC error code ResourceExhausted from Trillian. 429 Too Many Requests is a better HTTP status code to fit into the translation.

https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/429

Checklist

• I have updated the CHANGELOG.
  • Adjust the draft version number according to semantic versioning rules.
• I have updated documentation accordingly.

[mhutchinson]

If this was a brand new API then I would 100% support this. Unfortunately this is an established API in a mature ecosystem, and this is a change to the implicit contract that clients may now be depending on. Is there a specific issue you're trying to address with this?

[roger2hk]

There is no issue reported. I came across this when I looked at Gunnar A's question in Slack. We merged a similar PR (#1313) a while ago, so that is why I find a better CTFE HTTP status code for the rate limit error response from Trillian.

[mhutchinson]

@AlCutter WDYT? The reason I'm hesitant with this is that we've had a lot of problems with rogue client code not backing off when we tell them to. It's been a while since we saw an influx of such bad behaviour. If we change this, it's possible that some of the clients that are currently behaving will start to misbehave.

OTOH, if we returned better status codes then it's more likely that clients will do the right thing.

[JulyMoon87]

Perhaps find the reason for massive misbehavior first

…

On Wed, Mar 20, 2024, 9:57 AM Martin Hutchinson ***@***.***> wrote: @AlCutter <https://github.com/AlCutter> WDYT? The reason I'm hesitant with this is that we've had a lot of problems with rogue client code not backing off when we tell them to. It's been a while since we saw an influx of such bad behaviour. If we change this, it's possible that some of the clients that are currently behaving will start to misbehave. OTOH, if we returned better status codes then it's more likely that clients will do the right thing. — Reply to this email directly, view it on GitHub <#1401 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ASY4UXI7OCR62MMH2PLGX3LYZGPWZAVCNFSM6AAAAABE7Q6OV6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMBZG43TGOBXG4> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[AlCutter]

Returning 429 is much more descriptive so ultimately seems like the right thing to do.

If we're worried about breaking clients with the change, perhaps we can pop this behaviour behind a flag, and try an A/B experiment for a bit to get a sense of how it'll affect traffic before deciding whether to make it default-on and remove the flag?

[roger2hk]

According to this thread in ct-policy forum, the error code 429 Too Many Requests is already returned from Cloudflare and DigiCert logs. I believe monitors should have already considered the 429 Too Many Requests in the implementations.

[roger2hk]

According to this thread in ct-policy forum, the error code 429 Too Many Requests is already returned from Cloudflare and DigiCert logs. I believe monitors should have already considered the 429 Too Many Requests in the implementations.

@mhutchinson WDYT?

[mhutchinson]

According to this thread in ct-policy forum, the error code 429 Too Many Requests is already returned from Cloudflare and DigiCert logs. I believe monitors should have already considered the 429 Too Many Requests in the implementations.

@mhutchinson WDYT?

Sure, let's take the chance.