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

Handle rate limit in SDKs #77

Merged
merged 19 commits into from
Jan 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/lovely-readers-count.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'dotnet-sdk': minor
'@featureboard/node-sdk': minor
'@featureboard/js-sdk': minor
---

Add support for 429 Too Many Requests with backoff
5 changes: 5 additions & 0 deletions .changeset/shy-wolves-talk.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@featureboard/contracts': minor
---

Added Too Many Requests error
18 changes: 16 additions & 2 deletions libs/contracts/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,16 @@
export * from './lib/messages'
export * from './lib/notification-types'
export { TooManyRequestsError } from './lib/error-types'
export type { ClientMessages, SubscribeToEnvironment } from './lib/messages'
export type {
AudienceExceptionValue,
EffectiveFeatureValue,
FeatureAvailable,
FeatureConfiguration,
FeatureUnavailable,
FeatureValueAvailableNotification,
FeatureValueNotification,
FeatureValueUpdatedNotification,
NotificationType,
StateOfTheWorldEffectiveValuesNotification,
StateOfTheWorldNotification,
SubscriptionErrorNotification,
} from './lib/notification-types'
13 changes: 13 additions & 0 deletions libs/contracts/src/lib/error-types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
export class TooManyRequestsError extends Error {
statusCode: number
retryAfter: Date
timestamp: Date

constructor(message = 'Too Many Requests', retryAfter: Date) {
super(message)
this.name = 'TooManyRequestsError'
this.statusCode = 429
this.retryAfter = retryAfter
this.timestamp = new Date()
}
}
140 changes: 137 additions & 3 deletions libs/dotnet-sdk-test/FeatureBoardHttpClientTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@
using Moq;
using FeatureBoard.DotnetSdk.Models;
using System.Net.Http.Headers;
using Shouldly;

namespace FeatureBoard.DotnetSdk.Test
{
public class FeatureBoardHttpClientTests
{
private const string TestETag = @"""test""";
private EntityTagHeaderValue? _nullETag = null;
private RetryConditionHeaderValue? _retryAfter = null;

private static readonly Expression<Func<HttpRequestMessage, bool>> _defaultRequestMatcher = msg => msg.Method == HttpMethod.Get && msg.RequestUri!.OriginalString == FeatureBoardHttpClient.Action;

Expand Down Expand Up @@ -44,7 +46,7 @@ public async Task ItReturnsCorrectListOfFeatures()
IReadOnlyCollection<FeatureConfiguration>? actionArg = null;
void captureArgAction(IReadOnlyCollection<FeatureConfiguration> features) => actionArg = features;

var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, captureArgAction, new NullLogger<FeatureBoardHttpClient>());
var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, () => ref _retryAfter, captureArgAction, new NullLogger<FeatureBoardHttpClient>());

// Act
var result = await testSubject.RefreshFeatureConfiguration(CancellationToken.None);
Expand Down Expand Up @@ -86,7 +88,7 @@ static void nopAction(IReadOnlyCollection<FeatureConfiguration> features) { }
.Setup(client => client.SendAsync(It.Is<HttpRequestMessage>(hasEtagMatcher), It.IsAny<CancellationToken>()))
.ReturnsAsync(new HttpResponseMessage() { StatusCode = HttpStatusCode.NotModified });

var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, nopAction, new NullLogger<FeatureBoardHttpClient>());
var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, () => ref _retryAfter, nopAction, new NullLogger<FeatureBoardHttpClient>());

// Act
var initialResult = await testSubject.RefreshFeatureConfiguration(CancellationToken.None);
Expand All @@ -97,6 +99,138 @@ static void nopAction(IReadOnlyCollection<FeatureConfiguration> features) { }
Assert.False(subsequentResult);
}

