Skip to content

fix(config-cat): Rework error reporting #1242

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 9, 2025
Merged

fix(config-cat): Rework error reporting #1242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
de83140
Attempt at fixing https://github.com/open-feature/js-sdk-contrib/issu…
Mar 26, 2025
c56ba6e
Adjust tests
Mar 27, 2025
e8da972
Merge branch 'main' into fix/issue1212
lukas-reining Apr 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 36 additions & 18 deletions libs/providers/config-cat-web/src/lib/config-cat-web-provider.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
createConsoleLogger,
createFlagOverridesFromMap,
HookEvents,
IConfigCatCache,
ISettingUnion,
LogLevel,
OverrideBehaviour,
Expand Down Expand Up @@ -81,30 +82,47 @@ describe('ConfigCatWebProvider', () => {
});
});

it('should emit PROVIDER_ERROR event', () => {
const handler = jest.fn();
const eventData: [string, unknown] = ['error', { error: 'error' }];
it("should emit PROVIDER_READY event when underlying client is initialized after provider's initialize", async () => {
const cacheValue = '253370761200000\nW/"12345678-90a"\n{"f":{"booleanTrue":{"t":0,"v":{"b":true}}}}';

const fakeSharedCache = new (class implements IConfigCatCache {
private _value?: string;
get(key: string) {
return this._value;
}
set(key: string, value: string) {
this._value = value;
}
})();

const provider = ConfigCatWebProvider.create('configcat-sdk-1/1234567890123456789012/1234567890123456789012', {
cache: fakeSharedCache,
logger: createConsoleLogger(LogLevel.Off),
offline: true,
maxInitWaitTimeSeconds: 1,
});

provider.events.addHandler(ProviderEvents.Error, handler);
configCatEmitter.emit('clientError', ...eventData);
const readyHandler = jest.fn();
provider.events.addHandler(ProviderEvents.Ready, readyHandler);

expect(handler).toHaveBeenCalledWith({
message: eventData[0],
metadata: eventData[1],
});
});
try {
await provider.initialize();
} catch (err) {
expect((err as Error).message).toContain('underlying ConfigCat client could not initialize');
}

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

configCatEmitter.emit('clientError', 'error', { error: 'error' });
expect(errorHandler).toHaveBeenCalled();
fakeSharedCache.set('', cacheValue);

const readyHandler = jest.fn();
provider.events.addHandler(ProviderEvents.Ready, readyHandler);
// Make sure that the internal cache is refreshed.
await provider.configCatClient?.forceRefreshAsync();

provider.resolveBooleanEvaluation('booleanTrue', false, { targetingKey });

// Wait a little while for the Ready event to be emitted.
await new Promise((resolve) => setTimeout(resolve, 100));

await provider.resolveBooleanEvaluation('booleanTrue', false, { targetingKey });
expect(readyHandler).toHaveBeenCalled();
});
});
Expand Down
49 changes: 31 additions & 18 deletions libs/providers/config-cat-web/src/lib/config-cat-web-provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
transformContext,
} from '@openfeature/config-cat-core';
import {
ClientCacheState,
getClient,
IConfig,
IConfigCatClient,
Expand All @@ -30,7 +31,7 @@ import {
export class ConfigCatWebProvider implements Provider {
public readonly events = new OpenFeatureEventEmitter();
private readonly _clientFactory: (provider: ConfigCatWebProvider) => IConfigCatClient;
private _hasError = false;
private _isProviderReady = false;
private _client?: IConfigCatClient;

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

hooks.on('configChanged', (projectConfig: IConfig | undefined) =>
hooks.on('configChanged', (config: IConfig) =>
provider.events.emit(ProviderEvents.ConfigurationChanged, {
flagsChanged: projectConfig ? Object.keys(projectConfig.settings) : undefined,
flagsChanged: Object.keys(config.settings),
}),
);

hooks.on('clientError', (message: string, error) => {
provider._hasError = true;
provider.events.emit(ProviderEvents.Error, {
message: message,
metadata: error,
});
});
};

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

public async initialize(): Promise<void> {
const client = this._clientFactory(this);
await client.waitForReady();
const clientCacheState = await client.waitForReady();
this._client = client;

if (clientCacheState !== ClientCacheState.NoFlagData) {
this._isProviderReady = true;
} else {
// OpenFeature provider defines ready state like this: "The provider is ready to resolve flags."
// However, ConfigCat client's behavior is different: in some cases ready state may be reached
// even if the client's internal, in-memory cache hasn't been populated yet, that is,
// the client is not able to evaluate feature flags yet. In such cases we throw an error to
// prevent the provider from being set ready right away, and check for the ready state later.
throw Error('The underlying ConfigCat client could not initialize within maxInitWaitTimeSeconds.');
}
}

public get configCatClient() {
Expand Down Expand Up @@ -137,13 +141,22 @@ export class ConfigCatWebProvider implements Provider {

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

const { value, ...evaluationData } = this._client
.snapshot()
.getValueDetails(flagKey, configCatDefaultValue, transformContext(context));
const snapshot = this._client.snapshot();

const { value, ...evaluationData } = snapshot.getValueDetails(
flagKey,
configCatDefaultValue,
transformContext(context),
);

if (!this._isProviderReady && snapshot.cacheState !== ClientCacheState.NoFlagData) {
// Ideally, we would check ConfigCat client's initialization state in its "background" polling loop.
// This is not possible at the moment, so as a workaround, we do the check on feature flag evaluation.
// There are plans to improve this situation, so let's revise this
// as soon as ConfigCat SDK implements the necessary event.

if (this._hasError && !evaluationData.errorMessage && !evaluationData.errorException) {
this._hasError = false;
this.events.emit(ProviderEvents.Ready);
this._isProviderReady = true;
setTimeout(() => this.events.emit(ProviderEvents.Ready), 0);
}

if (evaluationData.isDefaultValue) {
Expand Down
58 changes: 40 additions & 18 deletions libs/providers/config-cat/src/lib/config-cat-provider.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
createConsoleLogger,
createFlagOverridesFromMap,
HookEvents,
IConfigCatCache,
ISettingUnion,
LogLevel,
OverrideBehaviour,
Expand Down Expand Up @@ -82,30 +83,51 @@ describe('ConfigCatProvider', () => {
});
});

it('should emit PROVIDER_ERROR event', () => {
const handler = jest.fn();
const eventData: [string, unknown] = ['error', { error: 'error' }];
it("should emit PROVIDER_READY event when underlying client is initialized after provider's initialize", async () => {
const cacheValue = '253370761200000\nW/"12345678-90a"\n{"f":{"booleanTrue":{"t":0,"v":{"b":true}}}}';

const fakeSharedCache = new (class implements IConfigCatCache {
private _value?: string;
get(key: string) {
return this._value;
}
set(key: string, value: string) {
this._value = value;
}
})();

const provider = ConfigCatProvider.create(
'configcat-sdk-1/1234567890123456789012/1234567890123456789012',
PollingMode.AutoPoll,
{
cache: fakeSharedCache,
logger: createConsoleLogger(LogLevel.Off),
offline: true,
maxInitWaitTimeSeconds: 1,
},
);

provider.events.addHandler(ProviderEvents.Error, handler);
configCatEmitter.emit('clientError', ...eventData);
const readyHandler = jest.fn();
provider.events.addHandler(ProviderEvents.Ready, readyHandler);

expect(handler).toHaveBeenCalledWith({
message: eventData[0],
metadata: eventData[1],
});
});
try {
await provider.initialize();
} catch (err) {
expect((err as Error).message).toContain('underlying ConfigCat client could not initialize');
}

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

configCatEmitter.emit('clientError', 'error', { error: 'error' });
expect(errorHandler).toHaveBeenCalled();
fakeSharedCache.set('', cacheValue);

const readyHandler = jest.fn();
provider.events.addHandler(ProviderEvents.Ready, readyHandler);
// Make sure that the internal cache is refreshed.
await provider.configCatClient?.forceRefreshAsync();

provider.resolveBooleanEvaluation('booleanTrue', false, { targetingKey });

// Wait a little while for the Ready event to be emitted.
await new Promise((resolve) => setTimeout(resolve, 100));

await provider.resolveBooleanEvaluation('booleanTrue', false, { targetingKey });
expect(readyHandler).toHaveBeenCalled();
});
});
Expand Down
46 changes: 28 additions & 18 deletions libs/providers/config-cat/src/lib/config-cat-provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@ import {
toResolutionDetails,
transformContext,
} from '@openfeature/config-cat-core';
import { PollingMode, SettingValue } from 'configcat-common';
import { ClientCacheState, PollingMode, SettingValue } from 'configcat-common';
import { IConfigCatClient, getClient, IConfig, OptionsForPollingMode } from 'configcat-node';

export class ConfigCatProvider implements Provider {
public readonly events = new OpenFeatureEventEmitter();
private readonly _clientFactory: (provider: ConfigCatProvider) => IConfigCatClient;
private _hasError = false;
private readonly _pollingMode: PollingMode;
private _isProviderReady = false;
private _client?: IConfigCatClient;

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

protected constructor(clientFactory: (provider: ConfigCatProvider) => IConfigCatClient) {
protected constructor(clientFactory: (provider: ConfigCatProvider) => IConfigCatClient, pollingMode: PollingMode) {
this._clientFactory = clientFactory;
this._pollingMode = pollingMode;
}

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

hooks.on('configChanged', (projectConfig: IConfig | undefined) =>
hooks.on('configChanged', (config: IConfig) =>
provider.events.emit(ProviderEvents.ConfigurationChanged, {
flagsChanged: projectConfig ? Object.keys(projectConfig.settings) : undefined,
flagsChanged: Object.keys(config.settings),
}),
);

hooks.on('clientError', (message: string, error) => {
provider._hasError = true;
provider.events.emit(ProviderEvents.Error, {
message: message,
metadata: error,
});
});
};

return getClient(sdkKey, pollingMode, options);
});
}, pollingMode ?? PollingMode.AutoPoll);
}

