Skip to content

Commit 413e962

Browse files
beeme1mrbeeme1mr
committed
avoid throwing, include metadata, add tests
Signed-off-by: Michael Beemer <[email protected]>
1 parent b8a9bd5 commit 413e962

File tree

13 files changed

+644
-419
lines changed

13 files changed

+644
-419
lines changed

‎libs/shared/flagd-core/src/lib/feature-flag.spec.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,26 @@ describe('Flagd flag structure', () => {
2929
expect(ff.variants.get('off')).toBeFalsy();
3030
});
3131

32+
it('should be constructed with valid input - string', () => {
33+
const input: Flag = {
34+
state: 'ENABLED',
35+
defaultVariant: 'off',
36+
variants: {
37+
on: 'on',
38+
off: 'off',
39+
},
40+
targeting: '',
41+
};
42+
43+
const ff = new FeatureFlag('test', input, logger);
44+
45+
expect(ff).toBeTruthy();
46+
expect(ff.state).toBe('ENABLED');
47+
expect(ff.defaultVariant).toBe('off');
48+
expect(ff.variants.get('on')).toBe('on');
49+
expect(ff.variants.get('off')).toBe('off');
50+
});
51+
3252
it('should be constructed with valid input - number', () => {
3353
const input: Flag = {
3454
state: 'ENABLED',

‎libs/shared/flagd-core/src/lib/feature-flag.ts‎

Lines changed: 57 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ import type {
44
ResolutionDetails,
55
JsonValue,
66
Logger,
7-
ResolutionReason,
87
EvaluationContext,
8+
ResolutionReason,
99
} from '@openfeature/core';
10-
import { ParseError, StandardResolutionReasons, GeneralError, TypeMismatchError } from '@openfeature/core';
10+
import { ParseError, StandardResolutionReasons, ErrorCode } from '@openfeature/core';
1111
import { sha1 } from 'object-hash';
1212
import { Targeting } from './targeting/targeting';
1313

@@ -22,7 +22,20 @@ export interface Flag {
2222
metadata?: FlagMetadata;
2323
}
2424

25-
type ResolutionDetailsWithFlagMetadata<T> = Required<Pick<ResolutionDetails<T>, 'flagMetadata'>> & ResolutionDetails<T>;
25+
type RequiredResolutionDetails<T> = Omit<ResolutionDetails<T>, 'value'> & {
26+
flagMetadata: FlagMetadata;
27+
} & (
28+
| {
29+
reason: 'ERROR';
30+
errorCode: ErrorCode;
31+
errorMessage: string;
32+
value?: never;
33+
}
34+
| {
35+
value: T;
36+
variant: string;
37+
}
38+
);
2639

2740
/**
2841
* Flagd flag configuration structure for internal reference.
@@ -35,9 +48,13 @@ export class FeatureFlag {
3548
private readonly _hash: string;
3649
private readonly _metadata: FlagMetadata;
3750
private readonly _targeting?: Targeting;
38-
private readonly _targetingParseError?: Error;
51+
private readonly _targetingParseErrorMessage?: string;
3952

40-
constructor(key: string, flag: Flag, logger: Logger) {
53+
constructor(
54+
key: string,
55+
flag: Flag,
56+
private readonly logger: Logger,
57+
) {
4158
this._key = key;
4259
this._state = flag['state'];
4360
this._defaultVariant = flag['defaultVariant'];
@@ -49,8 +66,8 @@ export class FeatureFlag {
4966
this._targeting = new Targeting(flag.targeting, logger);
5067
} catch (err) {
5168
const message = `Invalid targeting configuration for flag '${key}'`;
52-
logger.warn(message);
53-
this._targetingParseError = new ParseError(message, { cause: err });
69+
this.logger.warn(message);
70+
this._targetingParseErrorMessage = message;
5471
}
5572
}
5673
this._hash = sha1(flag);
@@ -82,21 +99,34 @@ export class FeatureFlag {
8299
return this._metadata;
83100
}
84101

85-
evaluate(evalCtx: EvaluationContext): ResolutionDetailsWithFlagMetadata<JsonValue> {
102+
evaluate(evalCtx: EvaluationContext, logger: Logger = this.logger): RequiredResolutionDetails<JsonValue> {
86103
let variant: string;
87104
let reason: ResolutionReason;
88105

89-
if (this._targetingParseError) {
90-
throw this._targetingParseError;
91-
} else if (!this._targeting) {
106+
if (this._targetingParseErrorMessage) {
107+
return {
108+
reason: StandardResolutionReasons.ERROR,
109+
errorCode: ErrorCode.PARSE_ERROR,
110+
errorMessage: this._targetingParseErrorMessage,
111+
flagMetadata: this.metadata,
112+
};
113+
}
114+
115+
if (!this._targeting) {
92116
variant = this._defaultVariant;
93117
reason = StandardResolutionReasons.STATIC;
94118
} else {
95119
let targetingResolution: JsonValue;
96120
try {
97-
targetingResolution = this._targeting.evaluate(this._key, evalCtx);
121+
targetingResolution = this._targeting.evaluate(this._key, evalCtx, logger);
98122
} catch (e) {
99-
throw new GeneralError(`Error evaluating targeting rule for flag '${this._key}'`, { cause: e });
123+
logger.debug(`Error evaluating targeting rule for flag '${this._key}': ${(e as Error).message}`);
124+
return {
125+
reason: StandardResolutionReasons.ERROR,
126+
errorCode: ErrorCode.GENERAL,
127+
errorMessage: `Error evaluating targeting rule for flag '${this._key}'`,
128+
flagMetadata: this.metadata,
129+
};
100130
}
101131

102132
// Return default variant if targeting resolution is null or undefined
@@ -110,13 +140,23 @@ export class FeatureFlag {
110140
}
111141
}
112142

113-
if (typeof variant !== 'string') {
114-
throw new TypeMismatchError(`Variant must be a string, but found '${typeof variant}'`);
115-
}
143+
// if (typeof variant !== 'string') {
144+
// return {
145+
// reason: StandardResolutionReasons.ERROR,
146+
// errorCode: ErrorCode.GENERAL,
147+
// errorMessage: `Variant must be a string, but found '${typeof variant}'`,
148+
// flagMetadata: this.metadata,
149+
// };
150+
// }
116151

117152
const resolvedVariant = this._variants.get(variant);
118153
if (resolvedVariant === undefined) {
119-
throw new GeneralError(`Variant '${variant}' not found in flag with key '${this._key}'`);
154+
return {
155+
reason: StandardResolutionReasons.ERROR,
156+
errorCode: ErrorCode.GENERAL,
157+
errorMessage: `Variant '${variant}' not found in flag with key '${this._key}'`,
158+
flagMetadata: this.metadata,
159+
};
120160
}
121161

122162
return {

‎libs/shared/flagd-core/src/lib/flagd-core.spec.ts‎

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,3 +251,119 @@ describe('flagd-core common flag definitions', () => {
251251
expect(() => core.setConfigurations(flagCfg)).toThrow(ParseError);
252252
});
253253
});
254+
255+
describe('flagd-core flag metadata', () => {
256+
const targetingFlag =
257+
'{"flags":{"targetedFlag":{"variants":{"first":"AAA","second":"BBB","third":"CCC"},"defaultVariant":"first","state":"ENABLED","targeting":{"if":[{"in":["@openfeature.dev",{"var":"email"}]},"second",null]},"metadata":{"owner": "mike"}},"shortCircuit":{"variants":{"true":true,"false":false},"defaultVariant":"false","state":"ENABLED","targeting":{"==":[{"var":"favoriteNumber"},1]}}},"metadata":{"id":"dev","version":"1"}}';
258+
let core: FlagdCore;
259+
260+
beforeAll(() => {
261+
core = new FlagdCore();
262+
core.setConfigurations(targetingFlag);
263+
});
264+
265+
it('should return "targetedFlag" flag metadata', () => {
266+
const resolved = core.resolveStringEvaluation('targetedFlag', 'none', { email: '[email protected]' });
267+
expect(resolved.flagMetadata).toEqual({ flagSetVersion: '1', flagSetId: 'dev', owner: 'mike' });
268+
});
269+
270+
it('should return "shortCircuit" flag metadata', () => {
271+
const resolved = core.resolveBooleanEvaluation('shortCircuit', false, { favoriteNumber: 1 });
272+
expect(resolved.flagMetadata).toEqual({ flagSetVersion: '1', flagSetId: 'dev' });
273+
});
274+
});
275+
276+
describe('flagd-core error conditions', () => {
277+
const errorFlags = {
278+
flags: {
279+
basic: {
280+
variants: { on: true, off: false },
281+
defaultVariant: 'on',
282+
state: 'ENABLED',
283+
},
284+
disabledFlag: {
285+
variants: { on: true, off: false },
286+
defaultVariant: 'on',
287+
state: 'DISABLED',
288+
},
289+
invalidTargetingRule: {
290+
variants: { on: true, off: false },
291+
defaultVariant: 'on',
292+
state: 'ENABLED',
293+
targeting: { invalid: true },
294+
},
295+
invalidVariantName: {
296+
variants: { true: true, false: false },
297+
defaultVariant: 'false',
298+
state: 'ENABLED',
299+
targeting: { if: [true, 'invalid'] },
300+
},
301+
},
302+
metadata: { id: 'dev', version: '1' },
303+
};
304+
let core: FlagdCore;
305+
306+
beforeAll(() => {
307+
core = new FlagdCore();
308+
core.setConfigurations(JSON.stringify(errorFlags));
309+
});
310+
311+
it('should not find the flag', () => {
312+
const resolved = core.resolveBooleanEvaluation('invalid', false, {});
313+
expect(resolved.reason).toBe(StandardResolutionReasons.ERROR);
314+
expect(resolved.errorCode).toBe(ErrorCode.FLAG_NOT_FOUND);
315+
expect(resolved.errorMessage).toBeTruthy();
316+
expect(resolved.flagMetadata).toEqual({ flagSetVersion: '1', flagSetId: 'dev' });
317+
});
318+
319+
it('should treat disabled flags as not found', () => {
320+
const resolved = core.resolveBooleanEvaluation('disabledFlag', false, {});
321+
expect(resolved.reason).toBe(StandardResolutionReasons.ERROR);
322+
expect(resolved.errorCode).toBe(ErrorCode.FLAG_NOT_FOUND);
323+
expect(resolved.errorMessage).toBeTruthy();
324+
expect(resolved.flagMetadata).toEqual({ flagSetVersion: '1', flagSetId: 'dev' });
325+
});
326+
327+
it('should return a parse error code', () => {
328+
const resolved = core.resolveBooleanEvaluation('invalidTargetingRule', false, {});
329+
expect(resolved.reason).toBe(StandardResolutionReasons.ERROR);
330+
expect(resolved.errorCode).toBe(ErrorCode.PARSE_ERROR);
331+
expect(resolved.errorMessage).toBeTruthy();
332+
expect(resolved.flagMetadata).toEqual({ flagSetVersion: '1', flagSetId: 'dev' });
333+
});
334+
335+
// it('should return a general error if targeting evaluate fails', () => {
336+
// const evaluationErrorCore = new FlagdCore({
337+
// setConfigurations: jest.fn(),
338+
// getFlag: () => {
339+
// const featureFlag = new FeatureFlag('basic', {}, logger)
340+
// featureFlag[_targeting] = () => throw new Error("something broke");
341+
// return featureFlag;
342+
// },
343+
// getFlags: jest.fn(),
344+
// getFlagSetMetadata: jest.fn(),
345+
// });
346+
347+
// const resolved = evaluationErrorCore.resolveBooleanEvaluation('basic', false, {});
348+
// expect(resolved.reason).toBe(StandardResolutionReasons.ERROR);
349+
// expect(resolved.errorCode).toBe(ErrorCode.PARSE_ERROR);
350+
// expect(resolved.errorMessage).toBeTruthy();
351+
// expect(resolved.flagMetadata).toEqual({ flagSetVersion: '1', flagSetId: 'dev' });
352+
// });
353+
354+
it('should return a general error if the variant is not a string', () => {
355+
const resolved = core.resolveBooleanEvaluation('invalidVariantName', false, {});
356+
expect(resolved.reason).toBe(StandardResolutionReasons.ERROR);
357+
expect(resolved.errorCode).toBe(ErrorCode.GENERAL);
358+
expect(resolved.errorMessage).toBeTruthy();
359+
expect(resolved.flagMetadata).toEqual({ flagSetVersion: '1', flagSetId: 'dev' });
360+
});
361+
362+
it('should return a type mismatch error', () => {
363+
const resolved = core.resolveStringEvaluation('basic', 'false', {});
364+
expect(resolved.reason).toBe(StandardResolutionReasons.ERROR);
365+
expect(resolved.errorCode).toBe(ErrorCode.TYPE_MISMATCH);
366+
expect(resolved.errorMessage).toBeTruthy();
367+
expect(resolved.flagMetadata).toEqual({ flagSetVersion: '1', flagSetId: 'dev' });
368+
});
369+
});

0 commit comments

Comments
 (0)