[Fact]
public async Task ItDoesNotProcessResponseIfTooManyRequests()
{
// Arrange
var content = JsonContent.Create(new[] { testFeatureConfig });
static void nopAction(IReadOnlyCollection<FeatureConfiguration> features) { }
var countHttpClient = 0;
Expression<Func<HttpRequestMessage, bool>> hasEtagMatcher = msg => _defaultRequestMatcher.Compile()(msg) && msg.Headers.IfNoneMatch.Any(t => t.Equals(new EntityTagHeaderValue(TestETag)));
_mockHttpClient
.Setup(client => client.SendAsync(It.Is<HttpRequestMessage>(hasEtagMatcher), It.IsAny<CancellationToken>()))
.ReturnsAsync((HttpRequestMessage request, CancellationToken _) =>
{
countHttpClient++;
if (countHttpClient == 1)
{
var response = new HttpResponseMessage(HttpStatusCode.TooManyRequests);
response.Headers.RetryAfter = new RetryConditionHeaderValue(new DateTimeOffset(DateTime.UtcNow.AddSeconds(1)));
return response;
}
else
{
var response = new HttpResponseMessage() { Content = content, RequestMessage = request };
response.Headers.ETag = new EntityTagHeaderValue(TestETag);
return response;
}
});

var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, () => ref _retryAfter, nopAction, new NullLogger<FeatureBoardHttpClient>());

// Act
var initialResult = await testSubject.RefreshFeatureConfiguration(CancellationToken.None);
var subsequentResult = await testSubject.RefreshFeatureConfiguration(CancellationToken.None);
Assert.Equal(1, countHttpClient);
var subsequentResult2 = await testSubject.RefreshFeatureConfiguration(CancellationToken.None);
Assert.Equal(1, countHttpClient);
Thread.Sleep(1000);
var subsequentResult3 = await testSubject.RefreshFeatureConfiguration(CancellationToken.None);

// Assert
Assert.True(initialResult);
Assert.False(subsequentResult);
Assert.False(subsequentResult2);
Assert.True(subsequentResult3);
Assert.True(_retryAfter == null);
Assert.Equal(2, countHttpClient);
}


[Fact]
public async Task ItDoesNotProcessResponseIfTooManyRequestsRetryAfterHeaderDate()
{
// Arrange
static void nopAction(IReadOnlyCollection<FeatureConfiguration> features) { }
var retryAfterDate = new DateTimeOffset(DateTime.UtcNow.AddSeconds(1));

Expression<Func<HttpRequestMessage, bool>> hasEtagMatcher = msg => _defaultRequestMatcher.Compile()(msg) && msg.Headers.IfNoneMatch.Any(t => t.Equals(new EntityTagHeaderValue(TestETag)));
_mockHttpClient
.Setup(client => client.SendAsync(It.Is<HttpRequestMessage>(hasEtagMatcher), It.IsAny<CancellationToken>()))
.ReturnsAsync((HttpRequestMessage request, CancellationToken _) =>
{
var response = new HttpResponseMessage(HttpStatusCode.TooManyRequests);
response.Headers.RetryAfter = new RetryConditionHeaderValue(retryAfterDate);
return response;
});

var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, () => ref _retryAfter, nopAction, new NullLogger<FeatureBoardHttpClient>());

// Act
var initialResult = await testSubject.RefreshFeatureConfiguration(CancellationToken.None);
var subsequentResult = await testSubject.RefreshFeatureConfiguration(CancellationToken.None);

// Assert
Assert.True(initialResult);
Assert.False(subsequentResult);
Assert.True(_retryAfter != null && _retryAfter.Date == retryAfterDate);
}

[Fact]
public async Task ItDoesNotProcessResponseIfTooManyRequestsRetryAfterHeaderSeconds()
{
// Arrange
static void nopAction(IReadOnlyCollection<FeatureConfiguration> features) { }

Expression<Func<HttpRequestMessage, bool>> hasEtagMatcher = msg => _defaultRequestMatcher.Compile()(msg) && msg.Headers.IfNoneMatch.Any(t => t.Equals(new EntityTagHeaderValue(TestETag)));
_mockHttpClient
.Setup(client => client.SendAsync(It.Is<HttpRequestMessage>(hasEtagMatcher), It.IsAny<CancellationToken>()))
.ReturnsAsync((HttpRequestMessage request, CancellationToken _) =>
{
var response = new HttpResponseMessage(HttpStatusCode.TooManyRequests);
response.Headers.RetryAfter = new RetryConditionHeaderValue(TimeSpan.FromSeconds(1));
return response;
});

var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, () => ref _retryAfter, nopAction, new NullLogger<FeatureBoardHttpClient>());

// Act
var initialResult = await testSubject.RefreshFeatureConfiguration(CancellationToken.None);
var subsequentResult = await testSubject.RefreshFeatureConfiguration(CancellationToken.None);

// Assert
Assert.True(initialResult);
Assert.False(subsequentResult);
Assert.True(_retryAfter != null && _retryAfter.Date != null);
}

