Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RateLimiter not working when auth is controlled by DelegatingHandler #273

Open
sblomstrand opened this issue Apr 3, 2023 · 3 comments
Open

Comments

@sblomstrand
Copy link

sblomstrand commented Apr 3, 2023

Since moving to the new Api we have started experiencing exceptions due to the rate-limiter.
Doing a little digging we discovered that the reason for this is the combination of our implementation authorization along with the call to the rate-limiters Throttle-method in BaseClient.

Background: We use Fortnox in a distributed environment and need to keep the access token available to multiple clients. To be able to do this we use a DelegatingHandler to the HttpClient that we inject into the FortnoxClient. In the handler we retrieve and set the Authorization-header to the correct token.

In the BaseClients SendAsync-method the throttling is performed on Authorization?.AccessToken which in our case has not been set yet (it will be se on the next line when the call to HttpClient.SendAsync is performed.

The RateLimiters Throttle does not throttle anything if the AccessToken is null.

RateLimiter:

    public async Task Trottle(string token)
    {
        if (token == null)
            return;

        var limiter = SelectRateLimiter(token);
        await limiter;
    }

BaseClient:

    public async Task<byte[]> SendAsync(HttpRequestMessage request)
    {
        try
        {
            Authorization?.ApplyTo(request);

            if (UseRateLimiter)
                await RateLimiter.Trottle(Authorization?.AccessToken).ConfigureAwait(false);

            using var response = await HttpClient.SendAsync(request).ConfigureAwait(false);

            if (response.IsSuccessStatusCode)
                return await response.Content.ReadAsByteArrayAsync().ConfigureAwait(false);

            await ErrorHandler.HandleErrorResponseAsync(response).ConfigureAwait(false);
            return default;
        }
        catch (HttpRequestException ex)
        {
            ErrorHandler.HandleNoResponse(ex);
            return default;
        }
    }

A suggested solution:
Expose a RateLimiterToken when creating a new FortnoxClient and use that if there is no AccessToken in BaseClient:

var client = new FortnoxClient {
   ...
   UseRateLimiter = true,
   RateLimiterToken = "some-token"
}

BaseClient:

if (UseRateLimiter)
   await RateLimiter.Trottle(Authorization?.AccessToken ?? RateLimiterToken).ConfigureAwait(false);

@sblomstrand
Copy link
Author

Another option would be changing the internal ApplyTo function in FortnoxAuthorization to public ApplyTo so that we can implement our own version.

@richardrandak
Copy link
Collaborator

Hello! I have released a nuget where to ApplyTo is set to public. Hope this helps you to create your implementation of the authorization.

@sblomstrand
Copy link
Author

Super, thanks!
Would it be to much to suggest a breaking change?
Task ApplyToAsync would let us handle the refresh in this method as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants