feat: add evaluation details to finally hook (#1087)

## This PR

- adds evaluation details to the `finally` stage in hooks.

### Notes

This breaks the signature of the `finally` stages based on [this spec
enhancement](https://github.com/open-feature/spec/pull/280). It is
**not** considered a breaking change to the SDK because hooks are marked
as experimental in the spec, and the change has no impact on known
hooks.

The noteworthy change to the interface is:

```diff
- finally?(hookContext: Readonly<HookContext<T>>, hookHints?: HookHints): HooksReturn;
+ finally?(hookContext: Readonly<HookContext<T>>, evaluationDetails: EvaluationDetails<T>, hookHints?: HookHints): HooksReturn;
```

### Follow-up Tasks

- Update the JS contribs repo

---------

Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
This commit is contained in:
Michael Beemer 2024-12-12 08:49:09 -05:00 committed by GitHub
parent 5e5b160221
commit 2135254c4b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 106 additions and 51 deletions

View File

@ -10,7 +10,9 @@ import type {
Logger, Logger,
TrackingEventDetails, TrackingEventDetails,
OpenFeatureError, OpenFeatureError,
ResolutionDetails} from '@openfeature/core'; FlagMetadata,
ResolutionDetails,
} from '@openfeature/core';
import { import {
ErrorCode, ErrorCode,
ProviderFatalError, ProviderFatalError,
@ -24,7 +26,7 @@ import type { FlagEvaluationOptions } from '../../evaluation';
import type { ProviderEvents } from '../../events'; import type { ProviderEvents } from '../../events';
import type { InternalEventEmitter } from '../../events/internal/internal-event-emitter'; import type { InternalEventEmitter } from '../../events/internal/internal-event-emitter';
import type { Hook } from '../../hooks'; import type { Hook } from '../../hooks';
import type { Provider} from '../../provider'; import type { Provider } from '../../provider';
import { ProviderStatus } from '../../provider'; import { ProviderStatus } from '../../provider';
import type { Client } from './../client'; import type { Client } from './../client';
@ -279,6 +281,8 @@ export class OpenFeatureClient implements Client {
logger: this._logger, logger: this._logger,
}; };
let evaluationDetails: EvaluationDetails<T>;
try { try {
const frozenContext = await this.beforeHooks(allHooks, hookContext, options); const frozenContext = await this.beforeHooks(allHooks, hookContext, options);
@ -287,27 +291,27 @@ export class OpenFeatureClient implements Client {
// run the referenced resolver, binding the provider. // run the referenced resolver, binding the provider.
const resolution = await resolver.call(this._provider, flagKey, defaultValue, frozenContext, this._logger); const resolution = await resolver.call(this._provider, flagKey, defaultValue, frozenContext, this._logger);
const evaluationDetails = { const resolutionDetails = {
...resolution, ...resolution,
flagMetadata: Object.freeze(resolution.flagMetadata ?? {}), flagMetadata: Object.freeze(resolution.flagMetadata ?? {}),
flagKey, flagKey,
}; };
if (evaluationDetails.errorCode) { if (resolutionDetails.errorCode) {
const err = instantiateErrorByErrorCode(evaluationDetails.errorCode); const err = instantiateErrorByErrorCode(resolutionDetails.errorCode);
await this.errorHooks(allHooksReversed, hookContext, err, options); await this.errorHooks(allHooksReversed, hookContext, err, options);
return this.getErrorEvaluationDetails(flagKey, defaultValue, err); evaluationDetails = this.getErrorEvaluationDetails(flagKey, defaultValue, err, resolutionDetails.flagMetadata);
} else {
await this.afterHooks(allHooksReversed, hookContext, resolutionDetails, options);
evaluationDetails = resolutionDetails;
} }
await this.afterHooks(allHooksReversed, hookContext, evaluationDetails, options);
return evaluationDetails;
} catch (err: unknown) { } catch (err: unknown) {
await this.errorHooks(allHooksReversed, hookContext, err, options); await this.errorHooks(allHooksReversed, hookContext, err, options);
return this.getErrorEvaluationDetails(flagKey, defaultValue, err); evaluationDetails = this.getErrorEvaluationDetails(flagKey, defaultValue, err);
} finally {
await this.finallyHooks(allHooksReversed, hookContext, options);
} }
await this.finallyHooks(allHooksReversed, hookContext, evaluationDetails, options);
return evaluationDetails;
} }
private async beforeHooks(hooks: Hook[], hookContext: HookContext, options: FlagEvaluationOptions) { private async beforeHooks(hooks: Hook[], hookContext: HookContext, options: FlagEvaluationOptions) {
@ -353,11 +357,16 @@ export class OpenFeatureClient implements Client {
} }
} }
private async finallyHooks(hooks: Hook[], hookContext: HookContext, options: FlagEvaluationOptions) { private async finallyHooks(
hooks: Hook[],
hookContext: HookContext,
evaluationDetails: EvaluationDetails<FlagValue>,
options: FlagEvaluationOptions,
) {
// run "finally" hooks sequentially // run "finally" hooks sequentially
for (const hook of hooks) { for (const hook of hooks) {
try { try {
await hook?.finally?.(hookContext, options.hookHints); await hook?.finally?.(hookContext, evaluationDetails, options.hookHints);
} catch (err) { } catch (err) {
this._logger.error(`Unhandled error during 'finally' hook: ${err}`); this._logger.error(`Unhandled error during 'finally' hook: ${err}`);
if (err instanceof Error) { if (err instanceof Error) {
@ -403,6 +412,7 @@ export class OpenFeatureClient implements Client {
flagKey: string, flagKey: string,
defaultValue: T, defaultValue: T,
err: unknown, err: unknown,
flagMetadata: FlagMetadata = {},
): EvaluationDetails<T> { ): EvaluationDetails<T> {
const errorMessage: string = (err as Error)?.message; const errorMessage: string = (err as Error)?.message;
const errorCode: ErrorCode = (err as OpenFeatureError)?.code || ErrorCode.GENERAL; const errorCode: ErrorCode = (err as OpenFeatureError)?.code || ErrorCode.GENERAL;
@ -412,7 +422,7 @@ export class OpenFeatureClient implements Client {
errorMessage, errorMessage,
value: defaultValue, value: defaultValue,
reason: StandardResolutionReasons.ERROR, reason: StandardResolutionReasons.ERROR,
flagMetadata: Object.freeze({}), flagMetadata: Object.freeze(flagMetadata),
flagKey, flagKey,
}; };
} }

View File

@ -439,6 +439,30 @@ describe('Hooks', () => {
}); });
}); });
}); });
describe('Requirement 4.3.8', () => {
it('"evaluation details" passed to the "finally" stage matches the evaluation details returned to the application author', async () => {
OpenFeature.setProvider(MOCK_PROVIDER);
let evaluationDetailsHooks;
const evaluationDetails = await client.getBooleanDetails(
FLAG_KEY,
false,
{},
{
hooks: [
{
finally: (_, details) => {
evaluationDetailsHooks = details;
},
},
],
},
);
expect(evaluationDetailsHooks).toEqual(evaluationDetails);
});
});
}); });
describe('Requirement 4.4.2', () => { describe('Requirement 4.4.2', () => {
@ -922,14 +946,14 @@ describe('Hooks', () => {
done(err); done(err);
} }
}, },
after: (_hookContext, _evaluationDetils, hookHints) => { after: (_hookContext, _evaluationDetails, hookHints) => {
try { try {
expect(hookHints?.hint).toBeTruthy(); expect(hookHints?.hint).toBeTruthy();
} catch (err) { } catch (err) {
done(err); done(err);
} }
}, },
finally: (_, hookHints) => { finally: (_, _evaluationDetails, hookHints) => {
try { try {
expect(hookHints?.hint).toBeTruthy(); expect(hookHints?.hint).toBeTruthy();
done(); done();

View File

@ -32,9 +32,13 @@ export interface BaseHook<T extends FlagValue = FlagValue, BeforeHookReturn = un
/** /**
* Runs after all other hook stages, regardless of success or error. * Runs after all other hook stages, regardless of success or error.
* Errors thrown here are unhandled by the client and will surface in application code.
* @param hookContext * @param hookContext
* @param evaluationDetails
* @param hookHints * @param hookHints
*/ */
finally?(hookContext: Readonly<HookContext<T>>, hookHints?: HookHints): HooksReturn; finally?(
hookContext: Readonly<HookContext<T>>,
evaluationDetails: EvaluationDetails<T>,
hookHints?: HookHints,
): HooksReturn;
} }

View File

@ -10,7 +10,9 @@ import type {
Logger, Logger,
TrackingEventDetails, TrackingEventDetails,
OpenFeatureError, OpenFeatureError,
ResolutionDetails } from '@openfeature/core'; FlagMetadata,
ResolutionDetails,
} from '@openfeature/core';
import { import {
ErrorCode, ErrorCode,
ProviderFatalError, ProviderFatalError,
@ -24,7 +26,7 @@ import type { FlagEvaluationOptions } from '../../evaluation';
import type { ProviderEvents } from '../../events'; import type { ProviderEvents } from '../../events';
import type { InternalEventEmitter } from '../../events/internal/internal-event-emitter'; import type { InternalEventEmitter } from '../../events/internal/internal-event-emitter';
import type { Hook } from '../../hooks'; import type { Hook } from '../../hooks';
import type { Provider} from '../../provider'; import type { Provider } from '../../provider';
import { ProviderStatus } from '../../provider'; import { ProviderStatus } from '../../provider';
import type { Client } from './../client'; import type { Client } from './../client';
@ -234,6 +236,8 @@ export class OpenFeatureClient implements Client {
logger: this._logger, logger: this._logger,
}; };
let evaluationDetails: EvaluationDetails<T>;
try { try {
this.beforeHooks(allHooks, hookContext, options); this.beforeHooks(allHooks, hookContext, options);
@ -242,27 +246,26 @@ export class OpenFeatureClient implements Client {
// run the referenced resolver, binding the provider. // run the referenced resolver, binding the provider.
const resolution = resolver.call(this._provider, flagKey, defaultValue, context, this._logger); const resolution = resolver.call(this._provider, flagKey, defaultValue, context, this._logger);
const evaluationDetails = { const resolutionDetails = {
...resolution, ...resolution,
flagMetadata: Object.freeze(resolution.flagMetadata ?? {}), flagMetadata: Object.freeze(resolution.flagMetadata ?? {}),
flagKey, flagKey,
}; };
if (evaluationDetails.errorCode) { if (resolutionDetails.errorCode) {
const err = instantiateErrorByErrorCode(evaluationDetails.errorCode); const err = instantiateErrorByErrorCode(resolutionDetails.errorCode);
this.errorHooks(allHooksReversed, hookContext, err, options); this.errorHooks(allHooksReversed, hookContext, err, options);
return this.getErrorEvaluationDetails(flagKey, defaultValue, err); evaluationDetails = this.getErrorEvaluationDetails(flagKey, defaultValue, err, resolutionDetails.flagMetadata);
} else {
this.afterHooks(allHooksReversed, hookContext, resolutionDetails, options);
evaluationDetails = resolutionDetails;
} }
this.afterHooks(allHooksReversed, hookContext, evaluationDetails, options);
return evaluationDetails;
} catch (err: unknown) { } catch (err: unknown) {
this.errorHooks(allHooksReversed, hookContext, err, options); this.errorHooks(allHooksReversed, hookContext, err, options);
return this.getErrorEvaluationDetails(flagKey, defaultValue, err); evaluationDetails = this.getErrorEvaluationDetails(flagKey, defaultValue, err);
} finally {
this.finallyHooks(allHooksReversed, hookContext, options);
} }
this.finallyHooks(allHooksReversed, hookContext, evaluationDetails, options);
return evaluationDetails;
} }
private beforeHooks(hooks: Hook[], hookContext: HookContext, options: FlagEvaluationOptions) { private beforeHooks(hooks: Hook[], hookContext: HookContext, options: FlagEvaluationOptions) {
@ -301,11 +304,16 @@ export class OpenFeatureClient implements Client {
} }
} }
private finallyHooks(hooks: Hook[], hookContext: HookContext, options: FlagEvaluationOptions) { private finallyHooks(
hooks: Hook[],
hookContext: HookContext,
evaluationDetails: EvaluationDetails<FlagValue>,
options: FlagEvaluationOptions,
) {
// run "finally" hooks sequentially // run "finally" hooks sequentially
for (const hook of hooks) { for (const hook of hooks) {
try { try {
hook?.finally?.(hookContext, options.hookHints); hook?.finally?.(hookContext, evaluationDetails, options.hookHints);
} catch (err) { } catch (err) {
this._logger.error(`Unhandled error during 'finally' hook: ${err}`); this._logger.error(`Unhandled error during 'finally' hook: ${err}`);
if (err instanceof Error) { if (err instanceof Error) {
@ -337,6 +345,7 @@ export class OpenFeatureClient implements Client {
flagKey: string, flagKey: string,
defaultValue: T, defaultValue: T,
err: unknown, err: unknown,
flagMetadata: FlagMetadata = {},
): EvaluationDetails<T> { ): EvaluationDetails<T> {
const errorMessage: string = (err as Error)?.message; const errorMessage: string = (err as Error)?.message;
const errorCode: ErrorCode = (err as OpenFeatureError)?.code || ErrorCode.GENERAL; const errorCode: ErrorCode = (err as OpenFeatureError)?.code || ErrorCode.GENERAL;
@ -346,7 +355,7 @@ export class OpenFeatureClient implements Client {
errorMessage, errorMessage,
value: defaultValue, value: defaultValue,
reason: StandardResolutionReasons.ERROR, reason: StandardResolutionReasons.ERROR,
flagMetadata: Object.freeze({}), flagMetadata: Object.freeze(flagMetadata),
flagKey, flagKey,
}; };
} }

View File

@ -1,16 +1,5 @@
import type { import type { Provider, ResolutionDetails, Client, FlagValueType, EvaluationContext, Hook } from '../src';
Provider, import { GeneralError, OpenFeature, StandardResolutionReasons, ErrorCode } from '../src';
ResolutionDetails,
Client,
FlagValueType,
EvaluationContext,
Hook} from '../src';
import {
GeneralError,
OpenFeature,
StandardResolutionReasons,
ErrorCode,
} from '../src';
const BOOLEAN_VALUE = true; const BOOLEAN_VALUE = true;
@ -282,6 +271,25 @@ describe('Hooks', () => {
}); });
}); });
}); });
describe('Requirement 4.3.8', () => {
it('"evaluation details" passed to the "finally" stage matches the evaluation details returned to the application author', () => {
OpenFeature.setProvider(MOCK_PROVIDER);
let evaluationDetailsHooks;
const evaluationDetails = client.getBooleanDetails(FLAG_KEY, false, {
hooks: [
{
finally: (_, details) => {
evaluationDetailsHooks = details;
},
},
],
});
expect(evaluationDetailsHooks).toEqual(evaluationDetails);
});
});
}); });
describe('Requirement 4.4.2', () => { describe('Requirement 4.4.2', () => {
@ -759,14 +767,14 @@ describe('Hooks', () => {
done(err); done(err);
} }
}, },
after: (_hookContext, _evaluationDetils, hookHints) => { after: (_hookContext, _evaluationDetails, hookHints) => {
try { try {
expect(hookHints?.hint).toBeTruthy(); expect(hookHints?.hint).toBeTruthy();
} catch (err) { } catch (err) {
done(err); done(err);
} }
}, },
finally: (_, hookHints) => { finally: (_, _evaluationDetails, hookHints) => {
try { try {
expect(hookHints?.hint).toBeTruthy(); expect(hookHints?.hint).toBeTruthy();
done(); done();