Skip to content

Commit 0be9c5d

Browse files
toddbaerttoddbaert
authored
fix: race adding handler during init (#501)
Fixes a race condition that prevents handlers added while their provider is initializing from running. Signed-off-by: Todd Baert <[email protected]>
1 parent 98265e5 commit 0be9c5d

File tree

5 files changed

+57
-10
lines changed

5 files changed

+57
-10
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ export class OpenFeatureAPI extends OpenFeatureCommonAPI<Provider> implements Ma
6060
// functions are passed here to make sure that these values are always up to date,
6161
// and so we don't have to make these public properties on the API class.
6262
() => this.getProviderForClient(name),
63-
() => this.getAndCacheEventEmitterForClient(name),
63+
() => this.buildAndCacheEventEmitterForClient(name),
6464
() => this._logger,
6565
{ name, version }
6666
);

‎packages/client/test/events.spec.ts‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,24 +12,29 @@ import {
1212
} from '../src';
1313
import { v4 as uuid } from 'uuid';
1414

15+
const TIMEOUT = 1000;
16+
1517
class MockProvider implements Provider {
1618
readonly metadata: ProviderMetadata;
1719
readonly events?: OpenFeatureEventEmitter;
1820
private hasInitialize: boolean;
1921
private failOnInit: boolean;
22+
private initDelay?: number;
2023
private enableEvents: boolean;
2124
status?: ProviderStatus = undefined;
2225

2326
constructor(options?: {
2427
hasInitialize?: boolean;
2528
initialStatus?: ProviderStatus;
29+
initDelay?: number;
2630
enableEvents?: boolean;
2731
failOnInit?: boolean;
2832
name?: string;
2933
}) {
3034
this.metadata = { name: options?.name ?? 'mock-provider' };
3135
this.hasInitialize = options?.hasInitialize ?? true;
3236
this.status = options?.initialStatus ?? ProviderStatus.NOT_READY;
37+
this.initDelay = options?.initDelay ?? 0;
3338
this.enableEvents = options?.enableEvents ?? true;
3439
this.failOnInit = options?.failOnInit ?? false;
3540

@@ -39,6 +44,7 @@ class MockProvider implements Provider {
3944

4045
if (this.hasInitialize) {
4146
this.initialize = jest.fn(async () => {
47+
await new Promise((resolve) => setTimeout(resolve, this.initDelay));
4248
if (this.failOnInit) {
4349
throw new Error('Provider initialization failed');
4450
}
@@ -69,13 +75,15 @@ class MockProvider implements Provider {
6975

7076
describe('Events', () => {
7177
// set timeouts short for this suite.
72-
jest.setTimeout(1000);
78+
jest.setTimeout(TIMEOUT);
7379
let clientId = uuid();
7480

7581
afterEach(() => {
7682
jest.clearAllMocks();
7783
clientId = uuid();
78-
});
84+
// hacky, but it's helpful to clear the handlers between tests
85+
(OpenFeature as any)._clientEventHandlers = new Map();
86+
(OpenFeature as any)._clientEvents = new Map(); });
7987

8088
beforeEach(() => {
8189
OpenFeature.setProvider(NOOP_PROVIDER);
@@ -296,6 +304,19 @@ describe('Events', () => {
296304
defaultProvider.events?.emit(ProviderEvents.ConfigurationChanged);
297305
});
298306

307+
it('handler added while while provider initializing runs', (done) => {
308+
const provider = new MockProvider({ name: 'race', initialStatus: ProviderStatus.NOT_READY, initDelay: TIMEOUT / 2 });
309+
310+
// set the default provider
311+
OpenFeature.setProvider(provider);
312+
const client = OpenFeature.getClient();
313+
314+
// add a handler while the provider is starting
315+
client.addHandler(ProviderEvents.Ready, () => {
316+
done();
317+
});
318+
});
319+
299320
it('PROVIDER_ERROR events populates the message field', (done) => {
300321
const provider = new MockProvider({ failOnInit: true });
301322
const client = OpenFeature.getClient(clientId);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ export class OpenFeatureAPI extends OpenFeatureCommonAPI<Provider> implements Ma
9898

9999
return new OpenFeatureClient(
100100
() => this.getProviderForClient(name),
101-
() => this.getAndCacheEventEmitterForClient(name),
101+
() => this.buildAndCacheEventEmitterForClient(name),
102102
() => this._logger,
103103
{ name, version },
104104
context

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

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,24 +12,29 @@ import {
1212
} from '../src';
1313
import { v4 as uuid } from 'uuid';
1414

15+
const TIMEOUT = 1000;
16+
1517
class MockProvider implements Provider {
1618
readonly metadata: ProviderMetadata;
1719
readonly events?: OpenFeatureEventEmitter;
1820
private hasInitialize: boolean;
1921
private failOnInit: boolean;
22+
private initDelay?: number;
2023
private enableEvents: boolean;
2124
status?: ProviderStatus = undefined;
2225

2326
constructor(options?: {
2427
hasInitialize?: boolean;
2528
initialStatus?: ProviderStatus;
29+
initDelay?: number;
2630
enableEvents?: boolean;
2731
failOnInit?: boolean;
2832
name?: string;
2933
}) {
3034
this.metadata = { name: options?.name ?? 'mock-provider' };
3135
this.hasInitialize = options?.hasInitialize ?? true;
3236
this.status = options?.initialStatus ?? ProviderStatus.NOT_READY;
37+
this.initDelay = options?.initDelay ?? 0;
3338
this.enableEvents = options?.enableEvents ?? true;
3439
this.failOnInit = options?.failOnInit ?? false;
3540

@@ -39,6 +44,7 @@ class MockProvider implements Provider {
3944

4045
if (this.hasInitialize) {
4146
this.initialize = jest.fn(async () => {
47+
await new Promise((resolve) => setTimeout(resolve, this.initDelay));
4248
if (this.failOnInit) {
4349
throw new Error('Provider initialization failed');
4450
}
@@ -72,12 +78,15 @@ class MockProvider implements Provider {
7278

7379
describe('Events', () => {
7480
// set timeouts short for this suite.
75-
jest.setTimeout(1000);
81+
jest.setTimeout(TIMEOUT);
7682
let clientId = uuid();
7783

7884
afterEach(() => {
7985
jest.clearAllMocks();
8086
clientId = uuid();
87+
// hacky, but it's helpful to clear the handlers between tests
88+
(OpenFeature as any)._clientEventHandlers = new Map();
89+
(OpenFeature as any)._clientEvents = new Map();
8190
});
8291

8392
beforeEach(() => {
@@ -299,6 +308,19 @@ describe('Events', () => {
299308
defaultProvider.events?.emit(ProviderEvents.ConfigurationChanged);
300309
});
301310

311+
it('handler added while while provider initializing runs', (done) => {
312+
const provider = new MockProvider({ name: 'race', initialStatus: ProviderStatus.NOT_READY, initDelay: TIMEOUT / 2 });
313+
314+
// set the default provider
315+
OpenFeature.setProvider(provider);
316+
const client = OpenFeature.getClient();
317+
318+
// add a handler while the provider is starting
319+
client.addHandler(ProviderEvents.Ready, () => {
320+
done();
321+
});
322+
});
323+
302324
it('PROVIDER_ERROR events populates the message field', (done) => {
303325
const provider = new MockProvider({ failOnInit: true });
304326
const client = OpenFeature.getClient(clientId);

‎packages/shared/src/open-feature.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -122,20 +122,20 @@ export abstract class OpenFeatureCommonAPI<P extends CommonProvider = CommonProv
122122
return this;
123123
}
124124

125-
// get the named emitter, or if this is the default provider, get all event emitters not associated with a provider
126-
const emitters = clientName ? [this.getAndCacheEventEmitterForClient(clientName)] : this.getUnboundEmitters();
125+
const emitters = this.getAssociatedEventEmitters(clientName);
127126

128127
if (typeof provider.initialize === 'function') {
129128
provider
130129
.initialize?.(this._context)
131130
?.then(() => {
132-
emitters.forEach((emitter) => {
131+
// fetch the most recent event emitters, some may have been added during init
132+
this.getAssociatedEventEmitters(clientName).forEach((emitter) => {
133133
emitter?.emit(ProviderEvents.Ready, { clientName });
134134
});
135135
this._events?.emit(ProviderEvents.Ready, { clientName });
136136
})
137137
?.catch((error) => {
138-
emitters.forEach((emitter) => {
138+
this.getAssociatedEventEmitters(clientName).forEach((emitter) => {
139139
emitter?.emit(ProviderEvents.Error, { clientName, message: error.message });
140140
});
141141
this._events?.emit(ProviderEvents.Error, { clientName, message: error.message });
@@ -171,7 +171,7 @@ export abstract class OpenFeatureCommonAPI<P extends CommonProvider = CommonProv
171171
return this._clientProviders.get(name) ?? this._defaultProvider;
172172
}
173173

174-
protected getAndCacheEventEmitterForClient(name?: string): InternalEventEmitter {
174+
protected buildAndCacheEventEmitterForClient(name?: string): InternalEventEmitter {
175175
const emitter = this._clientEvents.get(name);
176176

177177
if (emitter) {
@@ -204,6 +204,10 @@ export abstract class OpenFeatureCommonAPI<P extends CommonProvider = CommonProv
204204
].filter(isDefined);
205205
}
206206

207+
private getAssociatedEventEmitters(clientName: string | undefined) {
208+
return clientName ? [this.buildAndCacheEventEmitterForClient(clientName)] : this.getUnboundEmitters();
209+
}
210+
207211
private transferListeners(
208212
oldProvider: P,
209213
newProvider: P,

0 commit comments

Comments
 (0)