softwerkab / fortnox-csharp-api-sdk

.NET SDK for Fortnox API.
MIT License
53 stars 65 forks source link

RateLimiter not working when auth is controlled by DelegatingHandler #273

Open sblomstrand opened 1 year ago

sblomstrand commented 1 year ago

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 commented 1 year ago

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

richardrandak commented 1 year ago

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 commented 1 year ago

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.