Skip to content

Commit 0425619

Browse files
adams85adams85
authored
fix(config-cat): Rework error reporting (#1242)
1 parent a164bca commit 0425619

File tree

4 files changed

+135
-72
lines changed

4 files changed

+135
-72
lines changed

‎libs/providers/config-cat-web/src/lib/config-cat-web-provider.spec.ts

+36-18
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {
33
createConsoleLogger,
44
createFlagOverridesFromMap,
55
HookEvents,
6+
IConfigCatCache,
67
ISettingUnion,
78
LogLevel,
89
OverrideBehaviour,
@@ -81,30 +82,47 @@ describe('ConfigCatWebProvider', () => {
8182
});
8283
});
8384

84-
it('should emit PROVIDER_ERROR event', () => {
85-
const handler = jest.fn();
86-
const eventData: [string, unknown] = ['error', { error: 'error' }];
85+
it("should emit PROVIDER_READY event when underlying client is initialized after provider's initialize", async () => {
86+
const cacheValue = '253370761200000\nW/"12345678-90a"\n{"f":{"booleanTrue":{"t":0,"v":{"b":true}}}}';
87+
88+
const fakeSharedCache = new (class implements IConfigCatCache {
89+
private _value?: string;
90+
get(key: string) {
91+
return this._value;
92+
}
93+
set(key: string, value: string) {
94+
this._value = value;
95+
}
96+
})();
97+
98+
const provider = ConfigCatWebProvider.create('configcat-sdk-1/1234567890123456789012/1234567890123456789012', {
99+
cache: fakeSharedCache,
100+
logger: createConsoleLogger(LogLevel.Off),
101+
offline: true,
102+
maxInitWaitTimeSeconds: 1,
103+
});
87104

88-
provider.events.addHandler(ProviderEvents.Error, handler);
89-
configCatEmitter.emit('clientError', ...eventData);
105+
const readyHandler = jest.fn();
106+
provider.events.addHandler(ProviderEvents.Ready, readyHandler);
90107

91-
expect(handler).toHaveBeenCalledWith({
92-
message: eventData[0],
93-
metadata: eventData[1],
94-
});
95-
});
108+
try {
109+
await provider.initialize();
110+
} catch (err) {
111+
expect((err as Error).message).toContain('underlying ConfigCat client could not initialize');
112+
}
96113

97-
it('should emit PROVIDER_READY event after successful evaluation during ERROR condition', async () => {
98-
const errorHandler = jest.fn();
99-
provider.events.addHandler(ProviderEvents.Error, errorHandler);
114+
expect(readyHandler).toHaveBeenCalledTimes(0);
100115

101-
configCatEmitter.emit('clientError', 'error', { error: 'error' });
102-
expect(errorHandler).toHaveBeenCalled();
116+
fakeSharedCache.set('', cacheValue);
103117

104-
const readyHandler = jest.fn();
105-
provider.events.addHandler(ProviderEvents.Ready, readyHandler);
118+
// Make sure that the internal cache is refreshed.
119+
await provider.configCatClient?.forceRefreshAsync();
120+
121+
provider.resolveBooleanEvaluation('booleanTrue', false, { targetingKey });
122+
123+
// Wait a little while for the Ready event to be emitted.
124+
await new Promise((resolve) => setTimeout(resolve, 100));
106125

107-
await provider.resolveBooleanEvaluation('booleanTrue', false, { targetingKey });
108126
expect(readyHandler).toHaveBeenCalled();
109127
});
110128
});

‎libs/providers/config-cat-web/src/lib/config-cat-web-provider.ts

