Skip to content

fix(go-feature-flag-web): avoid infinite loop in waitWebsocketFinalSt… #1104

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
thomaspoignant merged 3 commits into from
Dec 13, 2024
Merged

fix(go-feature-flag-web): avoid infinite loop in waitWebsocketFinalSt… #1104

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
2d24c52
fix(go-feature-flag-web): avoid infinite loop in waitWebsocketFinalSt…
thomaspoignant Dec 5, 2024
2c37109
refactor waitWebsocketFinalStatus to be timeout based
thomaspoignant Dec 6, 2024
a1c52c2
Merge branch 'main' into fix-goff-2753
thomaspoignant Dec 9, 2024
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
139 changes: 93 additions & 46 deletions libs/providers/go-feature-flag-web/src/lib/go-feature-flag-web-provider.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,11 @@ describe('GoFeatureFlagWebProvider', () => {
const logger = new TestLogger();

beforeEach(async () => {
await WS.clean();
WS.clean();
await OpenFeature.close();
fetchMock.mockClear();
fetchMock.mockReset();
await jest.resetAllMocks();
jest.resetAllMocks();
websocketMockServer = new WS(websocketEndpoint, { jsonProtocol: true });
fetchMock.post(allFlagsEndpoint, defaultAllFlagResponse);
fetchMock.post(dataCollectorEndpoint, 200);
Expand All @@ -109,14 +109,14 @@ describe('GoFeatureFlagWebProvider', () => {
});

afterEach(async () => {
await WS.clean();
WS.clean();
websocketMockServer.close();
await OpenFeature.close();
await OpenFeature.clearHooks();
OpenFeature.clearHooks();
fetchMock.mockClear();
fetchMock.mockReset();
await defaultProvider?.onClose();
await jest.resetAllMocks();
jest.resetAllMocks();
readyHandler.mockReset();
errorHandler.mockReset();
configurationChangedHandler.mockReset();
Expand Down Expand Up @@ -145,8 +145,8 @@ describe('GoFeatureFlagWebProvider', () => {
describe('flag evaluation', () => {
it('should change evaluation value if context has changed', async () => {
await OpenFeature.setContext(defaultContext);
OpenFeature.setProvider('test-provider', defaultProvider);
const client = await OpenFeature.getClient('test-provider');
await OpenFeature.setProviderAndWait('test-provider', defaultProvider);
const client = OpenFeature.getClient('test-provider');
await websocketMockServer.connected;
await new Promise((resolve) => setTimeout(resolve, 5));

Expand All @@ -168,11 +168,11 @@ describe('GoFeatureFlagWebProvider', () => {
await OpenFeature.setContext(defaultContext);
const providerName = expect.getState().currentTestName || 'test';
OpenFeature.setProvider(providerName, newDefaultProvider());
const client = await OpenFeature.getClient(providerName);
const client = OpenFeature.getClient(providerName);
await websocketMockServer.connected;
// Need to wait before using the mock
await new Promise((resolve) => setTimeout(resolve, 5));
await websocketMockServer.close();
websocketMockServer.close();

const got = client.getBooleanDetails('bool_flag', false);
expect(got.reason).toEqual(StandardResolutionReasons.CACHED);
Expand All @@ -183,11 +183,11 @@ describe('GoFeatureFlagWebProvider', () => {
const providerName = expect.getState().currentTestName || 'test';
await OpenFeature.setContext(defaultContext);
OpenFeature.setProvider(providerName, newDefaultProvider());
const client = await OpenFeature.getClient(providerName);
const client = OpenFeature.getClient(providerName);
client.addHandler(ProviderEvents.Error, errorHandler);
// wait the event to be triggered
await new Promise((resolve) => setTimeout(resolve, 5));
expect(errorHandler).toBeCalled();
expect(errorHandler).toHaveBeenCalled();
expect(logger.inMemoryLogger['error'][0]).toEqual(
'GoFeatureFlagWebProvider: invalid token used to contact GO Feature Flag instance: Error: Request failed with status code 401',
);
Expand All @@ -196,15 +196,15 @@ describe('GoFeatureFlagWebProvider', () => {
it('should emit an error if we receive a 404 from GO Feature Flag', async () => {
fetchMock.post(allFlagsEndpoint, 404, { overwriteRoutes: true });
await OpenFeature.setContext(defaultContext);
OpenFeature.setProvider('test-provider', defaultProvider);
const client = await OpenFeature.getClient('test-provider');
await OpenFeature.setProviderAndWait('test-provider', defaultProvider);
const client = OpenFeature.getClient('test-provider');
client.addHandler(ProviderEvents.Ready, readyHandler);
client.addHandler(ProviderEvents.Error, errorHandler);
client.addHandler(ProviderEvents.Stale, staleHandler);
client.addHandler(ProviderEvents.ConfigurationChanged, configurationChangedHandler);
// wait the event to be triggered
await new Promise((resolve) => setTimeout(resolve, 5));
expect(errorHandler).toBeCalled();
expect(errorHandler).toHaveBeenCalled();
expect(logger.inMemoryLogger['error'][0]).toEqual(
'GoFeatureFlagWebProvider: impossible to call go-feature-flag relay proxy Error: Request failed with status code 404',
);
Expand All @@ -214,7 +214,7 @@ describe('GoFeatureFlagWebProvider', () => {
const flagKey = 'bool_flag';
await OpenFeature.setContext(defaultContext);
await OpenFeature.setProviderAndWait('test-provider', defaultProvider);
const client = await OpenFeature.getClient('test-provider');
const client = OpenFeature.getClient('test-provider');
await websocketMockServer.connected;
const got = client.getBooleanDetails(flagKey, false);
const want: EvaluationDetails<boolean> = {
Expand All @@ -233,7 +233,7 @@ describe('GoFeatureFlagWebProvider', () => {
const flagKey = 'string_flag';
await OpenFeature.setContext(defaultContext);
await OpenFeature.setProviderAndWait('test-provider', defaultProvider);
const client = await OpenFeature.getClient('test-provider');
const client = OpenFeature.getClient('test-provider');
await websocketMockServer.connected;
const got = client.getStringDetails(flagKey, 'false');
const want: EvaluationDetails<string> = {
Expand All @@ -252,7 +252,7 @@ describe('GoFeatureFlagWebProvider', () => {
const flagKey = 'number_flag';
await OpenFeature.setContext(defaultContext);
await OpenFeature.setProviderAndWait('test-provider', defaultProvider);
const client = await OpenFeature.getClient('test-provider');
const client = OpenFeature.getClient('test-provider');
await websocketMockServer.connected;
const got = client.getNumberDetails(flagKey, 456);
const want: EvaluationDetails<number> = {
Expand All @@ -271,7 +271,7 @@ describe('GoFeatureFlagWebProvider', () => {
const flagKey = 'object_flag';
await OpenFeature.setContext(defaultContext);
await OpenFeature.setProviderAndWait('test-provider', defaultProvider);
const client = await OpenFeature.getClient('test-provider');
const client = OpenFeature.getClient('test-provider');
await websocketMockServer.connected;
const got = client.getObjectDetails(flagKey, { error: true });
const want: EvaluationDetails<JsonValue> = {
Expand All @@ -290,7 +290,7 @@ describe('GoFeatureFlagWebProvider', () => {
const flagKey = 'bool_flag';
await OpenFeature.setContext(defaultContext);
await OpenFeature.setProviderAndWait('test-provider', defaultProvider);
const client = await OpenFeature.getClient('test-provider');
const client = OpenFeature.getClient('test-provider');
await websocketMockServer.connected;
const got = client.getStringDetails(flagKey, 'false');
const want: EvaluationDetails<string> = {
Expand All @@ -308,7 +308,7 @@ describe('GoFeatureFlagWebProvider', () => {
const flagKey = 'not-exist';
await OpenFeature.setContext(defaultContext);
await OpenFeature.setProviderAndWait('test-provider', defaultProvider);
const client = await OpenFeature.getClient('test-provider');
const client = OpenFeature.getClient('test-provider');
await websocketMockServer.connected;
const got = client.getBooleanDetails(flagKey, false);
const want: EvaluationDetails<boolean> = {
Expand Down Expand Up @@ -354,8 +354,8 @@ describe('GoFeatureFlagWebProvider', () => {
describe('eventing', () => {
it('should call client handler with ProviderEvents.Ready when websocket is connected', async () => {
// await OpenFeature.setContext(defaultContext); // we deactivate this call because the context is already set, and we want to avoid calling contextChanged function
OpenFeature.setProvider('test-provider', defaultProvider);
const client = await OpenFeature.getClient('test-provider');
await OpenFeature.setProviderAndWait('test-provider', defaultProvider);
const client = OpenFeature.getClient('test-provider');
client.addHandler(ProviderEvents.Ready, readyHandler);
client.addHandler(ProviderEvents.Error, errorHandler);
client.addHandler(ProviderEvents.Stale, staleHandler);
Expand All @@ -365,16 +365,16 @@ describe('GoFeatureFlagWebProvider', () => {
await websocketMockServer.connected;
await new Promise((resolve) => setTimeout(resolve, 5));

expect(readyHandler).toBeCalled();
expect(errorHandler).not.toBeCalled();
expect(configurationChangedHandler).not.toBeCalled();
expect(staleHandler).not.toBeCalled();
expect(readyHandler).toHaveBeenCalled();
expect(errorHandler).not.toHaveBeenCalled();
expect(configurationChangedHandler).not.toHaveBeenCalled();
expect(staleHandler).not.toHaveBeenCalled();
});

it('should call client handler with ProviderEvents.ConfigurationChanged when websocket is sending update', async () => {
// await OpenFeature.setContext(defaultContext); // we deactivate this call because the context is already set, and we want to avoid calling contextChanged function
OpenFeature.setProvider('test-provider', defaultProvider);
const client = await OpenFeature.getClient('test-provider');
await OpenFeature.setProviderAndWait('test-provider', defaultProvider);
const client = OpenFeature.getClient('test-provider');

client.addHandler(ProviderEvents.Ready, readyHandler);
client.addHandler(ProviderEvents.Error, errorHandler);
Expand Down Expand Up @@ -403,10 +403,10 @@ describe('GoFeatureFlagWebProvider', () => {
// waiting the call to the API to be successful
await new Promise((resolve) => setTimeout(resolve, 50));

expect(readyHandler).toBeCalled();
expect(errorHandler).not.toBeCalled();
expect(configurationChangedHandler).toBeCalled();
expect(staleHandler).not.toBeCalled();
expect(readyHandler).toHaveBeenCalled();
expect(errorHandler).not.toHaveBeenCalled();
expect(configurationChangedHandler).toHaveBeenCalled();
expect(staleHandler).not.toHaveBeenCalled();
expect(configurationChangedHandler.mock.calls[0][0]).toEqual({
clientName: 'test-provider',
domain: 'test-provider',
Expand All @@ -433,8 +433,8 @@ describe('GoFeatureFlagWebProvider', () => {
},
logger,
);
OpenFeature.setProvider('test-provider', provider);
const client = await OpenFeature.getClient('test-provider');
await OpenFeature.setProviderAndWait('test-provider', provider);
const client = OpenFeature.getClient('test-provider');
client.addHandler(ProviderEvents.Ready, readyHandler);
client.addHandler(ProviderEvents.Error, errorHandler);
client.addHandler(ProviderEvents.Stale, staleHandler);
Expand All @@ -444,14 +444,14 @@ describe('GoFeatureFlagWebProvider', () => {
await websocketMockServer.connected;

// Need to wait before using the mock
await new Promise((resolve) => setTimeout(resolve, 5));
await websocketMockServer.close();
await new Promise((resolve) => setTimeout(resolve, 50));
websocketMockServer.close();
await new Promise((resolve) => setTimeout(resolve, 300));

expect(readyHandler).toBeCalled();
expect(errorHandler).not.toBeCalled();
expect(configurationChangedHandler).not.toBeCalled();
expect(staleHandler).toBeCalled();
expect(readyHandler).toHaveBeenCalled();
expect(errorHandler).not.toHaveBeenCalled();
expect(configurationChangedHandler).not.toHaveBeenCalled();
expect(staleHandler).toHaveBeenCalled();
});
});

Expand All @@ -470,7 +470,7 @@ describe('GoFeatureFlagWebProvider', () => {
logger,
);

OpenFeature.setProvider(clientName, p);
await OpenFeature.setProviderAndWait(clientName, p);
const client = OpenFeature.getClient(clientName);
await websocketMockServer.connected;
await new Promise((resolve) => setTimeout(resolve, 5));
Expand Down Expand Up @@ -501,7 +501,7 @@ describe('GoFeatureFlagWebProvider', () => {
logger,
);

OpenFeature.setProvider(clientName, p);
await OpenFeature.setProviderAndWait(clientName, p);
const client = OpenFeature.getClient(clientName);
await websocketMockServer.connected;
await new Promise((resolve) => setTimeout(resolve, 5));
Expand Down Expand Up @@ -531,7 +531,7 @@ describe('GoFeatureFlagWebProvider', () => {
logger,
);

OpenFeature.setProvider(clientName, p);
await OpenFeature.setProviderAndWait(clientName, p);
const client = OpenFeature.getClient(clientName);
await websocketMockServer.connected;
await new Promise((resolve) => setTimeout(resolve, 5));
Expand Down Expand Up @@ -559,7 +559,7 @@ describe('GoFeatureFlagWebProvider', () => {
logger,
);

OpenFeature.setProvider(clientName, p);
await OpenFeature.setProviderAndWait(clientName, p);
const client = OpenFeature.getClient(clientName);
await websocketMockServer.connected;
await new Promise((resolve) => setTimeout(resolve, 5));
Expand All @@ -574,7 +574,7 @@ describe('GoFeatureFlagWebProvider', () => {
it('should have a log when data collector is not available', async () => {
const clientName = expect.getState().currentTestName ?? 'test-provider';
fetchMock.post(dataCollectorEndpoint, 500, { overwriteRoutes: true });
OpenFeature.setContext(defaultContext);
await OpenFeature.setContext(defaultContext);
const p = new GoFeatureFlagWebProvider(
{
endpoint: endpoint,
Expand All @@ -585,7 +585,7 @@ describe('GoFeatureFlagWebProvider', () => {
logger,
);

OpenFeature.setProvider(clientName, p);
await OpenFeature.setProviderAndWait(clientName, p);
const client = OpenFeature.getClient(clientName);
await websocketMockServer.connected;
await new Promise((resolve) => setTimeout(resolve, 5));
Expand All @@ -606,4 +606,51 @@ describe('GoFeatureFlagWebProvider', () => {
await OpenFeature.close();
});
});
it('should resolve when WebSocket is open', async () => {
const provider = new GoFeatureFlagWebProvider({ endpoint: 'http://localhost:1031', apiTimeout: 1000 });
await provider.initialize({ targetingKey: 'user-key' });
const websocket = new WebSocket(websocketEndpoint);
await websocketMockServer.connected;
await expect(provider.waitWebsocketFinalStatus(websocket)).resolves.toBeUndefined();
});

// how can I mock a websocket server to stay in CONNECTING state
it('should timeout if websocket stay in CONNECTING state', async () => {
const provider = new GoFeatureFlagWebProvider({ endpoint: 'http://localhost:1031', apiTimeout: 1000 });
await provider.initialize({ targetingKey: 'user-key' });
const websocket = new MockWebSocketConnectingState(websocketEndpoint);

// Now you can test the behavior when the WebSocket is in CONNECTING state
await expect(provider.waitWebsocketFinalStatus(websocket)).rejects.toBe(
'timeout of 1000 ms reached when initializing the websocket',
);
});
});

class MockWebSocketConnectingState extends WebSocket {
constructor(url: string, protocols?: string | string[]) {
super(url, protocols);
}

get readyState() {
return WebSocket.CONNECTING;
}

set onopen(_: { (this: WebSocket, event: Event): void; (): void }) {
// Do nothing to prevent setting the onopen handler
}

set onclose(_: { (): Promise<void>; (): void }) {
// Do nothing to prevent setting the onclose handler
}

addEventListener(
type: string,
listener: EventListenerOrEventListenerObject,
options?: boolean | AddEventListenerOptions,
): void {
if (type !== 'open' && type !== 'close') {
super.addEventListener(type, listener, options);
}
}
}
27 changes: 19 additions & 8 deletions libs/providers/go-feature-flag-web/src/lib/go-feature-flag-web-provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,9 @@ export class GoFeatureFlagWebProvider implements Provider {
this._logger?.debug(`${GoFeatureFlagWebProvider.name}: Trying to connect the websocket at ${wsURL}`);

this._websocket = new WebSocket(wsURL);
await this.waitWebsocketFinalStatus(this._websocket);
await this.waitWebsocketFinalStatus(this._websocket).catch((reason) => {
throw new Error(`impossible to connect to the websocket: ${reason}`);
});

this._websocket.onopen = (event) => {
this._logger?.info(`${GoFeatureFlagWebProvider.name}: Websocket to go-feature-flag open: ${event}`);
Expand All @@ -133,15 +135,24 @@ export class GoFeatureFlagWebProvider implements Provider {
* @param socket - the websocket you are waiting for
*/
waitWebsocketFinalStatus(socket: WebSocket): Promise<void> {
return new Promise((resolve) => {
const checkConnection = () => {
if (socket.readyState === WebSocket.OPEN || socket.readyState === WebSocket.CLOSED) {
return resolve();
return new Promise((resolve, reject) => {
// wait until the socket is in a stable state or until the timeout is reached
const websocketTimeout = this._apiTimeout !== 0 ? this._apiTimeout : 5000;
const timeout = setTimeout(() => {
if (socket.readyState !== WebSocket.OPEN && socket.readyState !== WebSocket.CLOSED) {
reject(`timeout of ${websocketTimeout} ms reached when initializing the websocket`);
}
// Wait 5 milliseconds before checking again
setTimeout(checkConnection, 5);
}, websocketTimeout);

socket.onopen = () => {
clearTimeout(timeout);
resolve();
};

socket.onclose = () => {
clearTimeout(timeout);
resolve();
};
checkConnection();
});
}

Expand Down
Loading