From c6d0b5da9c7f4c11319422fbe8c668a7613b044d Mon Sep 17 00:00:00 2001 From: Michael Beemer Date: Wed, 8 May 2024 13:38:04 -0400 Subject: [PATCH] fix: run error hook when provider returns reason error or error code (#926) ## This PR - runs error hook when provider returns reason error or error code ### Related Issues Fixes #925 ### Notes Based on a conversation in Slack: https://cloud-native.slack.com/archives/C06E4DE6S07/p1714581197391509 --------- Signed-off-by: Michael Beemer --- jest.config.ts | 4 +- .../client/src/client/open-feature-client.ts | 9 +++- packages/client/test/hooks.spec.ts | 23 ++++++++ .../server/src/client/open-feature-client.ts | 5 ++ packages/server/test/hooks.spec.ts | 47 ++++++++++++---- packages/shared/src/errors/index.ts | 54 +++++++++++++++---- 6 files changed, 120 insertions(+), 22 deletions(-) diff --git a/jest.config.ts b/jest.config.ts index c4b08a5c..b048809e 100644 --- a/jest.config.ts +++ b/jest.config.ts @@ -175,13 +175,13 @@ export default { displayName: 'react', testEnvironment: 'jsdom', preset: 'ts-jest', - testMatch: ['/packages/react/test/**/*.spec.ts*'], + testMatch: ['/packages/react/test/**/*.spec.{ts,tsx}'], moduleNameMapper: { '@openfeature/core': '/packages/shared/src', '@openfeature/web-sdk': '/packages/client/src', }, transform: { - '^.+\\.tsx$': [ + '^.+\\.(ts|tsx)$': [ 'ts-jest', { tsconfig: '/packages/react/test/tsconfig.json', diff --git a/packages/client/src/client/open-feature-client.ts b/packages/client/src/client/open-feature-client.ts index 500d3d61..9fb23383 100644 --- a/packages/client/src/client/open-feature-client.ts +++ b/packages/client/src/client/open-feature-client.ts @@ -15,7 +15,8 @@ import { ResolutionDetails, SafeLogger, StandardResolutionReasons, - statusMatchesEvent + instantiateErrorByErrorCode, + statusMatchesEvent, } from '@openfeature/core'; import { FlagEvaluationOptions } from '../evaluation'; import { ProviderEvents } from '../events'; @@ -208,7 +209,7 @@ export class OpenFeatureClient implements Client { try { this.beforeHooks(allHooks, hookContext, options); - + // short circuit evaluation entirely if provider is in a bad state if (this.providerStatus === ProviderStatus.NOT_READY) { throw new ProviderNotReadyError('provider has not yet initialized'); @@ -225,6 +226,10 @@ export class OpenFeatureClient implements Client { flagKey, }; + if (evaluationDetails.errorCode) { + throw instantiateErrorByErrorCode(evaluationDetails.errorCode); + } + this.afterHooks(allHooksReversed, hookContext, evaluationDetails, options); return evaluationDetails; diff --git a/packages/client/test/hooks.spec.ts b/packages/client/test/hooks.spec.ts index 902eba1c..d52884a7 100644 --- a/packages/client/test/hooks.spec.ts +++ b/packages/client/test/hooks.spec.ts @@ -7,6 +7,8 @@ import { GeneralError, OpenFeature, Hook, + StandardResolutionReasons, + ErrorCode, } from '../src'; const BOOLEAN_VALUE = true; @@ -206,6 +208,27 @@ describe('Hooks', () => { ], }); }); + + it('"error" must run if resolution details contains an error code', () => { + (MOCK_ERROR_PROVIDER.resolveBooleanEvaluation as jest.Mock).mockReturnValue({ + value: BOOLEAN_VALUE, + errorCode: ErrorCode.FLAG_NOT_FOUND, + }); + + const mockErrorHook = jest.fn(); + + const details = client.getBooleanDetails(FLAG_KEY, false, { + hooks: [{ error: mockErrorHook }], + }); + + expect(mockErrorHook).toHaveBeenCalled(); + expect(details).toEqual( + expect.objectContaining({ + errorCode: ErrorCode.FLAG_NOT_FOUND, + reason: StandardResolutionReasons.ERROR, + }), + ); + }); }); }); diff --git a/packages/server/src/client/open-feature-client.ts b/packages/server/src/client/open-feature-client.ts index d697c167..fb37b73f 100644 --- a/packages/server/src/client/open-feature-client.ts +++ b/packages/server/src/client/open-feature-client.ts @@ -16,6 +16,7 @@ import { ResolutionDetails, SafeLogger, StandardResolutionReasons, + instantiateErrorByErrorCode, statusMatchesEvent, } from '@openfeature/core'; import { FlagEvaluationOptions } from '../evaluation'; @@ -278,6 +279,10 @@ export class OpenFeatureClient implements Client, ManageContext> => { return Promise.reject({ - reason: ERROR_REASON, - errorCode: ERROR_CODE, + reason: StandardResolutionReasons.ERROR, + errorCode: ErrorCode.GENERAL, }); }), } as unknown as Provider; @@ -357,6 +365,27 @@ describe('Hooks', () => { ], }); }); + + it('"error" must run if resolution details contains an error code', async () => { + (MOCK_ERROR_PROVIDER.resolveBooleanEvaluation as jest.Mock).mockResolvedValueOnce({ + value: BOOLEAN_VALUE, + errorCode: ErrorCode.FLAG_NOT_FOUND, + }); + + const mockErrorHook = jest.fn(); + + const details = await client.getBooleanDetails(FLAG_KEY, false, undefined, { + hooks: [{ error: mockErrorHook }], + }); + + expect(mockErrorHook).toHaveBeenCalled(); + expect(details).toEqual( + expect.objectContaining({ + errorCode: ErrorCode.FLAG_NOT_FOUND, + reason: StandardResolutionReasons.ERROR, + }), + ); + }); }); }); @@ -636,8 +665,8 @@ describe('Hooks', () => { ], resolveBooleanEvaluation: jest.fn((): Promise> => { return Promise.reject({ - reason: ERROR_REASON, - errorCode: ERROR_CODE, + reason: StandardResolutionReasons.ERROR, + errorCode: ErrorCode.INVALID_CONTEXT, }); }), } as unknown as Provider; @@ -717,8 +746,8 @@ describe('Hooks', () => { ], resolveBooleanEvaluation: jest.fn((): Promise> => { return Promise.reject({ - reason: ERROR_REASON, - errorCode: ERROR_CODE, + reason: StandardResolutionReasons.ERROR, + errorCode: ErrorCode.PROVIDER_NOT_READY, }); }), } as unknown as Provider; diff --git a/packages/shared/src/errors/index.ts b/packages/shared/src/errors/index.ts index 6cde15e2..bd87e965 100644 --- a/packages/shared/src/errors/index.ts +++ b/packages/shared/src/errors/index.ts @@ -1,9 +1,45 @@ -export * from './general-error'; -export * from './flag-not-found-error'; -export * from './parse-error'; -export * from './type-mismatch-error'; -export * from './targeting-key-missing-error'; -export * from './invalid-context-error'; -export * from './open-feature-error-abstract'; -export * from './provider-not-ready-error'; -export * from './provider-fatal-error'; +import { ErrorCode } from '../evaluation'; + +import { FlagNotFoundError } from './flag-not-found-error'; +import { GeneralError } from './general-error'; +import { InvalidContextError } from './invalid-context-error'; +import { OpenFeatureError } from './open-feature-error-abstract'; +import { ParseError } from './parse-error'; +import { ProviderFatalError } from './provider-fatal-error'; +import { ProviderNotReadyError } from './provider-not-ready-error'; +import { TargetingKeyMissingError } from './targeting-key-missing-error'; +import { TypeMismatchError } from './type-mismatch-error'; + +const instantiateErrorByErrorCode = (errorCode: ErrorCode, message?: string): OpenFeatureError => { + switch (errorCode) { + case ErrorCode.FLAG_NOT_FOUND: + return new FlagNotFoundError(message); + case ErrorCode.PARSE_ERROR: + return new ParseError(message); + case ErrorCode.TYPE_MISMATCH: + return new TypeMismatchError(message); + case ErrorCode.TARGETING_KEY_MISSING: + return new TargetingKeyMissingError(message); + case ErrorCode.INVALID_CONTEXT: + return new InvalidContextError(message); + case ErrorCode.PROVIDER_NOT_READY: + return new ProviderNotReadyError(message); + case ErrorCode.PROVIDER_FATAL: + return new ProviderFatalError(message); + default: + return new GeneralError(message); + } +}; + +export { + FlagNotFoundError, + GeneralError, + InvalidContextError, + ParseError, + ProviderFatalError, + ProviderNotReadyError, + TargetingKeyMissingError, + TypeMismatchError, + OpenFeatureError, + instantiateErrorByErrorCode, +};