Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 3cccae1

Browse files
beeme1mrwichopy
beeme1mr
authored and
wichopy
committedDec 31, 2024
feat: add evaluation details to finally hook (open-feature#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](open-feature/spec#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 <[email protected]> Signed-off-by: Will Chou <[email protected]>
1 parent 23a7fbc commit 3cccae1

File tree

5 files changed

+106
-51
lines changed

5 files changed

+106
-51
lines changed
 

‎packages/server/src/client/internal/open-feature-client.ts

+26-16
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ import type {
1010
Logger,
1111
TrackingEventDetails,
1212
OpenFeatureError,
13-
ResolutionDetails} from '@openfeature/core';
13+
FlagMetadata,
14+
ResolutionDetails,
15+
} from '@openfeature/core';
1416
import {
1517
ErrorCode,
1618
ProviderFatalError,
@@ -24,7 +26,7 @@ import type { FlagEvaluationOptions } from '../../evaluation';
2426
import type { ProviderEvents } from '../../events';
2527
import type { InternalEventEmitter } from '../../events/internal/internal-event-emitter';
2628
import type { Hook } from '../../hooks';
27-
import type { Provider} from '../../provider';
29+
import type { Provider } from '../../provider';
2830
import { ProviderStatus } from '../../provider';
2931
import type { Client } from './../client';
3032

@@ -279,6 +281,8 @@ export class OpenFeatureClient implements Client {
279281
logger: this._logger,
280282
};
281283

284+
let evaluationDetails: EvaluationDetails<T>;
285+
282286
try {
283287
const frozenContext = await this.beforeHooks(allHooks, hookContext, options);
284288

@@ -287,27 +291,27 @@ export class OpenFeatureClient implements Client {
287291
// run the referenced resolver, binding the provider.
288292
const resolution = await resolver.call(this._provider, flagKey, defaultValue, frozenContext, this._logger);
289293

290-
const evaluationDetails = {
294+
const resolutionDetails = {
291295
...resolution,
292296
flagMetadata: Object.freeze(resolution.flagMetadata ?? {}),
293297
flagKey,
294298
};
295299

296-
if (evaluationDetails.errorCode) {
297-
const err = instantiateErrorByErrorCode(evaluationDetails.errorCode);
300+
if (resolutionDetails.errorCode) {
301+
const err = instantiateErrorByErrorCode(resolutionDetails.errorCode);
298302
await this.errorHooks(allHooksReversed, hookContext, err, options);
299-
return this.getErrorEvaluationDetails(flagKey, defaultValue, err);
303+
evaluationDetails = this.getErrorEvaluationDetails(flagKey, defaultValue, err, resolutionDetails.flagMetadata);
304+
} else {
305+
await this.afterHooks(allHooksReversed, hookContext, resolutionDetails, options);
306+
evaluationDetails = resolutionDetails;
300307
}
301-
302-
await this.afterHooks(allHooksReversed, hookContext, evaluationDetails, options);
303-
304-
return evaluationDetails;
305308
} catch (err: unknown) {
306309
await this.errorHooks(allHooksReversed, hookContext, err, options);
307-
return this.getErrorEvaluationDetails(flagKey, defaultValue, err);
308-
} finally {
309-
await this.finallyHooks(allHooksReversed, hookContext, options);
310+
evaluationDetails = this.getErrorEvaluationDetails(flagKey, defaultValue, err);
310311
}
312+
313+
await this.finallyHooks(allHooksReversed, hookContext, evaluationDetails, options);
314+
return evaluationDetails;
311315
}
312316

313317
private async beforeHooks(hooks: Hook[], hookContext: HookContext, options: FlagEvaluationOptions) {
@@ -353,11 +357,16 @@ export class OpenFeatureClient implements Client {
353357
}
354358
}
355359

356-
private async finallyHooks(hooks: Hook[], hookContext: HookContext, options: FlagEvaluationOptions) {
360+
private async finallyHooks(
361+
hooks: Hook[],
362+
hookContext: HookContext,
363+
evaluationDetails: EvaluationDetails<FlagValue>,
364+
options: FlagEvaluationOptions,
365+
) {
357366
// run "finally" hooks sequentially
358367
for (const hook of hooks) {
359368
try {
360-
await hook?.finally?.(hookContext, options.hookHints);
369+
await hook?.finally?.(hookContext, evaluationDetails, options.hookHints);
361370
} catch (err) {
362371
this._logger.error(`Unhandled error during 'finally' hook: ${err}`);
363372
if (err instanceof Error) {
@@ -403,6 +412,7 @@ export class OpenFeatureClient implements Client {
403412
flagKey: string,
404413
defaultValue: T,
405414
err: unknown,
415+
flagMetadata: FlagMetadata = {},
406416
): EvaluationDetails<T> {
407417
const errorMessage: string = (err as Error)?.message;
408418
const errorCode: ErrorCode = (err as OpenFeatureError)?.code || ErrorCode.GENERAL;
@@ -412,7 +422,7 @@ export class OpenFeatureClient implements Client {
412422
errorMessage,
413423
value: defaultValue,
414424
reason: StandardResolutionReasons.ERROR,
415-
flagMetadata: Object.freeze({}),
425+
flagMetadata: Object.freeze(flagMetadata),
416426
flagKey,
417427
};
418428
}

‎packages/server/test/hooks.spec.ts

+26-2
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,30 @@ describe('Hooks', () => {
439439
});
440440
});
441441
});
442+
443+
describe('Requirement 4.3.8', () => {
444+
it('"evaluation details" passed to the "finally" stage matches the evaluation details returned to the application author', async () => {
445+
OpenFeature.setProvider(MOCK_PROVIDER);
446+
let evaluationDetailsHooks;
447+
448+
const evaluationDetails = await client.getBooleanDetails(
449+
FLAG_KEY,
450+
false,
451+
{},
452+
{
453+
hooks: [
454+
{
455+
finally: (_, details) => {
456+
evaluationDetailsHooks = details;
457+
},
458+
},
459+
],
460+
},
461+
);
462+
463+
expect(evaluationDetailsHooks).toEqual(evaluationDetails);
464+
});
465+
});
442466
});
443467

444468
describe('Requirement 4.4.2', () => {
@@ -922,14 +946,14 @@ describe('Hooks', () => {
922946
done(err);
923947
}
924948
},
925-
after: (_hookContext, _evaluationDetils, hookHints) => {
949+
after: (_hookContext, _evaluationDetails, hookHints) => {
926950
try {
927951
expect(hookHints?.hint).toBeTruthy();
928952
} catch (err) {
929953
done(err);
930954
}
931955
},
932-
finally: (_, hookHints) => {
956+
finally: (_, _evaluationDetails, hookHints) => {
933957
try {
934958
expect(hookHints?.hint).toBeTruthy();
935959
done();

‎packages/shared/src/hooks/hook.ts

+6-2
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,13 @@ export interface BaseHook<T extends FlagValue = FlagValue, BeforeHookReturn = un
3232

3333
/**
3434
* Runs after all other hook stages, regardless of success or error.
35-
* Errors thrown here are unhandled by the client and will surface in application code.
3635
* @param hookContext
36+
* @param evaluationDetails
3737
* @param hookHints
3838
*/
39-
finally?(hookContext: Readonly<HookContext<T>>, hookHints?: HookHints): HooksReturn;
39+
finally?(
40+
hookContext: Readonly<HookContext<T>>,
41+
evaluationDetails: EvaluationDetails<T>,
42+
hookHints?: HookHints,
43+
): HooksReturn;
4044
}

‎packages/web/src/client/internal/open-feature-client.ts

+25-16
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ import type {
1010
Logger,
1111
TrackingEventDetails,
1212
OpenFeatureError,
13-
ResolutionDetails } from '@openfeature/core';
13+
FlagMetadata,
14+
ResolutionDetails,
15+
} from '@openfeature/core';
1416
import {
1517
ErrorCode,
1618
ProviderFatalError,
@@ -24,7 +26,7 @@ import type { FlagEvaluationOptions } from '../../evaluation';
2426
import type { ProviderEvents } from '../../events';
2527
import type { InternalEventEmitter } from '../../events/internal/internal-event-emitter';
2628
import type { Hook } from '../../hooks';
27-
import type { Provider} from '../../provider';
29+
import type { Provider } from '../../provider';
2830
import { ProviderStatus } from '../../provider';
2931
import type { Client } from './../client';
3032

@@ -234,6 +236,8 @@ export class OpenFeatureClient implements Client {
234236
logger: this._logger,
235237
};
236238

239+
let evaluationDetails: EvaluationDetails<T>;
240+
237241
try {
238242
this.beforeHooks(allHooks, hookContext, options);
239243

@@ -242,27 +246,26 @@ export class OpenFeatureClient implements Client {
242246
// run the referenced resolver, binding the provider.
243247
const resolution = resolver.call(this._provider, flagKey, defaultValue, context, this._logger);
244248

245-
const evaluationDetails = {
249+
const resolutionDetails = {
246250
...resolution,
247251
flagMetadata: Object.freeze(resolution.flagMetadata ?? {}),
248252
flagKey,
249253
};
250254

251-
if (evaluationDetails.errorCode) {
252-
const err = instantiateErrorByErrorCode(evaluationDetails.errorCode);
255+
if (resolutionDetails.errorCode) {
256+
const err = instantiateErrorByErrorCode(resolutionDetails.errorCode);
253257
this.errorHooks(allHooksReversed, hookContext, err, options);
254-
return this.getErrorEvaluationDetails(flagKey, defaultValue, err);
258+
evaluationDetails = this.getErrorEvaluationDetails(flagKey, defaultValue, err, resolutionDetails.flagMetadata);
259+
} else {
260+
this.afterHooks(allHooksReversed, hookContext, resolutionDetails, options);
261+
evaluationDetails = resolutionDetails;
255262
}
256-
257-
this.afterHooks(allHooksReversed, hookContext, evaluationDetails, options);
258-
259-
return evaluationDetails;
260263
} catch (err: unknown) {
261264
this.errorHooks(allHooksReversed, hookContext, err, options);
262-
return this.getErrorEvaluationDetails(flagKey, defaultValue, err);
263-
} finally {
264-
this.finallyHooks(allHooksReversed, hookContext, options);
265+
evaluationDetails = this.getErrorEvaluationDetails(flagKey, defaultValue, err);
265266
}
267+
this.finallyHooks(allHooksReversed, hookContext, evaluationDetails, options);
268+
return evaluationDetails;
266269
}
267270

268271
private beforeHooks(hooks: Hook[], hookContext: HookContext, options: FlagEvaluationOptions) {
@@ -301,11 +304,16 @@ export class OpenFeatureClient implements Client {
301304
}
302305
}
303306

304-
private finallyHooks(hooks: Hook[], hookContext: HookContext, options: FlagEvaluationOptions) {
307+
private finallyHooks(
308+
hooks: Hook[],
309+
hookContext: HookContext,
310+
evaluationDetails: EvaluationDetails<FlagValue>,
311+
options: FlagEvaluationOptions,
312+
) {
305313
// run "finally" hooks sequentially
306314
for (const hook of hooks) {
307315
try {
308-
hook?.finally?.(hookContext, options.hookHints);
316+
hook?.finally?.(hookContext, evaluationDetails, options.hookHints);
309317
} catch (err) {
310318
this._logger.error(`Unhandled error during 'finally' hook: ${err}`);
311319
if (err instanceof Error) {
@@ -337,6 +345,7 @@ export class OpenFeatureClient implements Client {
337345
flagKey: string,
338346
defaultValue: T,
339347
err: unknown,
348+
flagMetadata: FlagMetadata = {},
340349
): EvaluationDetails<T> {
341350
const errorMessage: string = (err as Error)?.message;
342351
const errorCode: ErrorCode = (err as OpenFeatureError)?.code || ErrorCode.GENERAL;
@@ -346,7 +355,7 @@ export class OpenFeatureClient implements Client {
346355
errorMessage,
347356
value: defaultValue,
348357
reason: StandardResolutionReasons.ERROR,
349-
flagMetadata: Object.freeze({}),
358+
flagMetadata: Object.freeze(flagMetadata),
350359
flagKey,
351360
};
352361
}

‎packages/web/test/hooks.spec.ts

+23-15
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,5 @@
1-
import type {
2-
Provider,
3-
ResolutionDetails,
4-
Client,
5-
FlagValueType,
6-
EvaluationContext,
7-
Hook} from '../src';
8-
import {
9-
GeneralError,
10-
OpenFeature,
11-
StandardResolutionReasons,
12-
ErrorCode,
13-
} from '../src';
1+
import type { Provider, ResolutionDetails, Client, FlagValueType, EvaluationContext, Hook } from '../src';
2+
import { GeneralError, OpenFeature, StandardResolutionReasons, ErrorCode } from '../src';
143

154
const BOOLEAN_VALUE = true;
165

@@ -282,6 +271,25 @@ describe('Hooks', () => {
282271
});
283272
});
284273
});
274+
275+
describe('Requirement 4.3.8', () => {
276+
it('"evaluation details" passed to the "finally" stage matches the evaluation details returned to the application author', () => {
277+
OpenFeature.setProvider(MOCK_PROVIDER);
278+
let evaluationDetailsHooks;
279+
280+
const evaluationDetails = client.getBooleanDetails(FLAG_KEY, false, {
281+
hooks: [
282+
{
283+
finally: (_, details) => {
284+
evaluationDetailsHooks = details;
285+
},
286+
},
287+
],
288+
});
289+
290+
expect(evaluationDetailsHooks).toEqual(evaluationDetails);
291+
});
292+
});
285293
});
286294

287295
describe('Requirement 4.4.2', () => {
@@ -759,14 +767,14 @@ describe('Hooks', () => {
759767
done(err);
760768
}
761769
},
762-
after: (_hookContext, _evaluationDetils, hookHints) => {
770+
after: (_hookContext, _evaluationDetails, hookHints) => {
763771
try {
764772
expect(hookHints?.hint).toBeTruthy();
765773
} catch (err) {
766774
done(err);
767775
}
768776
},
769-
finally: (_, hookHints) => {
777+
finally: (_, _evaluationDetails, hookHints) => {
770778
try {
771779
expect(hookHints?.hint).toBeTruthy();
772780
done();

0 commit comments

Comments
 (0)
Please sign in to comment.