Skip to content

feat: add evaluation details to finally hook #1087

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
beeme1mr merged 5 commits into from
Dec 12, 2024
Merged

feat: add evaluation details to finally hook #1087

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bfd150d
feat: add evaluation details to finally hook
beeme1mr Nov 21, 2024
9310ccf
add tests
beeme1mr Dec 9, 2024
e8f567f
Merge branch 'main' into expand-finally-hook-stage
beeme1mr Dec 9, 2024
10f557f
remove iife ito reduce memory allocations
beeme1mr Dec 11, 2024
9346463
add missing changes
beeme1mr Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 26 additions & 16 deletions packages/server/src/client/internal/open-feature-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import type {
Logger,
TrackingEventDetails,
OpenFeatureError,
ResolutionDetails} from '@openfeature/core';
FlagMetadata,
ResolutionDetails,
} from '@openfeature/core';
import {
ErrorCode,
ProviderFatalError,
Expand All @@ -24,7 +26,7 @@ import type { FlagEvaluationOptions } from '../../evaluation';
import type { ProviderEvents } from '../../events';
import type { InternalEventEmitter } from '../../events/internal/internal-event-emitter';
import type { Hook } from '../../hooks';
import type { Provider} from '../../provider';
import type { Provider } from '../../provider';
import { ProviderStatus } from '../../provider';
import type { Client } from './../client';

Expand Down Expand Up @@ -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);

Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand All @@ -412,7 +422,7 @@ export class OpenFeatureClient implements Client {
errorMessage,
value: defaultValue,
reason: StandardResolutionReasons.ERROR,
flagMetadata: Object.freeze({}),
flagMetadata: Object.freeze(flagMetadata),
flagKey,
};
}
Expand Down
28 changes: 26 additions & 2 deletions packages/server/test/hooks.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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();
Expand Down
8 changes: 6 additions & 2 deletions packages/shared/src/hooks/hook.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
41 changes: 25 additions & 16 deletions packages/web/src/client/internal/open-feature-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import type {
Logger,
TrackingEventDetails,
OpenFeatureError,
ResolutionDetails } from '@openfeature/core';
FlagMetadata,
ResolutionDetails,
} from '@openfeature/core';
import {
ErrorCode,
ProviderFatalError,
Expand All @@ -24,7 +26,7 @@ import type { FlagEvaluationOptions } from '../../evaluation';
import type { ProviderEvents } from '../../events';
import type { InternalEventEmitter } from '../../events/internal/internal-event-emitter';
import type { Hook } from '../../hooks';
import type { Provider} from '../../provider';
import type { Provider } from '../../provider';
import { ProviderStatus } from '../../provider';
import type { Client } from './../client';

Expand Down Expand Up @@ -234,6 +236,8 @@ export class OpenFeatureClient implements Client {
logger: this._logger,
};

let evaluationDetails: EvaluationDetails<T>;

try {
this.beforeHooks(allHooks, hookContext, options);

Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand All @@ -346,7 +355,7 @@ export class OpenFeatureClient implements Client {
errorMessage,
value: defaultValue,
reason: StandardResolutionReasons.ERROR,
flagMetadata: Object.freeze({}),
flagMetadata: Object.freeze(flagMetadata),
flagKey,
};
}
Expand Down
38 changes: 23 additions & 15 deletions packages/web/test/hooks.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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();
Expand Down