Skip to content

fix: only shutdown providers that are not attached to any client #444

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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 23 additions & 0 deletions packages/client/test/open-feature.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,29 @@ describe('OpenFeature', () => {

expect(namedClient.metadata.providerMetadata.name).toEqual(fakeProvider.metadata.name);
});

it('should close a provider if it is replaced and no other client uses it', async () => {
const provider1 = { ...MOCK_PROVIDER, onClose: jest.fn() };
const provider2 = { ...MOCK_PROVIDER, onClose: jest.fn() };

OpenFeature.setProvider('client1', provider1);
expect(provider1.onClose).not.toHaveBeenCalled();
OpenFeature.setProvider('client1', provider2);
expect(provider1.onClose).toHaveBeenCalledTimes(1);
});

it('should not close provider if it is used by another client', async () => {
const provider1 = { ...MOCK_PROVIDER, onClose: jest.fn() };

OpenFeature.setProvider('client1', provider1);
OpenFeature.setProvider('client2', provider1);

OpenFeature.setProvider('client1', { ...provider1 });
expect(provider1.onClose).not.toHaveBeenCalled();

OpenFeature.setProvider('client2', { ...provider1 });
expect(provider1.onClose).toHaveBeenCalledTimes(1);
});
});

describe('Requirement 1.1.4', () => {
Expand Down
23 changes: 23 additions & 0 deletions packages/server/test/open-feature.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,29 @@ describe('OpenFeature', () => {

expect(namedClient.metadata.providerMetadata.name).toEqual(fakeProvider.metadata.name);
});

it('should close a provider if it is replaced and no other client uses it', async () => {
const provider1 = { ...MOCK_PROVIDER, onClose: jest.fn() };
const provider2 = { ...MOCK_PROVIDER, onClose: jest.fn() };

OpenFeature.setProvider('client1', provider1);
expect(provider1.onClose).not.toHaveBeenCalled();
OpenFeature.setProvider('client1', provider2);
expect(provider1.onClose).toHaveBeenCalledTimes(1);
});

it('should not close provider if it is used by another client', async () => {
const provider1 = { ...MOCK_PROVIDER, onClose: jest.fn() };

OpenFeature.setProvider('client1', provider1);
OpenFeature.setProvider('client2', provider1);

OpenFeature.setProvider('client1', { ...provider1 });
expect(provider1.onClose).not.toHaveBeenCalled();

OpenFeature.setProvider('client2', { ...provider1 });
expect(provider1.onClose).toHaveBeenCalledTimes(1);
});
});

describe('Requirement 1.1.4', () => {
Expand Down
5 changes: 3 additions & 2 deletions packages/shared/src/open-feature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,11 @@ export abstract class OpenFeatureCommonAPI<P extends CommonProvider = CommonProv

this.transferListeners(oldProvider, provider, clientName, clientEmitter);

// Do not close the default provider if a named client used the default provider
if (!clientName || (clientName && oldProvider !== this._defaultProvider)) {
// Do not close a provider that is bound to any client
if (![...this._clientProviders.values(), this._defaultProvider].includes(oldProvider)) {
oldProvider?.onClose?.();
}

return this;
}

Expand Down