Add "retry-after" information to abuse detection exceptions

[SeanKilleen]

Offering a suggestion that I'm willing to look at implementing as well.

I was having a little too much fun exploring Github with some concurrent actor processes and tripped the abuse mechanism. I received a ForbiddenException with the reason and pointing me to Github's Abuse Rate Limits Guide.

There, I see that a header exists for Retry-After -- an integer representing the number of seconds to wait. Would be awesome to be able to retrieve this information so that I could respect it in the event that an abuse-style exception is triggered.

Current way

I think I could currently achieve this, catching the ForbiddenException, and then getting the headers directly from the IResponse passed in as part of the HttpResponse field. Is that correct?

A better way?

Would it make sense to inherit this exception type from Forbidden exception and supply the Retry-After value directly in the Exception?

Please let me know if there's interest in this. Thanks!

[ryangribble]

Hey @SeanKilleen that sounds like a great idea

inherit this exception type from Forbidden exception and supply the Retry-After value directly in the Exception?

This option sounds 👍 to me

[SeanKilleen]

@ryangribble sounds good. I'll plan to tackle this in the following way:

• Create a new Exception Inheriting from ForbiddenException
• Modify Connection.cs to throw the new exception (with tests)
• Add the Retry-After as a readonly field within the constructor based on the headers from the IResponse that are passed in.

[SeanKilleen]

@ryangribble any preferences on what to do if the Retry-After header isn't present or can't be parsed? We could default to a number, e.g. 60 seconds, or we could make the field a nullable int.

For now, will proceed with idea of a default value of 60 seconds.

[ryangribble]

Personally im not a fan of a non official default that we make up ourselves. I think make it nullable and if the value can't be parsed/isn't present then return null

[SeanKilleen]

Sure thing -- I'll make those adjustments.

[SeanKilleen]

Also, I added the failing test back. Normally my preference would have been to open a new PR against #1529 but this works fine too.