[Fact]
public async Task ItDoesNotProcessResponseIfTooManyRequestsRetryAfterHeaderNull()
{
// Arrange
static void nopAction(IReadOnlyCollection<FeatureConfiguration> features) { }

Expression<Func<HttpRequestMessage, bool>> hasEtagMatcher = msg => _defaultRequestMatcher.Compile()(msg) && msg.Headers.IfNoneMatch.Any(t => t.Equals(new EntityTagHeaderValue(TestETag)));
_mockHttpClient
.Setup(client => client.SendAsync(It.Is<HttpRequestMessage>(hasEtagMatcher), It.IsAny<CancellationToken>()))
.ReturnsAsync((HttpRequestMessage request, CancellationToken _) =>
{
var response = new HttpResponseMessage(HttpStatusCode.TooManyRequests);
return response;
});

var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, () => ref _retryAfter, nopAction, new NullLogger<FeatureBoardHttpClient>());

// Act
var initialResult = await testSubject.RefreshFeatureConfiguration(CancellationToken.None);
var subsequentResult = await testSubject.RefreshFeatureConfiguration(CancellationToken.None);

// Assert
Assert.True(initialResult);
Assert.False(subsequentResult);
Assert.True(_retryAfter != null && _retryAfter.Date != null);
}


[Theory]
[InlineData(HttpStatusCode.NotFound)]
Expand All @@ -112,7 +246,7 @@ public async Task ItDoesNotProcessResponseOnNonOkayResponse(HttpStatusCode httpS
_mockHttpClient
.Setup(client => client.SendAsync(It.Is<HttpRequestMessage>(_defaultRequestMatcher), It.IsAny<CancellationToken>()))
.ReturnsAsync((HttpRequestMessage request, CancellationToken _) => new HttpResponseMessage(httpStatusCode) { RequestMessage = request });
var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, exceptionAction, new NullLogger<FeatureBoardHttpClient>());
var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, () => ref _retryAfter, exceptionAction, new NullLogger<FeatureBoardHttpClient>());

// Act
var result = await testSubject.RefreshFeatureConfiguration(CancellationToken.None);
Expand Down
42 changes: 41 additions & 1 deletion libs/dotnet-sdk/FeatureBoardHttpClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,27 @@
namespace FeatureBoard.DotnetSdk;

public delegate ref EntityTagHeaderValue? LastETagProvider();
public delegate ref RetryConditionHeaderValue? RetryAfterProvider();

