Skip to content

Commit e9f9e74

Browse files
toddbaerttoddbaert
authored
fix: init in-process error, throw on invalid rules (open-feature#767)
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
1 parent fe1f4f9 commit e9f9e74

File tree

11 files changed

+212
-55
lines changed

11 files changed

+212
-55
lines changed

‎libs/providers/flagd/flagd-testbed‎

‎libs/providers/flagd/src/e2e/constants.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
export const FLAGD_NAME = 'flagd Provider';
2+
export const E2E_CLIENT_NAME = 'e2e';
3+
export const UNSTABLE_CLIENT_NAME = 'unstable';
4+
export const UNAVAILABLE_CLIENT_NAME = 'unavailable';

‎libs/providers/flagd/src/e2e/setup-in-process-provider.ts‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,44 @@
11
import assert from 'assert';
22
import { OpenFeature } from '@openfeature/server-sdk';
33
import { FlagdProvider } from '../lib/flagd-provider';
4-
5-
const FLAGD_NAME = 'flagd Provider';
6-
const E2E_CLIENT_NAME = 'e2e';
7-
const UNSTABLE_CLIENT_NAME = 'unstable';
4+
import { E2E_CLIENT_NAME, FLAGD_NAME, UNSTABLE_CLIENT_NAME, UNAVAILABLE_CLIENT_NAME } from './constants';
85

96
// register the flagd provider before the tests.
107
console.log('Setting flagd provider...');
118
OpenFeature.setProvider(
129
E2E_CLIENT_NAME,
13-
new FlagdProvider({ cache: 'disabled', resolverType: 'in-process', host: 'localhost', port: 9090 }),
10+
new FlagdProvider({ resolverType: 'in-process', host: 'localhost', port: 9090 }),
1411
);
1512
OpenFeature.setProvider(
1613
UNSTABLE_CLIENT_NAME,
1714
new FlagdProvider({ resolverType: 'in-process', host: 'localhost', port: 9091 }),
1815
);
16+
OpenFeature.setProvider(
17+
UNAVAILABLE_CLIENT_NAME,
18+
new FlagdProvider({ resolverType: 'in-process', host: 'localhost', port: 9092 }),
19+
);
1920
assert(
2021
OpenFeature.getProviderMetadata(E2E_CLIENT_NAME).name === FLAGD_NAME,
21-
new Error(`Expected ${FLAGD_NAME} provider to be configured, instead got: ${OpenFeature.providerMetadata.name}`),
22+
new Error(
23+
`Expected ${FLAGD_NAME} provider to be configured, instead got: ${
24+
OpenFeature.getProviderMetadata(E2E_CLIENT_NAME).name
25+
}`,
26+
),
2227
);
2328
assert(
2429
OpenFeature.getProviderMetadata(UNSTABLE_CLIENT_NAME).name === FLAGD_NAME,
25-
new Error(`Expected ${FLAGD_NAME} provider to be configured, instead got: ${OpenFeature.providerMetadata.name}`),
30+
new Error(
31+
`Expected ${FLAGD_NAME} provider to be configured, instead got: ${
32+
OpenFeature.getProviderMetadata(UNSTABLE_CLIENT_NAME).name
33+
}`,
34+
),
35+
);
36+
assert(
37+
OpenFeature.getProviderMetadata(UNAVAILABLE_CLIENT_NAME).name === FLAGD_NAME,
38+
new Error(
39+
`Expected ${FLAGD_NAME} provider to be configured, instead got: ${
40+
OpenFeature.getProviderMetadata(UNAVAILABLE_CLIENT_NAME).name
41+
}`,
42+
),
2643
);
2744
console.log('flagd provider configured!');

‎libs/providers/flagd/src/e2e/setup-rpc-provider.ts‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,35 @@
11
import assert from 'assert';
22
import { OpenFeature } from '@openfeature/server-sdk';
33
import { FlagdProvider } from '../lib/flagd-provider';
4-
5-
const FLAGD_NAME = 'flagd Provider';
6-
const E2E_CLIENT_NAME = 'e2e';
7-
const UNSTABLE_CLIENT_NAME = 'unstable';
4+
import { E2E_CLIENT_NAME, FLAGD_NAME, UNSTABLE_CLIENT_NAME, UNAVAILABLE_CLIENT_NAME } from './constants';
85

96
// register the flagd provider before the tests.
107
console.log('Setting flagd provider...');
118
OpenFeature.setProvider(E2E_CLIENT_NAME, new FlagdProvider({ cache: 'disabled' }));
129
OpenFeature.setProvider(UNSTABLE_CLIENT_NAME, new FlagdProvider({ cache: 'disabled', port: 8014 }));
10+
OpenFeature.setProvider(UNAVAILABLE_CLIENT_NAME, new FlagdProvider({ cache: 'disabled', port: 8015 }));
1311
assert(
1412
OpenFeature.getProviderMetadata(E2E_CLIENT_NAME).name === FLAGD_NAME,
15-
new Error(`Expected ${FLAGD_NAME} provider to be configured, instead got: ${OpenFeature.providerMetadata.name}`),
13+
new Error(
14+
`Expected ${FLAGD_NAME} provider to be configured, instead got: ${
15+
OpenFeature.getProviderMetadata(E2E_CLIENT_NAME).name
16+
}`,
17+
),
1618
);
1719
assert(
1820
OpenFeature.getProviderMetadata(UNSTABLE_CLIENT_NAME).name === FLAGD_NAME,
19-
new Error(`Expected ${FLAGD_NAME} provider to be configured, instead got: ${OpenFeature.providerMetadata.name}`),
21+
new Error(
22+
`Expected ${FLAGD_NAME} provider to be configured, instead got: ${
23+
OpenFeature.getProviderMetadata(UNSTABLE_CLIENT_NAME).name
24+
}`,
25+
),
26+
);
27+
assert(
28+
OpenFeature.getProviderMetadata(UNAVAILABLE_CLIENT_NAME).name === FLAGD_NAME,
29+
new Error(
30+
`Expected ${FLAGD_NAME} provider to be configured, instead got: ${
31+
OpenFeature.getProviderMetadata(UNAVAILABLE_CLIENT_NAME).name
32+
}`,
33+
),
2034
);
2135
console.log('flagd provider configured!');

‎libs/providers/flagd/src/e2e/step-definitions/evaluation.spec.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,13 @@ import {
99
StandardResolutionReasons,
1010
} from '@openfeature/server-sdk';
1111
import { defineFeature, loadFeature } from 'jest-cucumber';
12+
import { E2E_CLIENT_NAME } from '../constants';
1213

1314
// load the feature file.
1415
const feature = loadFeature('features/evaluation.feature');
1516

1617
// get a client (flagd provider registered in setup)
17-
const client = OpenFeature.getClient('e2e');
18+
const client = OpenFeature.getClient(E2E_CLIENT_NAME);
1819

1920
const givenAnOpenfeatureClientIsRegistered = (
2021
given: (stepMatcher: string, stepDefinitionCallback: () => void) => void,

‎libs/providers/flagd/src/e2e/step-definitions/flagd-json-evaluator.spec.ts‎

Lines changed: 72 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
1-
import { EvaluationContext, OpenFeature, ProviderEvents } from '@openfeature/server-sdk';
1+
import { EvaluationContext, EvaluationDetails, OpenFeature, ProviderEvents } from '@openfeature/server-sdk';
22
import { defineFeature, loadFeature } from 'jest-cucumber';
33
import { StepsDefinitionCallbackFunction } from 'jest-cucumber/dist/src/feature-definition-creation';
4+
import { E2E_CLIENT_NAME } from '../constants';
45

56
// load the feature file.
67
const feature = loadFeature('features/flagd-json-evaluator.feature');
78

89
// get a client (flagd provider registered in setup)
9-
const client = OpenFeature.getClient('e2e');
10+
const client = OpenFeature.getClient(E2E_CLIENT_NAME);
1011

1112
const aFlagProviderIsSet = (given: (stepMatcher: string, stepDefinitionCallback: () => void) => void) => {
1213
given('a flagd provider is set', () => undefined);
@@ -56,10 +57,12 @@ defineFeature(feature, (test) => {
5657
defaultValue = defaultVal;
5758
});
5859
and(
59-
/^a context containing a nested property with outer key "(.*)" and inner key "(.*)", with value "(.*)"$/,
60+
/^a context containing a nested property with outer key "(.*)" and inner key "(.*)", with value (.*)$/,
6061
(outerKey: string, innerKey: string, value: string) => {
62+
// we have to support string and non-string params in this test (we test invalid context value 3)
63+
const valueNoQuotes = value.replaceAll('"', '');
6164
evaluationContext[outerKey] = {
62-
[innerKey]: value,
65+
[innerKey]: parseInt(valueNoQuotes) || valueNoQuotes,
6366
};
6467
},
6568
);
@@ -71,7 +74,70 @@ defineFeature(feature, (test) => {
7174

7275
test('Substring operators', evaluateStringFlagWithContext);
7376

74-
test('Semantic version operator numeric comparision', evaluateStringFlagWithContext);
77+
test('Semantic version operator numeric comparison', evaluateStringFlagWithContext);
7578

76-
test('Semantic version operator semantic comparision', evaluateStringFlagWithContext);
79+
test('Semantic version operator semantic comparison', evaluateStringFlagWithContext);
80+
81+
test('Time-based operations', ({ given, when, and, then }) => {
82+
let flagKey: string;
83+
let defaultValue: number;
84+
const evaluationContext: EvaluationContext = {};
85+
86+
aFlagProviderIsSet(given);
87+
88+
when(/^an integer flag with key "(.*)" is evaluated with default value (\d+)$/, (key, defaultVal) => {
89+
flagKey = key;
90+
defaultValue = defaultVal;
91+
});
92+
93+
and(/^a context containing a key "(.*)", with value (.*)$/, (key, value) => {
94+
evaluationContext[key] = value;
95+
});
96+
then(/^the returned value should be (.*)$/, async (expectedValue) => {
97+
const value = await client.getNumberValue(flagKey, defaultValue, evaluationContext);
98+
expect(value).toEqual(parseInt(expectedValue));
99+
});
100+
});
101+
102+
test('Targeting by targeting key', ({ given, when, and, then }) => {
103+
let flagKey: string;
104+
let defaultValue: string;
105+
let details: EvaluationDetails<string>;
106+
107+
aFlagProviderIsSet(given);
108+
109+
when(/^a string flag with key "(.*)" is evaluated with default value "(.*)"$/, (key, defaultVal) => {
110+
flagKey = key;
111+
defaultValue = defaultVal;
112+
});
113+
114+
and(/^a context containing a targeting key with value "(.*)"$/, async (targetingKeyValue) => {
115+
details = await client.getStringDetails(flagKey, defaultValue, { targetingKey: targetingKeyValue });
116+
});
117+
118+
then(/^the returned value should be "(.*)"$/, (expectedValue) => {
119+
expect(details.value).toEqual(expectedValue);
120+
});
121+
122+
then(/^the returned reason should be "(.*)"$/, (expectedReason) => {
123+
expect(details.reason).toEqual(expectedReason);
124+
});
125+
});
126+
127+
test('Errors and edge cases', ({ given, when, then }) => {
128+
let flagKey: string;
129+
let defaultValue: number;
130+
131+
aFlagProviderIsSet(given);
132+
133+
when(/^an integer flag with key "(.*)" is evaluated with default value (.*)$/, (key, defaultVal) => {
134+
flagKey = key;
135+
defaultValue = parseInt(defaultVal);
136+
});
137+
138+
then(/^the returned value should be (.*)$/, async (expectedValue) => {
139+
const value = await client.getNumberValue(flagKey, defaultValue);
140+
expect(value).toEqual(parseInt(expectedValue));
141+
});
142+
});
77143
});

‎libs/providers/flagd/src/e2e/step-definitions/flagd-reconnect.unstable.spec.ts‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
import { OpenFeature, ProviderEvents } from '@openfeature/server-sdk';
22
import { defineFeature, loadFeature } from 'jest-cucumber';
3+
import { UNAVAILABLE_CLIENT_NAME, UNSTABLE_CLIENT_NAME } from '../constants';
34

45
jest.setTimeout(30000);
56

67
// load the feature file.
78
const feature = loadFeature('features/flagd-reconnect.feature');
89

910
// get a client (flagd provider registered in setup)
10-
const client = OpenFeature.getClient('unstable');
11+
const client = OpenFeature.getClient(UNSTABLE_CLIENT_NAME);
1112

1213
defineFeature(feature, (test) => {
1314
let readyRunCount = 0;
@@ -46,4 +47,22 @@ defineFeature(feature, (test) => {
4647
expect(readyRunCount).toBeGreaterThan(1);
4748
});
4849
});
50+
51+
test('Provider unavailable', ({ given, when, then }) => {
52+
let errorHandlerRun = 0;
53+
54+
given('flagd is unavailable', async () => {
55+
// handled in setup
56+
});
57+
58+
when('a flagd provider is set and initialization is awaited', () => {
59+
OpenFeature.getClient(UNAVAILABLE_CLIENT_NAME).addHandler(ProviderEvents.Error, () => {
60+
errorHandlerRun++;
61+
});
62+
});
63+
64+
then('an error should be indicated within the configured deadline', () => {
65+
expect(errorHandlerRun).toBeGreaterThan(0);
66+
});
67+
});
4968
});

‎libs/providers/flagd/src/e2e/step-definitions/flagd.spec.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1-
import { OpenFeature, ProviderEvents, EventDetails } from '@openfeature/server-sdk';
1+
import { OpenFeature, ProviderEvents } from '@openfeature/server-sdk';
22
import { defineFeature, loadFeature } from 'jest-cucumber';
3+
import { E2E_CLIENT_NAME } from '../constants';
34

45
// load the feature file.
56
const feature = loadFeature('features/flagd.feature');
67

78
// get a client (flagd provider registered in setup)
8-
const client = OpenFeature.getClient('e2e');
9+
const client = OpenFeature.getClient(E2E_CLIENT_NAME);
910

1011
const aFlagProviderIsSet = (given: (stepMatcher: string, stepDefinitionCallback: () => void) => void) => {
1112
given('a flagd provider is set', () => undefined);

‎libs/providers/flagd/src/lib/service/in-process/grpc/grpc-fetch.ts‎

Lines changed: 56 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -70,39 +70,67 @@ export class GrpcFetch implements DataFetch {
7070
) {
7171
this._logger?.debug('Starting gRPC sync connection');
7272
closeStreamIfDefined(this._syncStream);
73-
this._syncStream = this._syncClient.syncFlags(this._request);
73+
try {
74+
this._syncStream = this._syncClient.syncFlags(this._request);
75+
this._syncStream.on('data', (data: SyncFlagsResponse) => {
76+
this._logger?.debug(`Received sync payload`);
7477

75-
this._syncStream.on('data', (data: SyncFlagsResponse) => {
76-
this._logger?.debug(`Received sync payload`);
78+
try {
79+
const changes = dataCallback(data.flagConfiguration);
80+
if (this._initialized && changes.length > 0) {
81+
changedCallback(changes);
82+
}
83+
} catch (err) {
84+
this._logger?.debug('Error processing sync payload: ', (err as Error)?.message ?? 'unknown error');
85+
}
7786

78-
try {
79-
const changes = dataCallback(data.flagConfiguration);
80-
if (this._initialized && changes.length > 0) {
81-
changedCallback(changes);
87+
if (resolveConnect) {
88+
resolveConnect();
89+
} else if (!this._isConnected) {
90+
// Not the first connection and there's no active connection.
91+
this._logger?.debug('Reconnected to gRPC sync');
92+
reconnectCallback();
8293
}
83-
} catch (err) {
84-
this._logger?.debug('Error processing sync payload: ', (err as Error)?.message ?? 'unknown error');
85-
}
94+
this._isConnected = true;
95+
});
8696

87-
if (resolveConnect) {
88-
resolveConnect();
89-
} else if (!this._isConnected) {
90-
// Not the first connection and there's no active connection.
91-
this._logger?.debug('Reconnected to gRPC sync');
92-
reconnectCallback();
93-
}
94-
this._isConnected = true;
95-
});
97+
this._syncStream.on('error', (err: ServiceError | undefined) => {
98+
this.handleError(
99+
err as Error,
100+
dataCallback,
101+
reconnectCallback,
102+
changedCallback,
103+
disconnectCallback,
104+
rejectConnect,
105+
);
106+
});
107+
} catch (err) {
108+
this.handleError(
109+
err as Error,
110+
dataCallback,
111+
reconnectCallback,
112+
changedCallback,
113+
disconnectCallback,
114+
rejectConnect,
115+
);
116+
}
117+
}
96118

97-
this._syncStream.on('error', (err: ServiceError | undefined) => {
98-
this._logger?.error('Connection error, attempting to reconnect');
99-
this._logger?.debug(err);
100-
this._isConnected = false;
101-
const errorMessage = err?.message ?? 'Failed to connect to syncFlags stream';
102-
disconnectCallback(errorMessage);
103-
rejectConnect?.(new GeneralError(errorMessage));
104-
this.reconnect(dataCallback, reconnectCallback, changedCallback, disconnectCallback);
105-
});
119+
private handleError(
120+
err: Error,
121+
dataCallback: (flags: string) => string[],
122+
reconnectCallback: () => void,
123+
changedCallback: (flagsChanged: string[]) => void,
124+
disconnectCallback: (message: string) => void,
125+
rejectConnect?: (reason: Error) => void,
126+
) {
127+
this._logger?.error('Connection error, attempting to reconnect');
128+
this._logger?.debug(err);
129+
this._isConnected = false;
130+
const errorMessage = err?.message ?? 'Failed to connect to syncFlags stream';
131+
disconnectCallback(errorMessage);
132+
rejectConnect?.(new GeneralError(errorMessage));
133+
this.reconnect(dataCallback, reconnectCallback, changedCallback, disconnectCallback);
106134
}
107135

108136
private reconnect(

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,4 +225,12 @@ describe('flagd-core common flag definitions', () => {
225225
expect(resolved.reason).toBe(StandardResolutionReasons.STATIC);
226226
expect(resolved.variant).toBe('false');
227227
});
228+
229+
it('should throw with invalid targeting rules', () => {
230+
const core = new FlagdCore();
231+
const flagCfg = `{"flags":{"isEnabled":{"state":"ENABLED","variants":{"true":true,"false":false},"defaultVariant":"false","targeting":{"invalid": ["this is not valid targeting"]}}}}`;
232+
core.setConfigurations(flagCfg);
233+
234+
expect(() => core.resolveBooleanEvaluation('isEnabled', false, {}, console)).toThrow();
235+
});
228236
});

0 commit comments

Comments
 (0)