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:
parent
5e5b160221
commit
2135254c4b
|
|
@ -10,7 +10,9 @@ import type {
|
|||
Logger,
|
||||
TrackingEventDetails,
|
||||
OpenFeatureError,
|
||||
ResolutionDetails} from '@openfeature/core';
|
||||
FlagMetadata,
|
||||
ResolutionDetails,
|
||||
} from '@openfeature/core';
|
||||
import {
|
||||
ErrorCode,
|
||||
ProviderFatalError,
|
||||
|
|
@ -279,6 +281,8 @@ export class OpenFeatureClient implements Client {
|
|||
logger: this._logger,
|
||||
};
|
||||
|
||||
let evaluationDetails: EvaluationDetails<T>;
|
||||
|
||||
try {
|
||||
const frozenContext = await this.beforeHooks(allHooks, hookContext, options);
|
||||
|
||||
|
|
@ -287,27 +291,27 @@ export class OpenFeatureClient implements Client {
|
|||
// run the referenced resolver, binding the provider.
|
||||
const resolution = await resolver.call(this._provider, flagKey, defaultValue, frozenContext, this._logger);
|
||||
|
||||
const evaluationDetails = {
|
||||
const resolutionDetails = {
|
||||
...resolution,
|
||||
flagMetadata: Object.freeze(resolution.flagMetadata ?? {}),
|
||||
flagKey,
|
||||
};
|
||||
|
||||
if (evaluationDetails.errorCode) {
|
||||
const err = instantiateErrorByErrorCode(evaluationDetails.errorCode);
|
||||
if (resolutionDetails.errorCode) {
|
||||
const err = instantiateErrorByErrorCode(resolutionDetails.errorCode);
|
||||
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) {
|
||||
await this.errorHooks(allHooksReversed, hookContext, err, options);
|
||||
return this.getErrorEvaluationDetails(flagKey, defaultValue, err);
|
||||
} finally {
|
||||
await this.finallyHooks(allHooksReversed, hookContext, options);
|
||||
evaluationDetails = this.getErrorEvaluationDetails(flagKey, defaultValue, err);
|
||||
}
|
||||
|
||||
await this.finallyHooks(allHooksReversed, hookContext, evaluationDetails, options);
|
||||
return evaluationDetails;
|
||||
}
|
||||
|
||||
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
|
||||
for (const hook of hooks) {
|
||||
try {
|
||||
await hook?.finally?.(hookContext, options.hookHints);
|
||||
await hook?.finally?.(hookContext, evaluationDetails, options.hookHints);
|
||||
} catch (err) {
|
||||
this._logger.error(`Unhandled error during 'finally' hook: ${err}`);
|
||||
if (err instanceof Error) {
|
||||
|
|
@ -403,6 +412,7 @@ export class OpenFeatureClient implements Client {
|
|||
flagKey: string,
|
||||
defaultValue: T,
|
||||
err: unknown,
|
||||
flagMetadata: FlagMetadata = {},
|
||||
): EvaluationDetails<T> {
|
||||
const errorMessage: string = (err as Error)?.message;
|
||||
const errorCode: ErrorCode = (err as OpenFeatureError)?.code || ErrorCode.GENERAL;
|
||||
|
|
@ -412,7 +422,7 @@ export class OpenFeatureClient implements Client {
|
|||
errorMessage,
|
||||
value: defaultValue,
|
||||
reason: StandardResolutionReasons.ERROR,
|
||||
flagMetadata: Object.freeze({}),
|
||||
flagMetadata: Object.freeze(flagMetadata),
|
||||
flagKey,
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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', () => {
|
||||
|
|
@ -922,14 +946,14 @@ describe('Hooks', () => {
|
|||
done(err);
|
||||
}
|
||||
},
|
||||
after: (_hookContext, _evaluationDetils, hookHints) => {
|
||||
after: (_hookContext, _evaluationDetails, hookHints) => {
|
||||
try {
|
||||
expect(hookHints?.hint).toBeTruthy();
|
||||
} catch (err) {
|
||||
done(err);
|
||||
}
|
||||
},
|
||||
finally: (_, hookHints) => {
|
||||
finally: (_, _evaluationDetails, hookHints) => {
|
||||
try {
|
||||
expect(hookHints?.hint).toBeTruthy();
|
||||
done();
|
||||
|
|
|
|||
|
|
@ -32,9 +32,13 @@ export interface BaseHook<T extends FlagValue = FlagValue, BeforeHookReturn = un
|
|||
|
||||
/**
|
||||
* 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 evaluationDetails
|
||||
* @param hookHints
|
||||
*/
|
||||
finally?(hookContext: Readonly<HookContext<T>>, hookHints?: HookHints): HooksReturn;
|
||||
finally?(
|
||||
hookContext: Readonly<HookContext<T>>,
|
||||
evaluationDetails: EvaluationDetails<T>,
|
||||
hookHints?: HookHints,
|
||||
): HooksReturn;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -10,7 +10,9 @@ import type {
|
|||
Logger,
|
||||
TrackingEventDetails,
|
||||
OpenFeatureError,
|
||||
ResolutionDetails } from '@openfeature/core';
|
||||
FlagMetadata,
|
||||
ResolutionDetails,
|
||||
} from '@openfeature/core';
|
||||
import {
|
||||
ErrorCode,
|
||||
ProviderFatalError,
|
||||
|
|
@ -234,6 +236,8 @@ export class OpenFeatureClient implements Client {
|
|||
logger: this._logger,
|
||||
};
|
||||
|
||||
let evaluationDetails: EvaluationDetails<T>;
|
||||
|
||||
try {
|
||||
this.beforeHooks(allHooks, hookContext, options);
|
||||
|
||||
|
|
@ -242,27 +246,26 @@ export class OpenFeatureClient implements Client {
|
|||
// run the referenced resolver, binding the provider.
|
||||
const resolution = resolver.call(this._provider, flagKey, defaultValue, context, this._logger);
|
||||
|
||||
const evaluationDetails = {
|
||||
const resolutionDetails = {
|
||||
...resolution,
|
||||
flagMetadata: Object.freeze(resolution.flagMetadata ?? {}),
|
||||
flagKey,
|
||||
};
|
||||
|
||||
if (evaluationDetails.errorCode) {
|
||||
const err = instantiateErrorByErrorCode(evaluationDetails.errorCode);
|
||||
if (resolutionDetails.errorCode) {
|
||||
const err = instantiateErrorByErrorCode(resolutionDetails.errorCode);
|
||||
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) {
|
||||
this.errorHooks(allHooksReversed, hookContext, err, options);
|
||||
return this.getErrorEvaluationDetails(flagKey, defaultValue, err);
|
||||
} finally {
|
||||
this.finallyHooks(allHooksReversed, hookContext, options);
|
||||
evaluationDetails = this.getErrorEvaluationDetails(flagKey, defaultValue, err);
|
||||
}
|
||||
this.finallyHooks(allHooksReversed, hookContext, evaluationDetails, options);
|
||||
return evaluationDetails;
|
||||
}
|
||||
|
||||
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
|
||||
for (const hook of hooks) {
|
||||
try {
|
||||
hook?.finally?.(hookContext, options.hookHints);
|
||||
hook?.finally?.(hookContext, evaluationDetails, options.hookHints);
|
||||
} catch (err) {
|
||||
this._logger.error(`Unhandled error during 'finally' hook: ${err}`);
|
||||
if (err instanceof Error) {
|
||||
|
|
@ -337,6 +345,7 @@ export class OpenFeatureClient implements Client {
|
|||
flagKey: string,
|
||||
defaultValue: T,
|
||||
err: unknown,
|
||||
flagMetadata: FlagMetadata = {},
|
||||
): EvaluationDetails<T> {
|
||||
const errorMessage: string = (err as Error)?.message;
|
||||
const errorCode: ErrorCode = (err as OpenFeatureError)?.code || ErrorCode.GENERAL;
|
||||
|
|
@ -346,7 +355,7 @@ export class OpenFeatureClient implements Client {
|
|||
errorMessage,
|
||||
value: defaultValue,
|
||||
reason: StandardResolutionReasons.ERROR,
|
||||
flagMetadata: Object.freeze({}),
|
||||
flagMetadata: Object.freeze(flagMetadata),
|
||||
flagKey,
|
||||
};
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,16 +1,5 @@
|
|||
import type {
|
||||
Provider,
|
||||
ResolutionDetails,
|
||||
Client,
|
||||
FlagValueType,
|
||||
EvaluationContext,
|
||||
Hook} from '../src';
|
||||
import {
|
||||
GeneralError,
|
||||
OpenFeature,
|
||||
StandardResolutionReasons,
|
||||
ErrorCode,
|
||||
} from '../src';
|
||||
import type { Provider, ResolutionDetails, Client, FlagValueType, EvaluationContext, Hook } from '../src';
|
||||
import { GeneralError, OpenFeature, StandardResolutionReasons, ErrorCode } from '../src';
|
||||
|
||||
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', () => {
|
||||
|
|
@ -759,14 +767,14 @@ describe('Hooks', () => {
|
|||
done(err);
|
||||
}
|
||||
},
|
||||
after: (_hookContext, _evaluationDetils, hookHints) => {
|
||||
after: (_hookContext, _evaluationDetails, hookHints) => {
|
||||
try {
|
||||
expect(hookHints?.hint).toBeTruthy();
|
||||
} catch (err) {
|
||||
done(err);
|
||||
}
|
||||
},
|
||||
finally: (_, hookHints) => {
|
||||
finally: (_, _evaluationDetails, hookHints) => {
|
||||
try {
|
||||
expect(hookHints?.hint).toBeTruthy();
|
||||
done();
|
||||
|
|
|
|||
Loading…
Reference in New Issue