From acb358f45db4d91078d4bd2e6aad4ea6fd2f35ed Mon Sep 17 00:00:00 2001 From: Andreas Karlsson Date: Fri, 10 Jan 2025 13:23:42 +0100 Subject: [PATCH 1/4] fix: resolve hanging bug WIP --- packages/sdk/src/FlagResolution.ts | 2 +- packages/sdk/src/FlagResolverClient.test.ts | 9 ++++++++- packages/sdk/src/FlagResolverClient.ts | 3 ++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/sdk/src/FlagResolution.ts b/packages/sdk/src/FlagResolution.ts index 25d2ba4b..1e212eba 100644 --- a/packages/sdk/src/FlagResolution.ts +++ b/packages/sdk/src/FlagResolution.ts @@ -114,7 +114,7 @@ export class ReadyFlagResolution implements FlagResolution { ReadyFlagResolution.prototype.state = 'READY'; -class FailedFlagResolution implements FlagResolution { +export class FailedFlagResolution implements FlagResolution { declare state: 'ERROR'; constructor(readonly context: Value.Struct, readonly code: FlagEvaluation.ErrorCode, readonly message: string) {} diff --git a/packages/sdk/src/FlagResolverClient.test.ts b/packages/sdk/src/FlagResolverClient.test.ts index 2ec457ee..fda589c9 100644 --- a/packages/sdk/src/FlagResolverClient.test.ts +++ b/packages/sdk/src/FlagResolverClient.test.ts @@ -10,7 +10,7 @@ import { setMaxListeners } from 'node:events'; import { SdkId } from './generated/confidence/flags/resolver/v1/types'; import { abortableSleep, FetchBuilder } from './fetch-util'; import { ApplyFlagsRequest, ResolveFlagsRequest } from './generated/confidence/flags/resolver/v1/api'; -import { FlagResolution } from './FlagResolution'; +import { FailedFlagResolution, FlagResolution } from './FlagResolution'; import { Telemetry } from './Telemetry'; import { LibraryTraces_Library, LibraryTraces_TraceId, Platform } from './generated/confidence/telemetry/v1/telemetry'; const RESOLVE_ENDPOINT = 'https://resolver.confidence.dev/v1/flags:resolve'; @@ -270,6 +270,12 @@ describe('Backend environment Evaluation', () => { }); }); }); + + it('should resolve a FailedFlagResolution on fetch errors', async () => { + resolveHandlerMock.mockRejectedValue(new Error('Test error')); + const flagResolution = await instanceUnderTest.resolve({}, ['testflag']); + expect(flagResolution).toBeInstanceOf(FailedFlagResolution); + }); }); describe('intercept', () => { @@ -451,6 +457,7 @@ describe('CachingFlagResolverClient', () => { expect(pendingResolution.signal.aborted).toBe(true); }); }); + function nextCall(mock: jest.Mock): Promise { return new Promise(resolve => { const impl = mock.getMockImplementation(); diff --git a/packages/sdk/src/FlagResolverClient.ts b/packages/sdk/src/FlagResolverClient.ts index f4d5d098..a0be7985 100644 --- a/packages/sdk/src/FlagResolverClient.ts +++ b/packages/sdk/src/FlagResolverClient.ts @@ -157,8 +157,9 @@ export class FetchingFlagResolverClient implements FlagResolverClient { .catch(error => { if (error instanceof ResolveError) { return FlagResolution.failed(context, error.code, error.message); + } else { + return FlagResolution.failed(context, 'GENERAL', error.message); } - throw error; }) .finally(() => { this.markLatency(Date.now() - start); From 61d101844caa538370c163b23d571ce02dc10701 Mon Sep 17 00:00:00 2001 From: Nicklas Lundin Date: Fri, 10 Jan 2025 14:21:03 +0100 Subject: [PATCH 2/4] test: add extra test case --- packages/sdk/src/FlagResolverClient.test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/sdk/src/FlagResolverClient.test.ts b/packages/sdk/src/FlagResolverClient.test.ts index fda589c9..fcab669e 100644 --- a/packages/sdk/src/FlagResolverClient.test.ts +++ b/packages/sdk/src/FlagResolverClient.test.ts @@ -275,6 +275,12 @@ describe('Backend environment Evaluation', () => { resolveHandlerMock.mockRejectedValue(new Error('Test error')); const flagResolution = await instanceUnderTest.resolve({}, ['testflag']); expect(flagResolution).toBeInstanceOf(FailedFlagResolution); + expect(flagResolution.evaluate('testflag', {})).toEqual({ + errorCode: 'GENERAL', + errorMessage: '500: Test error', + reason: 'ERROR', + value: {}, + }); }); }); From ca900d70eaac0041617963a5bce034cd97594d83 Mon Sep 17 00:00:00 2001 From: Nicklas Lundin Date: Fri, 10 Jan 2025 14:37:14 +0100 Subject: [PATCH 3/4] fixup! test: add extra test case --- packages/sdk/src/FlagResolverClient.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/sdk/src/FlagResolverClient.ts b/packages/sdk/src/FlagResolverClient.ts index a0be7985..3a2c411f 100644 --- a/packages/sdk/src/FlagResolverClient.ts +++ b/packages/sdk/src/FlagResolverClient.ts @@ -157,9 +157,8 @@ export class FetchingFlagResolverClient implements FlagResolverClient { .catch(error => { if (error instanceof ResolveError) { return FlagResolution.failed(context, error.code, error.message); - } else { - return FlagResolution.failed(context, 'GENERAL', error.message); } + return FlagResolution.failed(context, 'GENERAL', error.message); }) .finally(() => { this.markLatency(Date.now() - start); From ab45361b81b656920d7c04b6dffe9c9d7a5ba768 Mon Sep 17 00:00:00 2001 From: Nicklas Lundin Date: Fri, 10 Jan 2025 16:31:55 +0100 Subject: [PATCH 4/4] test: add test case for client environment --- packages/sdk/src/FlagResolverClient.test.ts | 82 +++++++++++++-------- 1 file changed, 53 insertions(+), 29 deletions(-) diff --git a/packages/sdk/src/FlagResolverClient.test.ts b/packages/sdk/src/FlagResolverClient.test.ts index fcab669e..ac2336e4 100644 --- a/packages/sdk/src/FlagResolverClient.test.ts +++ b/packages/sdk/src/FlagResolverClient.test.ts @@ -45,28 +45,52 @@ const fetchImplementation = async (request: Request): Promise => { } }; -resolveHandlerMock.mockImplementation(createFlagResolutionResponse); +beforeEach(() => { + resolveHandlerMock.mockImplementation(createFlagResolutionResponse); +}); + +afterEach(() => { + resolveHandlerMock.mockClear(); + applyHandlerMock.mockClear(); +}); describe('Client environment Evaluation', () => { // const flagResolutionResponseJson = JSON.stringify(createFlagResolutionResponse()); - const instanceUnderTest = new FetchingFlagResolverClient({ - fetchImplementation, - clientSecret: 'secret', - applyTimeout: 10, - sdk: { - id: SdkId.SDK_ID_JS_CONFIDENCE, - version: 'test', - }, - environment: 'client', - resolveTimeout: 10, - telemetry: new Telemetry({ disabled: true, logger: { warn: jest.fn() }, environment: 'client' }), + let instanceUnderTest: FetchingFlagResolverClient; + + beforeEach(() => { + instanceUnderTest = new FetchingFlagResolverClient({ + fetchImplementation, + clientSecret: 'secret', + applyTimeout: 10, + sdk: { + id: SdkId.SDK_ID_JS_CONFIDENCE, + version: 'test', + }, + environment: 'client', + resolveTimeout: 10, + telemetry: new Telemetry({ disabled: true, logger: { warn: jest.fn() }, environment: 'client' }), + }); + }); + + it('should resolve a FailedFlagResolution on fetch errors', async () => { + resolveHandlerMock.mockRejectedValue(new Error('Test error')); + const flagResolution = await instanceUnderTest.resolve({}, []); + expect(flagResolution).toBeInstanceOf(FailedFlagResolution); + // Expect this error to log as a Resolve timeout in the client environment + // This is due the request logic that's only used in the client environment + expect(flagResolution.evaluate('testflag', {})).toEqual({ + errorCode: 'TIMEOUT', + errorMessage: 'Resolve timeout', + reason: 'ERROR', + value: {}, + }); }); describe('apply', () => { it('should send an apply event', async () => { const flagResolution = await instanceUnderTest.resolve({}, []); flagResolution.evaluate('testflag.bool', false); - const [applyRequest] = await nextMockArgs(applyHandlerMock); expect(applyRequest).toMatchObject({ clientSecret: 'secret', @@ -81,23 +105,23 @@ describe('Client environment Evaluation', () => { ], }); }); - }); - it('should apply when a flag has no segment match', async () => { - const flagResolution = await instanceUnderTest.resolve({}, []); - flagResolution.evaluate('no-seg-flag.enabled', false); - const [applyRequest] = await nextMockArgs(applyHandlerMock); - expect(applyRequest).toMatchObject({ - clientSecret: 'secret', - resolveToken: dummyResolveToken, - sendTime: expect.any(Date), - sdk: { id: 13, version: 'test' }, - flags: [ - { - applyTime: expect.any(Date), - flag: 'flags/no-seg-flag', - }, - ], + it('should apply when a flag has no segment match', async () => { + const flagResolution = await instanceUnderTest.resolve({}, []); + flagResolution.evaluate('no-seg-flag.enabled', false); + const [applyRequest] = await nextMockArgs(applyHandlerMock); + expect(applyRequest).toMatchObject({ + clientSecret: 'secret', + resolveToken: dummyResolveToken, + sendTime: expect.any(Date), + sdk: { id: 13, version: 'test' }, + flags: [ + { + applyTime: expect.any(Date), + flag: 'flags/no-seg-flag', + }, + ], + }); }); }); });