+31-18
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
transformContext,
2020
} from '@openfeature/config-cat-core';
2121
import {
22+
ClientCacheState,
2223
getClient,
2324
IConfig,
2425
IConfigCatClient,
@@ -30,7 +31,7 @@ import {
3031
export class ConfigCatWebProvider implements Provider {
3132
public readonly events = new OpenFeatureEventEmitter();
3233
private readonly _clientFactory: (provider: ConfigCatWebProvider) => IConfigCatClient;
33-
private _hasError = false;
34+
private _isProviderReady = false;
3435
private _client?: IConfigCatClient;
3536

3637
public runsOn: Paradigm = 'client';
@@ -53,19 +54,11 @@ export class ConfigCatWebProvider implements Provider {
5354
options.setupHooks = (hooks) => {
5455
oldSetupHooks?.(hooks);
5556

56-
hooks.on('configChanged', (projectConfig: IConfig | undefined) =>
57+
hooks.on('configChanged', (config: IConfig) =>
5758
provider.events.emit(ProviderEvents.ConfigurationChanged, {
58-
flagsChanged: projectConfig ? Object.keys(projectConfig.settings) : undefined,
59+
flagsChanged: Object.keys(config.settings),
5960
}),
6061
);
61-
62-
hooks.on('clientError', (message: string, error) => {
63-
provider._hasError = true;
64-
provider.events.emit(ProviderEvents.Error, {
65-
message: message,
66-
metadata: error,
67-
});
68-
});
6962
};
7063

7164
return getClient(sdkKey, PollingMode.AutoPoll, options);
@@ -74,8 +67,19 @@ export class ConfigCatWebProvider implements Provider {
7467

7568
public async initialize(): Promise<void> {
7669
const client = this._clientFactory(this);
77-
await client.waitForReady();
70+
const clientCacheState = await client.waitForReady();
7871
this._client = client;
72+
73+
if (clientCacheState !== ClientCacheState.NoFlagData) {
74+
this._isProviderReady = true;
75+
} else {
76+
// OpenFeature provider defines ready state like this: "The provider is ready to resolve flags."
77+
// However, ConfigCat client's behavior is different: in some cases ready state may be reached
78+
// even if the client's internal, in-memory cache hasn't been populated yet, that is,
79+
// the client is not able to evaluate feature flags yet. In such cases we throw an error to
80+
// prevent the provider from being set ready right away, and check for the ready state later.
81+
throw Error('The underlying ConfigCat client could not initialize within maxInitWaitTimeSeconds.');
82+
}
7983
}
8084

8185
public get configCatClient() {
@@ -137,13 +141,22 @@ export class ConfigCatWebProvider implements Provider {
137141

138142
const configCatDefaultValue = flagType !== 'object' ? (defaultValue as SettingValue) : JSON.stringify(defaultValue);
139143

140-
const { value, ...evaluationData } = this._client
141-
.snapshot()
142-
.getValueDetails(flagKey, configCatDefaultValue, transformContext(context));
144+
const snapshot = this._client.snapshot();
145+
146+
const { value, ...evaluationData } = snapshot.getValueDetails(
147+
flagKey,
148+
configCatDefaultValue,
149+
transformContext(context),
150+
);
151+
152+
if (!this._isProviderReady && snapshot.cacheState !== ClientCacheState.NoFlagData) {
153+
// Ideally, we would check ConfigCat client's initialization state in its "background" polling loop.
154+
// This is not possible at the moment, so as a workaround, we do the check on feature flag evaluation.
155+
// There are plans to improve this situation, so let's revise this
156+
// as soon as ConfigCat SDK implements the necessary event.
143157

144-
if (this._hasError && !evaluationData.errorMessage && !evaluationData.errorException) {
145-
this._hasError = false;
146-
this.events.emit(ProviderEvents.Ready);
158+
this._isProviderReady = true;
159+
setTimeout(() => this.events.emit(ProviderEvents.Ready), 0);
147160
}
148161

149162
if (evaluationData.isDefaultValue) {

‎libs/providers/config-cat/src/lib/config-cat-provider.spec.ts

+40-18
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
createConsoleLogger,
55
createFlagOverridesFromMap,
66
HookEvents,
7+
IConfigCatCache,
78
ISettingUnion,
89
LogLevel,
910
OverrideBehaviour,
@@ -82,30 +83,51 @@ describe('ConfigCatProvider', () => {
8283
});
8384
});
8485

85-
it('should emit PROVIDER_ERROR event', () => {
86-
const handler = jest.fn();
87-
const eventData: [string, unknown] = ['error', { error: 'error' }];
86+
it("should emit PROVIDER_READY event when underlying client is initialized after provider's initialize", async () => {
87+
const cacheValue = '253370761200000\nW/"12345678-90a"\n{"f":{"booleanTrue":{"t":0,"v":{"b":true}}}}';
88+
89+
const fakeSharedCache = new (class implements IConfigCatCache {
90+
private _value?: string;
91+
get(key: string) {
92+
return this._value;
93+
}
94+
set(key: string, value: string) {
95+
this._value = value;
96+
}
97+
})();
98+
99+
const provider = ConfigCatProvider.create(
100+
'configcat-sdk-1/1234567890123456789012/1234567890123456789012',
101+
PollingMode.AutoPoll,
102+
{
103+
cache: fakeSharedCache,
104+
logger: createConsoleLogger(LogLevel.Off),
105+
offline: true,
106+
maxInitWaitTimeSeconds: 1,
107+
},
108+
);
88109

89-
provider.events.addHandler(ProviderEvents.Error, handler);
90-
configCatEmitter.emit('clientError', ...eventData);
110+
const readyHandler = jest.fn();
111+
provider.events.addHandler(ProviderEvents.Ready, readyHandler);
91112

92-
expect(handler).toHaveBeenCalledWith({
93-
message: eventData[0],
94-
metadata: eventData[1],
95-
});
96-
});
113+
try {
114+
await provider.initialize();
115+
} catch (err) {
116+
expect((err as Error).message).toContain('underlying ConfigCat client could not initialize');
117+
}
97118

98-
it('should emit PROVIDER_READY event after successful evaluation during ERROR condition', async () => {
99-
const errorHandler = jest.fn();
100-
provider.events.addHandler(ProviderEvents.Error, errorHandler);
119+
expect(readyHandler).toHaveBeenCalledTimes(0);
101120

102-
configCatEmitter.emit('clientError', 'error', { error: 'error' });
103-
expect(errorHandler).toHaveBeenCalled();
121+
fakeSharedCache.set('', cacheValue);
104122

105-
const readyHandler = jest.fn();
106-
provider.events.addHandler(ProviderEvents.Ready, readyHandler);
123+
// Make sure that the internal cache is refreshed.
124+
await provider.configCatClient?.forceRefreshAsync();
125+
126+
provider.resolveBooleanEvaluation('booleanTrue', false, { targetingKey });
127+
128+
// Wait a little while for the Ready event to be emitted.
129+
await new Promise((resolve) => setTimeout(resolve, 100));
107130

108-
await provider.resolveBooleanEvaluation('booleanTrue', false, { targetingKey });
109131
expect(readyHandler).toHaveBeenCalled();
110132
});
111133
});

‎libs/providers/config-cat/src/lib/config-cat-provider.ts

+28-18
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,14 @@ import {
1818
toResolutionDetails,
1919
transformContext,
2020
} from '@openfeature/config-cat-core';
21-
import { PollingMode, SettingValue } from 'configcat-common';
21+
import { ClientCacheState, PollingMode, SettingValue } from 'configcat-common';
2222
import { IConfigCatClient, getClient, IConfig, OptionsForPollingMode } from 'configcat-node';
2323

2424
export class ConfigCatProvider implements Provider {
2525
public readonly events = new OpenFeatureEventEmitter();
2626
private readonly _clientFactory: (provider: ConfigCatProvider) => IConfigCatClient;
27-
private _hasError = false;
27+
private readonly _pollingMode: PollingMode;
28+
private _isProviderReady = false;
2829
private _client?: IConfigCatClient;
2930

3031
public runsOn: Paradigm = 'server';
@@ -33,8 +34,9 @@ export class ConfigCatProvider implements Provider {
3334
name: ConfigCatProvider.name,
3435
};
3536

36-
protected constructor(clientFactory: (provider: ConfigCatProvider) => IConfigCatClient) {
37+
protected constructor(clientFactory: (provider: ConfigCatProvider) => IConfigCatClient, pollingMode: PollingMode) {
3738
this._clientFactory = clientFactory;
39+
this._pollingMode = pollingMode;
3840
}
3941

4042
public static create<TMode extends PollingMode>(
@@ -50,29 +52,32 @@ export class ConfigCatProvider implements Provider {
5052
options.setupHooks = (hooks) => {
5153
oldSetupHooks?.(hooks);
5254

53-
hooks.on('configChanged', (projectConfig: IConfig | undefined) =>
55+
hooks.on('configChanged', (config: IConfig) =>
5456
provider.events.emit(ProviderEvents.ConfigurationChanged, {
55-
flagsChanged: projectConfig ? Object.keys(projectConfig.settings) : undefined,
57+
flagsChanged: Object.keys(config.settings),
5658
}),
5759
);
58-
59-
hooks.on('clientError', (message: string, error) => {
60-
provider._hasError = true;
61-
provider.events.emit(ProviderEvents.Error, {
62-
message: message,
63-
metadata: error,
64-
});
65-
});
6660
};
6761

6862
return getClient(sdkKey, pollingMode, options);
69-
});
63+
}, pollingMode ?? PollingMode.AutoPoll);
7064
}
7165

7266
public async initialize(): Promise<void> {
7367
const client = this._clientFactory(this);
74-
await client.waitForReady();
68+
const clientCacheState = await client.waitForReady();
7569
this._client = client;
70+
71+
if (this._pollingMode !== PollingMode.AutoPoll || clientCacheState !== ClientCacheState.NoFlagData) {
72+
this._isProviderReady = true;
73+
} else {
74+
// OpenFeature provider defines ready state like this: "The provider is ready to resolve flags."
75+
// However, ConfigCat client's behavior is different: in some cases ready state may be reached
76+
// even if the client's internal, in-memory cache hasn't been populated yet, that is,
77+
// the client is not able to evaluate feature flags yet. In such cases we throw an error to
78+
// prevent the provider from being set ready right away, and check for the ready state later.
79+
throw Error('The underlying ConfigCat client could not initialize within maxInitWaitTimeSeconds.');
80+
}
7681
}
7782

7883
public get configCatClient() {
@@ -140,9 +145,14 @@ export class ConfigCatProvider implements Provider {
140145
transformContext(context),
141146
);
142147

143-
if (this._hasError && !evaluationData.errorMessage && !evaluationData.errorException) {
144-
this._hasError = false;
145-
this.events.emit(ProviderEvents.Ready);
148+
if (!this._isProviderReady && this._client.snapshot().cacheState !== ClientCacheState.NoFlagData) {
149+
// Ideally, we would check ConfigCat client's initialization state in its "background" polling loop.
150+
// This is not possible at the moment, so as a workaround, we do the check on feature flag evaluation.
151+
// There are plans to improve this situation, so let's revise this
152+
// as soon as ConfigCat SDK implements the necessary event.
153+
154+
this._isProviderReady = true;
155+
setTimeout(() => this.events.emit(ProviderEvents.Ready), 0);
146156
}
147157

148158
if (evaluationData.isDefaultValue) {

0 commit comments

Comments
 (0)