"Retry-after" header not part of EgnyteApiException

egnyte / egnyte-dotnet

.NET SDK for the Egnyte Public API

5 stars 17 forks source link

"Retry-after" header not part of EgnyteApiException #3

Closed GrahamTheCoder closed 5 years ago

GrahamTheCoder commented 6 years ago

On https://developers.egnyte.com/Frequently_Asked_Questions#df7 it says that when the quota limit is hit, the correct retry period will be returned in the header. But that information is thrown away here: https://github.com/egnyte/egnyte-dotnet/blob/c5f31cc2cff1a70ab4842b0ae69799139b28ef9c/Egnyte.Api/Common/ServiceHandler.cs#L45

Possible solutions:

Provide a special exception type EgnyteRateLimitException extending EgnyteApiException but with an extra property of RetryAfter.
Have an option for the client code to do the retry itself automatically after the correct period
Take the general approach of adding the headers to the data dictionary of the exception

P.S. Thanks for the useful repo!

naugtur commented 6 years ago

That shouldn't be hard to fix. I'll see what I can get arranged. In the meantime - we're open to pull requests.

But for now I can tell you the retry value doesn't change. It's either 1 second if you run out of your QPS limit or a day if you've reached the daily quota.

Realistically, it only makes sense to implement a retry for the first case, which is - pause all requests for a full second and then continue.

GrahamTheCoder commented 6 years ago

Thanks, makes sense. I've also noticed the error message something like "Developer over QPS" is sometimes returned as if it's data on download file, but I haven't pinned down why, and it could be the underlying api's issue

naugtur commented 6 years ago

Your key is limited to 2 requests per second per user, so you need to send them at that pace to get the most out of it. If you try to download something, only accept body of a HTTP200 as the content.

Also, have you seen this? https://egnyte.github.io/integrations-cookbook/throttling.html

GrahamTheCoder commented 6 years ago

Thanks, I have seen that. The problem is that the API sometimes returns a status code in the 2xx range but sets the content to <h1>Developer Over Qps</h1> for some reason.

mikuam commented 5 years ago

@GrahamTheCoder I just published nuget package version v.0.1.18, that extends EgnyteApiException with all headers from the response. Please check if that works for you.

GrahamTheCoder commented 5 years ago

Sorry I didn't get around to trying it yet, thanks for the fix!