diff --git a/.changeset/lovely-readers-count.md b/.changeset/lovely-readers-count.md new file mode 100644 index 00000000..71050646 --- /dev/null +++ b/.changeset/lovely-readers-count.md @@ -0,0 +1,7 @@ +--- +'dotnet-sdk': minor +'@featureboard/node-sdk': minor +'@featureboard/js-sdk': minor +--- + +Add support for 429 Too Many Requests with backoff diff --git a/.changeset/shy-wolves-talk.md b/.changeset/shy-wolves-talk.md new file mode 100644 index 00000000..ec76963f --- /dev/null +++ b/.changeset/shy-wolves-talk.md @@ -0,0 +1,5 @@ +--- +'@featureboard/contracts': minor +--- + +Added Too Many Requests error diff --git a/libs/contracts/src/index.ts b/libs/contracts/src/index.ts index 1ff2b6c3..3f64429e 100644 --- a/libs/contracts/src/index.ts +++ b/libs/contracts/src/index.ts @@ -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' diff --git a/libs/contracts/src/lib/error-types.ts b/libs/contracts/src/lib/error-types.ts new file mode 100644 index 00000000..7e8d0a88 --- /dev/null +++ b/libs/contracts/src/lib/error-types.ts @@ -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() + } +} diff --git a/libs/dotnet-sdk-test/FeatureBoardHttpClientTests.cs b/libs/dotnet-sdk-test/FeatureBoardHttpClientTests.cs index 000f0599..68382c08 100644 --- a/libs/dotnet-sdk-test/FeatureBoardHttpClientTests.cs +++ b/libs/dotnet-sdk-test/FeatureBoardHttpClientTests.cs @@ -8,6 +8,7 @@ using Moq; using FeatureBoard.DotnetSdk.Models; using System.Net.Http.Headers; +using Shouldly; namespace FeatureBoard.DotnetSdk.Test { @@ -15,6 +16,7 @@ public class FeatureBoardHttpClientTests { private const string TestETag = @"""test"""; private EntityTagHeaderValue? _nullETag = null; + private RetryConditionHeaderValue? _retryAfter = null; private static readonly Expression> _defaultRequestMatcher = msg => msg.Method == HttpMethod.Get && msg.RequestUri!.OriginalString == FeatureBoardHttpClient.Action; @@ -44,7 +46,7 @@ public async Task ItReturnsCorrectListOfFeatures() IReadOnlyCollection? actionArg = null; void captureArgAction(IReadOnlyCollection features) => actionArg = features; - var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, captureArgAction, new NullLogger()); + var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, () => ref _retryAfter, captureArgAction, new NullLogger()); // Act var result = await testSubject.RefreshFeatureConfiguration(CancellationToken.None); @@ -86,7 +88,7 @@ static void nopAction(IReadOnlyCollection features) { } .Setup(client => client.SendAsync(It.Is(hasEtagMatcher), It.IsAny())) .ReturnsAsync(new HttpResponseMessage() { StatusCode = HttpStatusCode.NotModified }); - var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, nopAction, new NullLogger()); + var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, () => ref _retryAfter, nopAction, new NullLogger()); // Act var initialResult = await testSubject.RefreshFeatureConfiguration(CancellationToken.None); @@ -97,6 +99,138 @@ static void nopAction(IReadOnlyCollection features) { } Assert.False(subsequentResult); } + [Fact] + public async Task ItDoesNotProcessResponseIfTooManyRequests() + { + // Arrange + var content = JsonContent.Create(new[] { testFeatureConfig }); + static void nopAction(IReadOnlyCollection features) { } + var countHttpClient = 0; + Expression> hasEtagMatcher = msg => _defaultRequestMatcher.Compile()(msg) && msg.Headers.IfNoneMatch.Any(t => t.Equals(new EntityTagHeaderValue(TestETag))); + _mockHttpClient + .Setup(client => client.SendAsync(It.Is(hasEtagMatcher), It.IsAny())) + .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()); + + // 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 features) { } + var retryAfterDate = new DateTimeOffset(DateTime.UtcNow.AddSeconds(1)); + + Expression> hasEtagMatcher = msg => _defaultRequestMatcher.Compile()(msg) && msg.Headers.IfNoneMatch.Any(t => t.Equals(new EntityTagHeaderValue(TestETag))); + _mockHttpClient + .Setup(client => client.SendAsync(It.Is(hasEtagMatcher), It.IsAny())) + .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()); + + // 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 features) { } + + Expression> hasEtagMatcher = msg => _defaultRequestMatcher.Compile()(msg) && msg.Headers.IfNoneMatch.Any(t => t.Equals(new EntityTagHeaderValue(TestETag))); + _mockHttpClient + .Setup(client => client.SendAsync(It.Is(hasEtagMatcher), It.IsAny())) + .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()); + + // 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 features) { } + + Expression> hasEtagMatcher = msg => _defaultRequestMatcher.Compile()(msg) && msg.Headers.IfNoneMatch.Any(t => t.Equals(new EntityTagHeaderValue(TestETag))); + _mockHttpClient + .Setup(client => client.SendAsync(It.Is(hasEtagMatcher), It.IsAny())) + .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()); + + // 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)] @@ -112,7 +246,7 @@ public async Task ItDoesNotProcessResponseOnNonOkayResponse(HttpStatusCode httpS _mockHttpClient .Setup(client => client.SendAsync(It.Is(_defaultRequestMatcher), It.IsAny())) .ReturnsAsync((HttpRequestMessage request, CancellationToken _) => new HttpResponseMessage(httpStatusCode) { RequestMessage = request }); - var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, exceptionAction, new NullLogger()); + var testSubject = new FeatureBoardHttpClient(_mockHttpClient.Object, () => ref _nullETag, () => ref _retryAfter, exceptionAction, new NullLogger()); // Act var result = await testSubject.RefreshFeatureConfiguration(CancellationToken.None); diff --git a/libs/dotnet-sdk/FeatureBoardHttpClient.cs b/libs/dotnet-sdk/FeatureBoardHttpClient.cs index 22cc88ed..9ed268ae 100644 --- a/libs/dotnet-sdk/FeatureBoardHttpClient.cs +++ b/libs/dotnet-sdk/FeatureBoardHttpClient.cs @@ -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> _processResult; private readonly ILogger _logger; - public FeatureBoardHttpClient(HttpClient httpClient, LastETagProvider lastModifiedTimeProvider, Action> processResult, ILogger logger) + public FeatureBoardHttpClient(HttpClient httpClient, LastETagProvider lastModifiedTimeProvider, RetryAfterProvider retryAfterProvider, Action> processResult, ILogger logger) { _httpClient = httpClient; _processResult = processResult; _logger = logger; _eTag = lastModifiedTimeProvider; + _retryAfter = retryAfterProvider; } public async Task RefreshFeatureConfiguration(CancellationToken cancellationToken) @@ -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()); + 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: + 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; @@ -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(); + 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; } } diff --git a/libs/dotnet-sdk/Registration/RegisterFeatureBoard.cs b/libs/dotnet-sdk/Registration/RegisterFeatureBoard.cs index 15a4dfd4..d01d8958 100644 --- a/libs/dotnet-sdk/Registration/RegisterFeatureBoard.cs +++ b/libs/dotnet-sdk/Registration/RegisterFeatureBoard.cs @@ -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(this IServiceCollection services, IConfigurationRoot configuration) where TFeatures : class, IFeatures { @@ -28,6 +29,7 @@ public static FeatureBoardBuilder AddFeatureBoard(this IServiceCollec }); services.AddSingleton(() => ref lastETag); + services.AddSingleton(() => ref retryAfter); services.AddScoped, FeatureBoardClient>(); services.AddScoped(static c => c.GetRequiredService>()); diff --git a/libs/js-sdk/src/ensure-single.ts b/libs/js-sdk/src/ensure-single.ts index f9bb6a6d..dc63fe84 100644 --- a/libs/js-sdk/src/ensure-single.ts +++ b/libs/js-sdk/src/ensure-single.ts @@ -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(cb: () => Promise): () => Promise { +export function createEnsureSingleWithBackoff( + cb: () => Promise, +): () => Promise { let current: Promise | 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 } } diff --git a/libs/js-sdk/src/index.ts b/libs/js-sdk/src/index.ts index ccbba94e..7975c36c 100644 --- a/libs/js-sdk/src/index.ts +++ b/libs/js-sdk/src/index.ts @@ -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' diff --git a/libs/js-sdk/src/tests/http-client.spec.ts b/libs/js-sdk/src/tests/http-client.spec.ts index 3fe18abf..19433add 100644 --- a/libs/js-sdk/src/tests/http-client.spec.ts +++ b/libs/js-sdk/src/tests/http-client.spec.ts @@ -1,4 +1,7 @@ -import type { EffectiveFeatureValue } from '@featureboard/contracts' +import { + TooManyRequestsError, + type EffectiveFeatureValue, +} from '@featureboard/contracts' import { HttpResponse, http } from 'msw' import { setupServer } from 'msw/node' import { describe, expect, it } from 'vitest' @@ -699,4 +702,190 @@ describe('http client', () => { server.close() } }) + + it('Retry to connect when received 429 from HTTP Client API during initialization', async () => { + const values: EffectiveFeatureValue[] = [ + { + featureKey: 'my-feature', + value: 'service-value', + }, + ] + + let count = 0 + const server = setupServer( + http.get('https://client.featureboard.app/effective', () => { + count++ + if (count <= 2) { + return new Response(null, { + status: 429, + headers: { 'Retry-After': '1' }, + }) + } + return HttpResponse.json(values, { + headers: { + etag: new Date().toISOString(), + }, + }) + }), + ) + server.listen({ onUnhandledRequest: 'error' }) + + try { + const httpClient = createBrowserClient({ + environmentApiKey: 'env-api-key', + audiences: [], + api: featureBoardHostedService, + updateStrategy: { kind: 'manual' }, + }) + await httpClient.waitForInitialised() + httpClient.close() + } finally { + server.resetHandlers() + server.close() + } + expect(count).toBe(3) + }) + + it('Block HTTP client API call after 429 response from HTTP Client API according to retry-after seconds', async () => { + const values: EffectiveFeatureValue[] = [ + { + featureKey: 'my-feature', + value: 'service-value', + }, + ] + + let count = 0 + const server = setupServer( + http.get( + 'https://client.featureboard.app/effective', + () => { + count++ + return HttpResponse.json(values, { + headers: { + etag: new Date().toISOString(), + }, + }) + }, + { once: true }, + ), + http.get( + 'https://client.featureboard.app/effective', + () => { + count++ + return new Response(null, { + status: 429, + headers: { 'Retry-After': '1' }, + }) + }, + { once: true }, + ), + http.get('https://client.featureboard.app/effective', () => { + count++ + return HttpResponse.json(values, { + headers: { + etag: new Date().toISOString(), + }, + }) + }), + ) + server.listen({ onUnhandledRequest: 'error' }) + + try { + const httpClient = createBrowserClient({ + environmentApiKey: 'env-api-key', + audiences: [], + api: featureBoardHostedService, + updateStrategy: { kind: 'manual' }, + }) + + await httpClient.waitForInitialised() + await expect(() => + httpClient.updateFeatures(), + ).rejects.toThrowError(TooManyRequestsError) + await expect(() => + httpClient.updateFeatures(), + ).rejects.toThrowError(TooManyRequestsError) + await new Promise((resolve) => setTimeout(resolve, 1000)) + await httpClient.updateFeatures() + + httpClient.close() + } finally { + server.resetHandlers() + server.close() + } + expect(count).toBe(3) + }) + + it('Block HTTP client API call after 429 response from HTTP Client API according to retry-after date', async () => { + const values: EffectiveFeatureValue[] = [ + { + featureKey: 'my-feature', + value: 'service-value', + }, + ] + + let count = 0 + const server = setupServer( + http.get( + 'https://client.featureboard.app/effective', + () => { + count++ + return HttpResponse.json(values, { + headers: { + etag: new Date().toISOString(), + }, + }) + }, + { once: true }, + ), + http.get( + 'https://client.featureboard.app/effective', + () => { + count++ + const retryAfter = new Date( + new Date().getTime() + 1000, + ).toUTCString() + return new Response(null, { + status: 429, + headers: { 'Retry-After': retryAfter }, + }) + }, + { once: true }, + ), + http.get('https://client.featureboard.app/effective', () => { + count++ + return HttpResponse.json(values, { + headers: { + etag: new Date().toISOString(), + }, + }) + }), + ) + server.listen({ onUnhandledRequest: 'error' }) + + try { + const httpClient = createBrowserClient({ + environmentApiKey: 'env-api-key', + audiences: [], + api: featureBoardHostedService, + updateStrategy: { kind: 'manual' }, + }) + + await httpClient.waitForInitialised() + await expect(() => + httpClient.updateFeatures(), + ).rejects.toThrowError(TooManyRequestsError) + await expect(() => + httpClient.updateFeatures(), + ).rejects.toThrowError(TooManyRequestsError) + await new Promise((resolve) => setTimeout(resolve, 1000)) + await httpClient.updateFeatures() + + httpClient.close() + } finally { + server.resetHandlers() + server.close() + } + expect(count).toBe(3) + }) }) diff --git a/libs/js-sdk/src/tests/mode-polling.spec.ts b/libs/js-sdk/src/tests/mode-polling.spec.ts index 96aba7d1..02b9a449 100644 --- a/libs/js-sdk/src/tests/mode-polling.spec.ts +++ b/libs/js-sdk/src/tests/mode-polling.spec.ts @@ -137,6 +137,67 @@ describe('Polling update mode', () => { server.close() } }) + + it('Do NOT throw error or make call to HTTP Client API when Too Many Requests (429) has been returned', async () => { + const values: EffectiveFeatureValue[] = [ + { + featureKey: 'my-feature', + value: 'service-default-value', + }, + ] + + let countAPICalls = 0 + const server = setupServer( + http.get( + 'https://client.featureboard.app/effective', + () => { + countAPICalls++ + return HttpResponse.json(values) + }, + { once: true }, + ), + http.get('https://client.featureboard.app/effective', () => { + countAPICalls++ + return new Response(null, { + status: 429, + headers: { 'Retry-After': '2' }, + }) + }), + ) + server.listen({ onUnhandledRequest: 'error' }) + + try { + const client = createBrowserClient({ + environmentApiKey: 'fake-key', + audiences: [], + updateStrategy: { + kind: 'polling', + options: { intervalMs: 100 }, + }, + }) + + await client.waitForInitialised() + // Wait for the interval to expire + await new Promise((resolve) => setTimeout(resolve, 100)) + const value1 = client.client.getFeatureValue( + 'my-feature', + 'default-value', + ) + expect(value1).toEqual('service-default-value') + // Wait for interval to expire + await new Promise((resolve) => setTimeout(resolve, 100)) + const value2 = client.client.getFeatureValue( + 'my-feature', + 'default-value', + ) + expect(value2).toEqual('service-default-value') + } finally { + server.resetHandlers() + server.close() + } + expect(countAPICalls).toBe(2) + expect.assertions(3) + }) }) declare module '@featureboard/js-sdk' { diff --git a/libs/js-sdk/src/update-strategies/createManualUpdateStrategy.ts b/libs/js-sdk/src/update-strategies/createManualUpdateStrategy.ts index 1c293cc7..73d7ee84 100644 --- a/libs/js-sdk/src/update-strategies/createManualUpdateStrategy.ts +++ b/libs/js-sdk/src/update-strategies/createManualUpdateStrategy.ts @@ -1,4 +1,4 @@ -import { createEnsureSingle } from '../ensure-single' +import { createEnsureSingleWithBackoff } from '../ensure-single' import { fetchFeaturesConfigurationViaHttp } from '../utils/fetchFeaturesConfiguration' import type { EffectiveConfigUpdateStrategy } from './update-strategies' import { updatesLog } from './updates-log' @@ -10,14 +10,16 @@ export function createManualUpdateStrategy( httpEndpoint: string, ): EffectiveConfigUpdateStrategy { let etag: undefined | string - let fetchUpdatesSingle: undefined | (() => Promise) + let fetchUpdatesSingle: + | undefined + | (() => Promise) return { async connect(stateStore) { // Force update etag = undefined // Ensure that we don't trigger another request while one is in flight - fetchUpdatesSingle = createEnsureSingle(async () => { + fetchUpdatesSingle = createEnsureSingleWithBackoff(async () => { etag = await fetchFeaturesConfigurationViaHttp( httpEndpoint, stateStore.audiences, diff --git a/libs/js-sdk/src/update-strategies/createPollingUpdateStrategy.ts b/libs/js-sdk/src/update-strategies/createPollingUpdateStrategy.ts index d5d8778f..c7372d0d 100644 --- a/libs/js-sdk/src/update-strategies/createPollingUpdateStrategy.ts +++ b/libs/js-sdk/src/update-strategies/createPollingUpdateStrategy.ts @@ -1,4 +1,5 @@ -import { createEnsureSingle } from '../ensure-single' +import { error } from 'console' +import { createEnsureSingleWithBackoff } from '../ensure-single' import { fetchFeaturesConfigurationViaHttp } from '../utils/fetchFeaturesConfiguration' import { pollingUpdates } from '../utils/pollingUpdates' import type { EffectiveConfigUpdateStrategy } from './update-strategies' @@ -21,7 +22,7 @@ export function createPollingUpdateStrategy( etag = undefined // Ensure that we don't trigger another request while one is in flight - fetchUpdatesSingle = createEnsureSingle(async () => { + fetchUpdatesSingle = createEnsureSingleWithBackoff(async () => { etag = await fetchFeaturesConfigurationViaHttp( httpEndpoint, stateStore.audiences, @@ -39,7 +40,9 @@ export function createPollingUpdateStrategy( if (fetchUpdatesSingle) { pollingUpdatesDebugLog('Polling for updates (%o)', etag) // Catch errors here to ensure no unhandled promise rejections after a poll - return fetchUpdatesSingle().catch(() => {}) + return fetchUpdatesSingle().catch((error: Error) => { + updatesLog(error) + }) } }, intervalMs) diff --git a/libs/js-sdk/src/utils/fetchFeaturesConfiguration.ts b/libs/js-sdk/src/utils/fetchFeaturesConfiguration.ts index 85b20228..aa9cd1e4 100644 --- a/libs/js-sdk/src/utils/fetchFeaturesConfiguration.ts +++ b/libs/js-sdk/src/utils/fetchFeaturesConfiguration.ts @@ -1,4 +1,7 @@ -import type { EffectiveFeatureValue } from '@featureboard/contracts' +import { + TooManyRequestsError, + type EffectiveFeatureValue, +} from '@featureboard/contracts' import type { EffectiveFeatureStateStore } from '../effective-feature-state-store' import { getEffectiveEndpoint } from '../update-strategies/getEffectiveEndpoint' import { compareArrays } from './compare-arrays' @@ -11,7 +14,7 @@ export async function fetchFeaturesConfigurationViaHttp( stateStore: EffectiveFeatureStateStore, etag: string | undefined, getCurrentAudiences: () => string[], -) { +): Promise { const effectiveEndpoint = getEffectiveEndpoint( featureBoardEndpoint, audiences, @@ -28,6 +31,34 @@ export async function fetchFeaturesConfigurationViaHttp( }, }) + if (response.status === 429) { + // Too many requests + const retryAfterHeader = response.headers.get('Retry-After') + const retryAfterInt = retryAfterHeader + ? parseInt(retryAfterHeader, 10) + : 60 + const retryAfter = + retryAfterHeader && !retryAfterInt + ? new Date(retryAfterHeader) + : new Date() + + if (retryAfterInt) { + const retryAfterTime = retryAfter.getTime() + retryAfterInt * 1000 + retryAfter.setTime(retryAfterTime) + } + + throw new TooManyRequestsError( + `Failed to get latest features: Service returned ${ + response.status + }${response.statusText ? ' ' + response.statusText : ''}. ${ + retryAfterHeader + ? 'Retry after: ' + retryAfter.toUTCString() + : '' + } `, + retryAfter, + ) + } + if (response.status !== 200 && response.status !== 304) { httpClientDebug( `Failed to fetch updates (%o): ${ diff --git a/libs/js-sdk/src/utils/retry.ts b/libs/js-sdk/src/utils/retry.ts index 01c594fa..cc4380d5 100644 --- a/libs/js-sdk/src/utils/retry.ts +++ b/libs/js-sdk/src/utils/retry.ts @@ -1,3 +1,4 @@ +import { TooManyRequestsError } from '@featureboard/contracts' import { debugLog } from '../log' const maxRetries = 5 @@ -12,6 +13,7 @@ export async function retry( try { return await fn() } catch (error) { + let retryAfterMs = 0 if (cancellationToken?.cancel) { debugLog('Cancel retry function') return Promise.resolve() @@ -20,7 +22,21 @@ export async function retry( // Max retries throw error } - const delayMs = initialDelayMs * Math.pow(backoffFactor, retryAttempt) + if ( + error instanceof TooManyRequestsError && + error.retryAfter > new Date() + ) { + retryAfterMs = error.retryAfter.getTime() - new Date().getTime() + } + const delayMs = + retryAfterMs === 0 + ? initialDelayMs * Math.pow(backoffFactor, retryAttempt) + : retryAfterMs + if (delayMs > 180000) { + // If delay is longer than 3 min throw error + // Todo: Replace with cancellation token with timeout + throw error + } await delay(delayMs) // Wait for the calculated delay return retry(fn, cancellationToken, retryAttempt + 1) // Retry the operation recursively } diff --git a/libs/node-sdk/src/tests/http-client.spec.ts b/libs/node-sdk/src/tests/http-client.spec.ts index 90af9b80..85003ec6 100644 --- a/libs/node-sdk/src/tests/http-client.spec.ts +++ b/libs/node-sdk/src/tests/http-client.spec.ts @@ -1,4 +1,7 @@ -import type { FeatureConfiguration } from '@featureboard/contracts' +import { + TooManyRequestsError, + type FeatureConfiguration, +} from '@featureboard/contracts' import { featureBoardHostedService } from '@featureboard/js-sdk' import { HttpResponse, http } from 'msw' import { setupServer } from 'msw/node' @@ -197,6 +200,190 @@ describe('http client', () => { } }) + it('Retry to connect when received 429 from HTTP Client API during initialization', async () => { + const values: FeatureConfiguration[] = [ + { + featureKey: 'my-feature', + audienceExceptions: [], + defaultValue: 'service-default-value', + }, + ] + + let count = 0 + const server = setupServer( + http.get('https://client.featureboard.app/all', () => { + count++ + if (count <= 2) { + return new Response(null, { + status: 429, + headers: { 'Retry-After': '1' }, + }) + } + return HttpResponse.json(values, { + headers: { + etag: new Date().toISOString(), + }, + }) + }), + ) + server.listen({ onUnhandledRequest: 'error' }) + + try { + const httpClient = createServerClient({ + environmentApiKey: 'env-api-key', + api: featureBoardHostedService, + updateStrategy: { kind: 'manual' }, + }) + await httpClient.waitForInitialised() + httpClient.close() + } finally { + server.resetHandlers() + server.close() + } + expect(count).toBe(3) + }) + + it('Block HTTP client API call after 429 response from HTTP Client API according to retry-after seconds', async () => { + const values: FeatureConfiguration[] = [ + { + featureKey: 'my-feature', + audienceExceptions: [], + defaultValue: 'service-default-value', + }, + ] + + let count = 0 + const server = setupServer( + http.get( + 'https://client.featureboard.app/all', + () => { + count++ + return HttpResponse.json(values, { + headers: { + etag: new Date().toISOString(), + }, + }) + }, + { once: true }, + ), + http.get( + 'https://client.featureboard.app/all', + () => { + count++ + return new Response(null, { + status: 429, + headers: { 'Retry-After': '1' }, + }) + }, + { once: true }, + ), + http.get('https://client.featureboard.app/all', () => { + count++ + return HttpResponse.json(values, { + headers: { + etag: new Date().toISOString(), + }, + }) + }), + ) + server.listen({ onUnhandledRequest: 'error' }) + + try { + const httpClient = createServerClient({ + environmentApiKey: 'env-api-key', + api: featureBoardHostedService, + updateStrategy: { kind: 'manual' }, + }) + + await httpClient.waitForInitialised() + await expect(() => + httpClient.updateFeatures(), + ).rejects.toThrowError(TooManyRequestsError) + await expect(() => + httpClient.updateFeatures(), + ).rejects.toThrowError(TooManyRequestsError) + await new Promise((resolve) => setTimeout(resolve, 1000)) + await httpClient.updateFeatures() + + httpClient.close() + } finally { + server.resetHandlers() + server.close() + } + expect(count).toBe(3) + }) + + it('Block HTTP client API call after 429 response from HTTP Client API according to retry-after date', async () => { + const values: FeatureConfiguration[] = [ + { + featureKey: 'my-feature', + audienceExceptions: [], + defaultValue: 'service-default-value', + }, + ] + + let count = 0 + const server = setupServer( + http.get( + 'https://client.featureboard.app/all', + () => { + count++ + return HttpResponse.json(values, { + headers: { + etag: new Date().toISOString(), + }, + }) + }, + { once: true }, + ), + http.get( + 'https://client.featureboard.app/all', + () => { + count++ + const retryAfter = new Date(new Date().getTime() + 1000).toUTCString() + return new Response(null, { + status: 429, + headers: { 'Retry-After': retryAfter }, + }) + }, + { once: true }, + ), + http.get('https://client.featureboard.app/all', () => { + count++ + return HttpResponse.json(values, { + headers: { + etag: new Date().toISOString(), + }, + }) + }), + ) + server.listen({ onUnhandledRequest: 'error' }) + + try { + const httpClient = createServerClient({ + environmentApiKey: 'env-api-key', + api: featureBoardHostedService, + updateStrategy: { kind: 'manual' }, + }) + + await httpClient.waitForInitialised() + await expect(() => + httpClient.updateFeatures(), + ).rejects.toThrowError(TooManyRequestsError) + await expect(() => + httpClient.updateFeatures(), + ).rejects.toThrowError(TooManyRequestsError) + await new Promise((resolve) => setTimeout(resolve, 1000)) + await httpClient.updateFeatures() + + httpClient.close() + } finally { + server.resetHandlers() + server.close() + } + expect(count).toBe(3) + }) + // Below tests are testing behavior around https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-None-Match it('Attaches etag header to update requests', async () => { const values: FeatureConfiguration[] = [ diff --git a/libs/node-sdk/src/tests/mode-on-request.spec.ts b/libs/node-sdk/src/tests/mode-on-request.spec.ts index a3b11e5c..68329c20 100644 --- a/libs/node-sdk/src/tests/mode-on-request.spec.ts +++ b/libs/node-sdk/src/tests/mode-on-request.spec.ts @@ -39,6 +39,7 @@ describe('On request update mode', () => { server.resetHandlers() server.close() } + expect.assertions(1) }) it('throws if request() is not awaited in request mode', async () => { @@ -77,6 +78,7 @@ describe('On request update mode', () => { server.resetHandlers() server.close() } + expect.assertions(1) }) // To reduce load on the FeatureBoard server, we only fetch the values once they are considered old @@ -124,6 +126,7 @@ describe('On request update mode', () => { server.resetHandlers() server.close() } + expect.assertions(1) }) it('fetches update when response is expired', async () => { @@ -153,20 +156,85 @@ describe('On request update mode', () => { }), ) server.listen({ onUnhandledRequest: 'error' }) + try { + const connection = createServerClient({ + environmentApiKey: 'fake-key', + updateStrategy: { + kind: 'on-request', + options: { maxAgeMs: 1 }, + }, + }) + await connection.waitForInitialised() + + // Ensure response has expired + await new Promise((resolve) => setTimeout(resolve, 10)) - const connection = createServerClient({ - environmentApiKey: 'fake-key', - updateStrategy: { kind: 'on-request', options: { maxAgeMs: 1 } }, - }) - await connection.waitForInitialised() + const client = await connection.request([]) + expect( + client.getFeatureValue('my-feature', 'default-value'), + ).toEqual('new-service-default-value') + } finally { + server.resetHandlers() + server.close() + } + expect.assertions(1) + }) - // Ensure response has expired - await new Promise((resolve) => setTimeout(resolve, 10)) + it('Do NOT throw error or make call to HTTP Client API when Too Many Requests (429) has been returned', async () => { + const values: FeatureConfiguration[] = [ + { + featureKey: 'my-feature', + audienceExceptions: [], + defaultValue: 'service-default-value', + }, + ] - const client = await connection.request([]) - expect(client.getFeatureValue('my-feature', 'default-value')).toEqual( - 'new-service-default-value', + let countAPICalls = 0 + const server = setupServer( + http.get( + 'https://client.featureboard.app/all', + () => { + countAPICalls++ + return HttpResponse.json(values) + }, + { once: true }, + ), + http.get('https://client.featureboard.app/all', () => { + countAPICalls++ + return new Response(null, { + status: 429, + headers: { 'Retry-After': '2' }, + }) + }), ) + server.listen({ onUnhandledRequest: 'error' }) + + try { + const client = createServerClient({ + environmentApiKey: 'fake-key', + updateStrategy: { + kind: 'on-request', + options: { maxAgeMs: 100 }, + }, + }) + await client.waitForInitialised() + // Wait for the on-request max age to expire + await new Promise((resolve) => setTimeout(resolve, 100)) + await client.request([]) + // Wait for the on-request max age to expire + await new Promise((resolve) => setTimeout(resolve, 100)) + const requestClient = await client.request([]) + const value = requestClient.getFeatureValue( + 'my-feature', + 'default-value', + ) + expect(value).toEqual('service-default-value') + } finally { + server.resetHandlers() + server.close() + } + expect(countAPICalls).toBe(2) + expect.assertions(2) }) }) diff --git a/libs/node-sdk/src/tests/mode-polling.spec.ts b/libs/node-sdk/src/tests/mode-polling.spec.ts index 36b193f2..48f68c31 100644 --- a/libs/node-sdk/src/tests/mode-polling.spec.ts +++ b/libs/node-sdk/src/tests/mode-polling.spec.ts @@ -44,6 +44,7 @@ describe('Polling update mode', () => { server.resetHandlers() server.close() } + expect.assertions(1) }) it('sets up interval correctly', async () => { @@ -82,6 +83,7 @@ describe('Polling update mode', () => { server.resetHandlers() server.close() } + expect.assertions(2) }) it('fetches updates when interval fires', async () => { @@ -118,19 +120,87 @@ describe('Polling update mode', () => { ) server.listen({ onUnhandledRequest: 'error' }) - const client = createServerClient({ - environmentApiKey: 'fake-key', - updateStrategy: 'polling', - }) - await client.waitForInitialised() + try { + const client = createServerClient({ + environmentApiKey: 'fake-key', + updateStrategy: 'polling', + }) + await client.waitForInitialised() + + const pollCallback = (setMock.mock.calls[0] as any)[0] + await pollCallback() + + const value = client + .request([]) + .getFeatureValue('my-feature', 'default-value') + expect(value).toEqual('new-service-default-value') + } finally { + server.resetHandlers() + server.close() + } + expect.assertions(1) + }) + + it('Do NOT throw error or make call to HTTP Client API when Too Many Requests (429) has been returned', async () => { + const values: FeatureConfiguration[] = [ + { + featureKey: 'my-feature', + audienceExceptions: [], + defaultValue: 'service-default-value', + }, + ] - const pollCallback = (setMock.mock.calls[0] as any)[0] - await pollCallback() + let countAPICalls = 0 + const server = setupServer( + http.get( + 'https://client.featureboard.app/all', + () => { + countAPICalls++ + return HttpResponse.json(values) + }, + { once: true }, + ), + http.get('https://client.featureboard.app/all', () => { + countAPICalls++ + return new Response(null, { + status: 429, + headers: { 'Retry-After': '2' }, + }) + }), + ) + server.listen({ onUnhandledRequest: 'error' }) - const value = client - .request([]) - .getFeatureValue('my-feature', 'default-value') - expect(value).toEqual('new-service-default-value') + try { + const client = createServerClient({ + environmentApiKey: 'fake-key', + updateStrategy: { + kind: 'polling', + options: { intervalMs: 100 }, + }, + }) + await client.waitForInitialised() + // Wait for the interval to expire + await new Promise((resolve) => setTimeout(resolve, 100)) + const requestClient1 = await client.request([]) + const value1 = requestClient1.getFeatureValue( + 'my-feature', + 'default-value', + ) + expect(value1).toEqual('service-default-value') + // Wait for interval to expire + await new Promise((resolve) => setTimeout(resolve, 100)) + const requestClient2 = await client.request([]) + const value2 = requestClient2.getFeatureValue( + 'my-feature', + 'default-value', + ) + expect(value2).toEqual('service-default-value') + } finally { + server.resetHandlers() + server.close() + } + expect(countAPICalls).toBe(2) + expect.assertions(3) }) }) diff --git a/libs/node-sdk/src/update-strategies/createManualUpdateStrategy.ts b/libs/node-sdk/src/update-strategies/createManualUpdateStrategy.ts index 5f8396aa..005a4938 100644 --- a/libs/node-sdk/src/update-strategies/createManualUpdateStrategy.ts +++ b/libs/node-sdk/src/update-strategies/createManualUpdateStrategy.ts @@ -1,4 +1,4 @@ -import { createEnsureSingle } from '@featureboard/js-sdk' +import { createEnsureSingleWithBackoff } from '@featureboard/js-sdk' import { fetchFeaturesConfigurationViaHttp } from '../utils/fetchFeaturesConfiguration' import { getAllEndpoint } from './getAllEndpoint' import type { AllConfigUpdateStrategy } from './update-strategies' @@ -12,8 +12,7 @@ export function createManualUpdateStrategy( return { async connect(stateStore) { - // Ensure that we don't trigger another request while one is in flight - fetchUpdatesSingle = createEnsureSingle(async () => { + fetchUpdatesSingle = createEnsureSingleWithBackoff(async () => { const allEndpoint = getAllEndpoint(httpEndpoint) etag = await fetchFeaturesConfigurationViaHttp( allEndpoint, diff --git a/libs/node-sdk/src/update-strategies/createOnRequestUpdateStrategy.ts b/libs/node-sdk/src/update-strategies/createOnRequestUpdateStrategy.ts index 875bb72e..2d0aa958 100644 --- a/libs/node-sdk/src/update-strategies/createOnRequestUpdateStrategy.ts +++ b/libs/node-sdk/src/update-strategies/createOnRequestUpdateStrategy.ts @@ -1,4 +1,4 @@ -import { createEnsureSingle } from '@featureboard/js-sdk' +import { createEnsureSingleWithBackoff } from '@featureboard/js-sdk' import { fetchFeaturesConfigurationViaHttp } from '../utils/fetchFeaturesConfiguration' import { getAllEndpoint } from './getAllEndpoint' import type { AllConfigUpdateStrategy } from './update-strategies' @@ -16,7 +16,7 @@ export function createOnRequestUpdateStrategy( return { async connect(stateStore) { // Ensure that we don't trigger another request while one is in flight - fetchUpdatesSingle = createEnsureSingle(async () => { + fetchUpdatesSingle = createEnsureSingleWithBackoff(async () => { const allEndpoint = getAllEndpoint(httpEndpoint) etag = await fetchFeaturesConfigurationViaHttp( allEndpoint, @@ -27,9 +27,8 @@ export function createOnRequestUpdateStrategy( ) }) - return fetchUpdatesSingle().then((response) => { + return fetchUpdatesSingle().then(() => { responseExpires = Date.now() + maxAgeMs - return response }) }, close() { @@ -47,12 +46,20 @@ export function createOnRequestUpdateStrategy( if (fetchUpdatesSingle) { const now = Date.now() if (!responseExpires || now >= responseExpires) { - responseExpires = now + maxAgeMs - updatesLog('Response expired, fetching updates: %o', { - maxAgeMs, - newExpiry: responseExpires, - }) return fetchUpdatesSingle() + .then(() => { + responseExpires = now + maxAgeMs + updatesLog( + 'Response expired, fetching updates: %o', + { + maxAgeMs, + newExpiry: responseExpires, + }, + ) + }) + .catch((error: Error) => { + updatesLog(error) + }) } updatesLog('Response not expired: %o', { diff --git a/libs/node-sdk/src/update-strategies/createPollingUpdateStrategy.ts b/libs/node-sdk/src/update-strategies/createPollingUpdateStrategy.ts index 6a5244c4..2aabe854 100644 --- a/libs/node-sdk/src/update-strategies/createPollingUpdateStrategy.ts +++ b/libs/node-sdk/src/update-strategies/createPollingUpdateStrategy.ts @@ -1,8 +1,9 @@ -import { createEnsureSingle } from '@featureboard/js-sdk' +import { createEnsureSingleWithBackoff } from '@featureboard/js-sdk' import { fetchFeaturesConfigurationViaHttp } from '../utils/fetchFeaturesConfiguration' import { pollingUpdates } from '../utils/pollingUpdates' import { getAllEndpoint } from './getAllEndpoint' import type { AllConfigUpdateStrategy } from './update-strategies' +import { updatesLog } from './updates-log' export function createPollingUpdateStrategy( environmentApiKey: string, @@ -16,7 +17,7 @@ export function createPollingUpdateStrategy( return { async connect(stateStore) { // Ensure that we don't trigger another request while one is in flight - fetchUpdatesSingle = createEnsureSingle(async () => { + fetchUpdatesSingle = createEnsureSingleWithBackoff(async () => { const allEndpoint = getAllEndpoint(httpEndpoint) etag = await fetchFeaturesConfigurationViaHttp( allEndpoint, @@ -33,7 +34,9 @@ export function createPollingUpdateStrategy( stopPolling = pollingUpdates(() => { if (fetchUpdatesSingle) { // Catch errors here to ensure no unhandled promise rejections after a poll - return fetchUpdatesSingle().catch(() => {}) + return fetchUpdatesSingle().catch((error: Error) => { + updatesLog(error) + }) } }, intervalMs) diff --git a/libs/node-sdk/src/utils/fetchFeaturesConfiguration.ts b/libs/node-sdk/src/utils/fetchFeaturesConfiguration.ts index 9bda4164..517755e0 100644 --- a/libs/node-sdk/src/utils/fetchFeaturesConfiguration.ts +++ b/libs/node-sdk/src/utils/fetchFeaturesConfiguration.ts @@ -1,4 +1,7 @@ -import type { FeatureConfiguration } from '@featureboard/contracts' +import { + TooManyRequestsError, + type FeatureConfiguration, +} from '@featureboard/contracts' import type { AllFeatureStateStore } from '../feature-state-store' import { httpClientDebug } from './http-log' @@ -8,12 +11,13 @@ export async function fetchFeaturesConfigurationViaHttp( stateStore: AllFeatureStateStore, etag: string | undefined, updateTrigger: string, -) { +): Promise { httpClientDebug( 'Fetching updates: trigger=%s, lastModified=%s', updateTrigger, etag, ) + const response = await fetch(allEndpoint, { method: 'GET', headers: { @@ -22,6 +26,34 @@ export async function fetchFeaturesConfigurationViaHttp( }, }) + if (response.status === 429) { + // Too many requests + const retryAfterHeader = response.headers.get('Retry-After') + const retryAfterInt = retryAfterHeader + ? parseInt(retryAfterHeader, 10) + : 60 + const retryAfter = + retryAfterHeader && !retryAfterInt + ? new Date(retryAfterHeader) + : new Date() + + if (retryAfterInt) { + const retryAfterTime = retryAfter.getTime() + retryAfterInt * 1000 + retryAfter.setTime(retryAfterTime) + } + + throw new TooManyRequestsError( + `Failed to get latest features: Service returned ${ + response.status + }${response.statusText ? ' ' + response.statusText : ''}. ${ + retryAfterHeader + ? 'Retry after: ' + retryAfter.toUTCString() + : '' + } `, + retryAfter, + ) + } + if (response.status !== 200 && response.status !== 304) { throw new Error( `Failed to get latest flags: Service returned error ${response.status} (${response.statusText})`,