internal sealed class FeatureBoardHttpClient : IFeatureBoardHttpClient
{
internal static readonly string Action = "all";

private LastETagProvider _eTag;

private RetryAfterProvider _retryAfter;
private readonly HttpClient _httpClient;
private readonly Action<IReadOnlyCollection<FeatureConfiguration>> _processResult;
private readonly ILogger _logger;


public FeatureBoardHttpClient(HttpClient httpClient, LastETagProvider lastModifiedTimeProvider, Action<IReadOnlyCollection<FeatureConfiguration>> processResult, ILogger<FeatureBoardHttpClient> logger)
public FeatureBoardHttpClient(HttpClient httpClient, LastETagProvider lastModifiedTimeProvider, RetryAfterProvider retryAfterProvider, Action<IReadOnlyCollection<FeatureConfiguration>> processResult, ILogger<FeatureBoardHttpClient> logger)
{
_httpClient = httpClient;
_processResult = processResult;
_logger = logger;
_eTag = lastModifiedTimeProvider;
_retryAfter = retryAfterProvider;
}

public async Task<bool?> RefreshFeatureConfiguration(CancellationToken cancellationToken)
Expand All @@ -33,14 +37,34 @@ public FeatureBoardHttpClient(HttpClient httpClient, LastETagProvider lastModifi
if (null != eTag)
request.Headers.IfNoneMatch.Add(eTag);

var retryAfter = _retryAfter();
if (retryAfter != null && retryAfter.Date > DateTimeOffset.Now)
{
// Do not call API
_logger.LogWarning("Failed to get latest flags. Service returned error 429 (Too Many Requests). Retry after: {retryAfter}", retryAfter.Date.Value.ToString());
idadaniels marked this conversation as resolved.
Show resolved Hide resolved
return false;
}
using var response = await _httpClient.SendAsync(request, cancellationToken);

retryAfter = updateRetryAfterRef(response.Headers.RetryAfter);

switch (response.StatusCode)
{
case HttpStatusCode.NotModified:
_logger.LogDebug("No changes");
return false;

//HttpStatusCode.TooManyRequests not defined in .NET Standards 2.0
case (HttpStatusCode)429:
idadaniels marked this conversation as resolved.
Show resolved Hide resolved
if (response.Headers.RetryAfter == null)
{
// No retry after header set, hold back call to client api for 60 seconds
var retryAfterDate = DateTimeOffset.Now.AddMinutes(1);
retryAfter = updateRetryAfterRef(new RetryConditionHeaderValue(retryAfterDate));
}
_logger.LogWarning("Failed to get latest flags. Service returned error 429 (Too Many Requests). Retry after: {retryAfter}", retryAfter!.Date!.Value.ToString());
return false;

case not HttpStatusCode.OK:
_logger.LogError("Failed to get latest flags: Service returned error {statusCode}({responseBody})", response.StatusCode, await response.Content.ReadAsStringAsync());
return null;
Expand All @@ -59,6 +83,22 @@ void updateEtagRef(EntityTagHeaderValue? responseTag) // Sync method to allow us
_logger.LogDebug("Fetching updates done, eTag={eTag}", _eTag);
}

RetryConditionHeaderValue? updateRetryAfterRef(RetryConditionHeaderValue? responseRetryAfter) // Sync method to allow use of eTag ref-local variable
{
ref var retryAfter = ref _retryAfter();
idadaniels marked this conversation as resolved.
Show resolved Hide resolved
if (responseRetryAfter?.Date == null && responseRetryAfter?.Delta != null)
{
var retryAfterDate = DateTimeOffset.Now + responseRetryAfter.Delta.Value;
retryAfter = new RetryConditionHeaderValue(retryAfterDate);
}
else
{
retryAfter = responseRetryAfter;
}

return _retryAfter();
}

return true;
}
}
2 changes: 2 additions & 0 deletions libs/dotnet-sdk/Registration/RegisterFeatureBoard.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ public static class RegisterFeatureBoard
private static UpdateStrategy _updateStrategy = UpdateStrategy.None;

private static EntityTagHeaderValue? lastETag = null;
private static RetryConditionHeaderValue? retryAfter = null;

public static FeatureBoardBuilder AddFeatureBoard<TFeatures>(this IServiceCollection services, IConfigurationRoot configuration) where TFeatures : class, IFeatures
{
Expand All @@ -28,6 +29,7 @@ public static FeatureBoardBuilder AddFeatureBoard<TFeatures>(this IServiceCollec
});

services.AddSingleton<LastETagProvider>(() => ref lastETag);
services.AddSingleton<RetryAfterProvider>(() => ref retryAfter);

services.AddScoped<IFeatureBoardClient<TFeatures>, FeatureBoardClient<TFeatures>>();
services.AddScoped<IFeatureBoardClient>(static c => c.GetRequiredService<IFeatureBoardClient<TFeatures>>());
Expand Down
27 changes: 22 additions & 5 deletions libs/js-sdk/src/ensure-single.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,31 @@
import { TooManyRequestsError } from '@featureboard/contracts'

/** De-dupes calls while the promise is in flight, otherwise will trigger again */
export function createEnsureSingle<T>(cb: () => Promise<T>): () => Promise<T> {
export function createEnsureSingleWithBackoff<T>(
cb: () => Promise<T>,
): () => Promise<T> {
let current: Promise<T> | undefined
let tooManyRequestsError: TooManyRequestsError | undefined

return () => {
if (
tooManyRequestsError &&
tooManyRequestsError.retryAfter > new Date()
) {
return Promise.reject(tooManyRequestsError)
}
if (!current) {
current = cb().finally(() => {
current = undefined
})
current = cb()
.catch((error: Error) => {
if (error instanceof TooManyRequestsError) {
tooManyRequestsError = error
}
throw error
})
.finally(() => {
current = undefined
})
}

return current
}
}
2 changes: 1 addition & 1 deletion libs/js-sdk/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export type { FeatureBoardClient } from './features-client'

// TODO We should make these 'internal' and not export them
// Need to figure out how to do that with the current build setup
export { createEnsureSingle } from './ensure-single'
export { createEnsureSingleWithBackoff } from './ensure-single'
export { featureBoardHostedService } from './featureboard-service-urls'
export { retry } from './utils/retry'

Expand Down
Loading