Skip to content

feat: add support for abort controllers to event handlers #1151

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 1 commit into from
Feb 26, 2025
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
18 changes: 7 additions & 11 deletions packages/react/src/evaluation/use-feature-flag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ function attachHandlersAndResolve<T extends FlagValue>(
const defaultedOptions = { ...DEFAULT_OPTIONS, ...useProviderOptions(), ...normalizeOptions(options) };
const client = useOpenFeatureClient();
const status = useOpenFeatureClientStatus();
const controller = new AbortController();

// suspense
if (defaultedOptions.suspendUntilReady && status === ProviderStatus.NOT_READY) {
Expand Down Expand Up @@ -322,28 +323,23 @@ function attachHandlersAndResolve<T extends FlagValue>(
useEffect(() => {
if (status === ProviderStatus.NOT_READY) {
// update when the provider is ready
client.addHandler(ProviderEvents.Ready, updateEvaluationDetailsCallback);
client.addHandler(ProviderEvents.Ready, updateEvaluationDetailsCallback, { signal: controller.signal });
}

if (defaultedOptions.updateOnContextChanged) {
// update when the context changes
client.addHandler(ProviderEvents.ContextChanged, updateEvaluationDetailsCallback);
client.addHandler(ProviderEvents.ContextChanged, updateEvaluationDetailsCallback, { signal: controller.signal });
}
return () => {
// cleanup the handlers
client.removeHandler(ProviderEvents.Ready, updateEvaluationDetailsCallback);
client.removeHandler(ProviderEvents.ContextChanged, updateEvaluationDetailsCallback);
};
}, []);

useEffect(() => {
if (defaultedOptions.updateOnConfigurationChanged) {
// update when the provider configuration changes
client.addHandler(ProviderEvents.ConfigurationChanged, configurationChangeCallback);
client.addHandler(ProviderEvents.ConfigurationChanged, configurationChangeCallback, {
signal: controller.signal,
});
}
return () => {
// cleanup the handlers
client.removeHandler(ProviderEvents.ConfigurationChanged, configurationChangeCallback);
controller.abort();
};
}, []);

Expand Down
20 changes: 8 additions & 12 deletions packages/react/src/provider/use-open-feature-client-status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,18 @@ import { ProviderEvents } from '@openfeature/web-sdk';
export function useOpenFeatureClientStatus(): ProviderStatus {
const client = useOpenFeatureClient();
const [status, setStatus] = useState<ProviderStatus>(client.providerStatus);
const controller = new AbortController();

useEffect(() => {
const updateStatus = () => setStatus(client.providerStatus);
client.addHandler(ProviderEvents.ConfigurationChanged, updateStatus);
client.addHandler(ProviderEvents.ContextChanged, updateStatus);
client.addHandler(ProviderEvents.Error, updateStatus);
client.addHandler(ProviderEvents.Ready, updateStatus);
client.addHandler(ProviderEvents.Stale, updateStatus);
client.addHandler(ProviderEvents.Reconciling, updateStatus);
client.addHandler(ProviderEvents.ConfigurationChanged, updateStatus, { signal: controller.signal });
client.addHandler(ProviderEvents.ContextChanged, updateStatus, { signal: controller.signal });
client.addHandler(ProviderEvents.Error, updateStatus, { signal: controller.signal });
client.addHandler(ProviderEvents.Ready, updateStatus, { signal: controller.signal });
client.addHandler(ProviderEvents.Stale, updateStatus, { signal: controller.signal });
client.addHandler(ProviderEvents.Reconciling, updateStatus, { signal: controller.signal });
return () => {
client.removeHandler(ProviderEvents.ConfigurationChanged, updateStatus);
client.removeHandler(ProviderEvents.ContextChanged, updateStatus);
client.removeHandler(ProviderEvents.Error, updateStatus);
client.removeHandler(ProviderEvents.Ready, updateStatus);
client.removeHandler(ProviderEvents.Stale, updateStatus);
client.removeHandler(ProviderEvents.Reconciling, updateStatus);
controller.abort();
};
}, [client]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type {
OpenFeatureError,
FlagMetadata,
ResolutionDetails,
EventOptions,
} from '@openfeature/core';
import {
ErrorCode,
Expand Down Expand Up @@ -79,7 +80,7 @@ export class OpenFeatureClient implements Client {
return this.providerStatusAccessor();
}

addHandler(eventType: ProviderEvents, handler: EventHandler): void {
addHandler(eventType: ProviderEvents, handler: EventHandler, options?: EventOptions): void {
this.emitterAccessor().addHandler(eventType, handler);
const shouldRunNow = statusMatchesEvent(eventType, this._providerStatus);

Expand All @@ -95,6 +96,12 @@ export class OpenFeatureClient implements Client {
this._logger?.error('Error running event handler:', err);
}
}

if (options?.signal && typeof options.signal.addEventListener === 'function') {
options.signal.addEventListener('abort', () => {
this.removeHandler(eventType, handler);
});
}
}

removeHandler(eventType: ProviderEvents, handler: EventHandler) {
Expand Down
31 changes: 30 additions & 1 deletion packages/server/test/events.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,21 @@ describe('Events', () => {
expect(OpenFeature.getHandlers(eventType)).toHaveLength(0);
});

it('The API provides a function allowing the removal of event handlers', () => {
it('The event handler can be removed using an abort signal', () => {
const abortController = new AbortController();
const handler1 = jest.fn();
const handler2 = jest.fn();
const eventType = ProviderEvents.Stale;

OpenFeature.addHandler(eventType, handler1, { signal: abortController.signal });
OpenFeature.addHandler(eventType, handler2);
expect(OpenFeature.getHandlers(eventType)).toHaveLength(2);

abortController.abort();
expect(OpenFeature.getHandlers(eventType)).toHaveLength(1);
});

it('The API provides a function allowing the removal of event handlers from client', () => {
const client = OpenFeature.getClient(domain);
const handler = jest.fn();
const eventType = ProviderEvents.Stale;
Expand All @@ -459,6 +473,21 @@ describe('Events', () => {
client.removeHandler(eventType, handler);
expect(client.getHandlers(eventType)).toHaveLength(0);
});

it('The event handler on the client can be removed using an abort signal', () => {
const abortController = new AbortController();
const client = OpenFeature.getClient(domain);
const handler1 = jest.fn();
const handler2 = jest.fn();
const eventType = ProviderEvents.Stale;

client.addHandler(eventType, handler1, { signal: abortController.signal });
client.addHandler(eventType, handler2);
expect(client.getHandlers(eventType)).toHaveLength(2);

abortController.abort();
expect(client.getHandlers(eventType)).toHaveLength(1);
});
});

describe('Requirement 5.3.1', () => {
Expand Down
7 changes: 7 additions & 0 deletions packages/shared/src/events/eventing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,17 @@ export type EventDetails<
export type EventHandler<
T extends ServerProviderEvents | ClientProviderEvents = ServerProviderEvents | ClientProviderEvents,
> = (eventDetails?: EventDetails<T>) => Promise<unknown> | unknown;
export type EventOptions = {
signal?: AbortSignal;
};

export interface Eventing<T extends ServerProviderEvents | ClientProviderEvents> {
/**
* Adds a handler for the given provider event type.
* The handlers are called in the order they have been added.
* @param eventType The provider event type to listen to
* @param {EventHandler} handler The handler to run on occurrence of the event type
* @param {EventOptions} options Optional options such as signal for aborting
*/
addHandler(
eventType: T extends ClientProviderEvents
Expand All @@ -83,14 +87,17 @@ export interface Eventing<T extends ServerProviderEvents | ClientProviderEvents>
? ClientProviderEvents.ConfigurationChanged
: ServerProviderEvents.ConfigurationChanged
>,
options?: EventOptions,
): void;
addHandler(
eventType: T extends ClientProviderEvents ? ClientNotChangeEvents : ServerNotChangeEvents,
handler: EventHandler<T extends ClientProviderEvents ? ClientNotChangeEvents : ServerNotChangeEvents>,
options?: EventOptions,
): void;
addHandler(
eventType: T extends ClientProviderEvents ? ClientProviderEvents : ServerProviderEvents,
handler: EventHandler<T extends ClientProviderEvents ? ClientProviderEvents : ServerProviderEvents>,
options?: EventOptions,
): void;

/**
Expand Down
19 changes: 12 additions & 7 deletions packages/shared/src/open-feature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,13 @@ import type {
EventDetails,
EventHandler,
Eventing,
GenericEventEmitter} from './events';
import {
AllProviderEvents,
statusMatchesEvent,
EventOptions,
GenericEventEmitter,
} from './events';
import { AllProviderEvents, statusMatchesEvent } from './events';
import { isDefined } from './filter';
import type { BaseHook, EvaluationLifeCycle } from './hooks';
import type { Logger, ManageLogger} from './logger';
import type { Logger, ManageLogger } from './logger';
import { DefaultLogger, SafeLogger } from './logger';
import type { ClientProviderStatus, CommonProvider, ProviderMetadata, ServerProviderStatus } from './provider';
import { objectOrUndefined, stringOrUndefined } from './type-guards';
Expand Down Expand Up @@ -154,8 +153,9 @@ export abstract class OpenFeatureCommonAPI<
* API (global) events run for all providers.
* @param {AnyProviderEvent} eventType The provider event type to listen to
* @param {EventHandler} handler The handler to run on occurrence of the event type
* @param {EventOptions} options Optional options such as signal for aborting
*/
addHandler<T extends AnyProviderEvent>(eventType: T, handler: EventHandler): void {
addHandler<T extends AnyProviderEvent>(eventType: T, handler: EventHandler, options?: EventOptions): void {
[...new Map([[undefined, this._defaultProvider]]), ...this._domainScopedProviders].forEach((keyProviderTuple) => {
const domain = keyProviderTuple[0];
const provider = keyProviderTuple[1].provider;
Expand All @@ -173,6 +173,11 @@ export abstract class OpenFeatureCommonAPI<
});

this._apiEmitter.addHandler(eventType, handler);
if (options?.signal && typeof options.signal.addEventListener === 'function') {
options.signal.addEventListener('abort', () => {
this.removeHandler(eventType, handler);
});
}
}

/**
Expand Down Expand Up @@ -248,7 +253,7 @@ export abstract class OpenFeatureCommonAPI<
// initialize the provider if it implements "initialize" and it's not already registered
if (typeof provider.initialize === 'function' && !this.allProviders.includes(provider)) {
initializationPromise = provider
.initialize?.(domain ? this._domainScopedContext.get(domain) ?? this._context : this._context)
.initialize?.(domain ? (this._domainScopedContext.get(domain) ?? this._context) : this._context)
?.then(() => {
wrappedProvider.status = this._statusEnumType.READY;
// fetch the most recent event emitters, some may have been added during init
Expand Down
36 changes: 10 additions & 26 deletions packages/shared/test/events.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,23 @@ class TestEventEmitter extends GenericEventEmitter<AnyProviderEvent> {
}
}

// a little function to make sure we're at least waiting for the event loop
// a little function to make sure we're at least waiting for the event loop
// to clear before we start making assertions
const wait = (millis = 0) => {
return new Promise(resolve => {setTimeout(resolve, millis);});
return new Promise((resolve) => {
setTimeout(resolve, millis);
});
};

describe('GenericEventEmitter', () => {
const emitter = new TestEventEmitter();

afterEach(() => {
emitter.removeAllHandlers();
});

describe('addHandler should', function () {
it('attach handler for event type', async function () {
const emitter = new TestEventEmitter();

const handler1 = jest.fn();
emitter.addHandler(AllProviderEvents.Ready, handler1);
emitter.emit(AllProviderEvents.Ready);
Expand All @@ -35,8 +41,6 @@ describe('GenericEventEmitter', () => {
});

it('attach several handlers for event type', async function () {
const emitter = new TestEventEmitter();

const handler1 = jest.fn();
const handler2 = jest.fn();
const handler3 = jest.fn();
Expand Down Expand Up @@ -64,7 +68,6 @@ describe('GenericEventEmitter', () => {
debug: () => done(),
};

const emitter = new TestEventEmitter();
emitter.setLogger(logger);

emitter.addHandler(AllProviderEvents.Ready, async () => {
Expand All @@ -74,8 +77,6 @@ describe('GenericEventEmitter', () => {
});

it('trigger handler for event type', async function () {
const emitter = new TestEventEmitter();

const handler1 = jest.fn();
emitter.addHandler(AllProviderEvents.Ready, handler1);
emitter.emit(AllProviderEvents.Ready);
Expand All @@ -87,7 +88,6 @@ describe('GenericEventEmitter', () => {

it('trigger handler for event type with event data', async function () {
const event: ReadyEvent = { message: 'message' };
const emitter = new TestEventEmitter();

const handler1 = jest.fn();
emitter.addHandler(AllProviderEvents.Ready, handler1);
Expand All @@ -99,8 +99,6 @@ describe('GenericEventEmitter', () => {
});

it('trigger several handlers for event type', async function () {
const emitter = new TestEventEmitter();

const handler1 = jest.fn();
const handler2 = jest.fn();
const handler3 = jest.fn();
Expand All @@ -121,8 +119,6 @@ describe('GenericEventEmitter', () => {

describe('removeHandler should', () => {
it('remove single handler', async function () {
const emitter = new TestEventEmitter();

const handler1 = jest.fn();
emitter.addHandler(AllProviderEvents.Ready, handler1);

Expand All @@ -138,8 +134,6 @@ describe('GenericEventEmitter', () => {

describe('removeAllHandlers should', () => {
it('remove all handlers for event type', async function () {
const emitter = new TestEventEmitter();

const handler1 = jest.fn();
const handler2 = jest.fn();
emitter.addHandler(AllProviderEvents.Ready, handler1);
Expand All @@ -156,8 +150,6 @@ describe('GenericEventEmitter', () => {
});

it('remove same handler when assigned to multiple events', async function () {
const emitter = new TestEventEmitter();

const handler = jest.fn();
emitter.addHandler(AllProviderEvents.Stale, handler);
emitter.addHandler(AllProviderEvents.ContextChanged, handler);
Expand All @@ -174,8 +166,6 @@ describe('GenericEventEmitter', () => {
});

it('allow addition/removal of duplicate handlers', async function () {
const emitter = new TestEventEmitter();

const handler = jest.fn();
emitter.addHandler(AllProviderEvents.Stale, handler);
emitter.addHandler(AllProviderEvents.Stale, handler);
Expand All @@ -191,8 +181,6 @@ describe('GenericEventEmitter', () => {
});

it('allow duplicate event handlers and call them', async function () {
const emitter = new TestEventEmitter();

const handler = jest.fn();
emitter.addHandler(AllProviderEvents.Stale, handler);
emitter.addHandler(AllProviderEvents.Stale, handler);
Expand All @@ -205,8 +193,6 @@ describe('GenericEventEmitter', () => {
});

it('remove all handlers only for event type', async function () {
const emitter = new TestEventEmitter();

const handler1 = jest.fn();
const handler2 = jest.fn();
emitter.addHandler(AllProviderEvents.Ready, handler1);
Expand All @@ -223,8 +209,6 @@ describe('GenericEventEmitter', () => {
});

it('remove all handlers if no event type is given', async function () {
const emitter = new TestEventEmitter();

const handler1 = jest.fn();
const handler2 = jest.fn();
emitter.addHandler(AllProviderEvents.Ready, handler1);
Expand Down
9 changes: 8 additions & 1 deletion packages/web/src/client/internal/open-feature-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type {
OpenFeatureError,
FlagMetadata,
ResolutionDetails,
EventOptions,
} from '@openfeature/core';
import {
ErrorCode,
Expand Down Expand Up @@ -74,7 +75,7 @@ export class OpenFeatureClient implements Client {
return this.providerStatusAccessor();
}

addHandler(eventType: ProviderEvents, handler: EventHandler): void {
addHandler(eventType: ProviderEvents, handler: EventHandler, options: EventOptions): void {
this.emitterAccessor().addHandler(eventType, handler);
const shouldRunNow = statusMatchesEvent(eventType, this.providerStatus);

Expand All @@ -90,6 +91,12 @@ export class OpenFeatureClient implements Client {
this._logger?.error('Error running event handler:', err);
}
}

if (options?.signal && typeof options.signal.addEventListener === 'function') {
options.signal.addEventListener('abort', () => {
this.removeHandler(eventType, handler);
});
}
}

removeHandler(notificationType: ProviderEvents, handler: EventHandler): void {
Expand Down
Loading