public async initialize(): Promise<void> {
const client = this._clientFactory(this);
await client.waitForReady();
const clientCacheState = await client.waitForReady();
this._client = client;

if (this._pollingMode !== PollingMode.AutoPoll || clientCacheState !== ClientCacheState.NoFlagData) {
this._isProviderReady = true;
} else {
// OpenFeature provider defines ready state like this: "The provider is ready to resolve flags."
// However, ConfigCat client's behavior is different: in some cases ready state may be reached
// even if the client's internal, in-memory cache hasn't been populated yet, that is,
// the client is not able to evaluate feature flags yet. In such cases we throw an error to
// prevent the provider from being set ready right away, and check for the ready state later.
throw Error('The underlying ConfigCat client could not initialize within maxInitWaitTimeSeconds.');
}
}

public get configCatClient() {
Expand Down Expand Up @@ -140,9 +145,14 @@ export class ConfigCatProvider implements Provider {
transformContext(context),
);

if (this._hasError && !evaluationData.errorMessage && !evaluationData.errorException) {
this._hasError = false;
this.events.emit(ProviderEvents.Ready);
if (!this._isProviderReady && this._client.snapshot().cacheState !== ClientCacheState.NoFlagData) {
// Ideally, we would check ConfigCat client's initialization state in its "background" polling loop.
// This is not possible at the moment, so as a workaround, we do the check on feature flag evaluation.
// There are plans to improve this situation, so let's revise this
// as soon as ConfigCat SDK implements the necessary event.

this._isProviderReady = true;
setTimeout(() => this.events.emit(ProviderEvents.Ready), 0);
}

if (evaluationData.isDefaultValue) {
Expand Down