Skip to content
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ describe('sdk-client storage', () => {
},
);

// Register an error listener so errors are emitted (not just logged).
ldc.on('error', () => {});
const changePromise = onChangePromise();
await ldc.identify(context);
await changePromise;
Expand Down Expand Up @@ -136,6 +138,8 @@ describe('sdk-client storage', () => {
emitter = ldc.emitter;
jest.spyOn(emitter as LDEmitter, 'emit');

// Register an error listener so errors are emitted (not just logged).
ldc.on('error', () => {});
await ldc.identify(context);
await jest.runAllTimersAsync();

Expand Down
28 changes: 27 additions & 1 deletion packages/shared/sdk-client/__tests__/LDClientImpl.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,32 @@ describe('sdk-client object', () => {
expect(logger.error).toHaveBeenNthCalledWith(2, expect.stringContaining('Received error 404'));
});

test('registering on(error) suppresses internal console logging', async () => {
mockPlatform.requests.createEventSource.mockImplementation(
(streamUri: string = '', options: any = {}) => {
mockEventSource = new MockEventSource(streamUri, options);
mockEventSource.simulateError({ status: 404, message: 'test-error' });
return mockEventSource;
},
);

const errorHandler = jest.fn();
ldc.on('error', errorHandler);

const carContext: LDContext = { kind: 'car', key: 'test-car' };
await expect(ldc.identify(carContext)).rejects.toThrow('test-error');

expect(errorHandler).toHaveBeenCalledWith(
expect.objectContaining({ car: { key: 'test-car' } }),
expect.objectContaining({ message: 'test-error' }),
);

const maybeReportErrorCalls = (logger.error as jest.Mock).mock.calls.filter(
([msg]: [string]) => typeof msg === 'string' && msg.startsWith('error:'),
);
expect(maybeReportErrorCalls).toHaveLength(0);
});

test('identify change and error listeners', async () => {
// @ts-ignore
const { emitter } = ldc;
Expand All @@ -331,7 +357,7 @@ describe('sdk-client object', () => {
// No default listeners. This is important for clients to be able to determine if there are
// any listeners and act on that information.
expect(emitter.listenerCount('change')).toEqual(0);
expect(emitter.listenerCount('error')).toEqual(1);
expect(emitter.listenerCount('error')).toEqual(0);
});

test('can complete identification using storage', async () => {
Expand Down
57 changes: 57 additions & 0 deletions packages/shared/sdk-client/__tests__/LDEmitter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,63 @@ describe('LDEmitter', () => {
);
});

describe('maybeReportError', () => {
it('emits error to listeners when listeners are registered', () => {
const errorHandler = jest.fn();
const context = { kind: 'user', key: 'test-user' };
const err = new Error('test-error');

emitter.on('error', errorHandler);
emitter.maybeReportError(context, err);

expect(errorHandler).toHaveBeenCalledWith(context, err);
expect(logger.error).not.toHaveBeenCalled();
});

it('logs error when no listeners are registered', () => {
const context = { kind: 'user', key: 'test-user' };
const err = new Error('test-error');

emitter.maybeReportError(context, err);

expect(logger.error).toHaveBeenCalledWith(expect.stringContaining('test-error'));
});

it('suppresses logging when a listener is added after construction', () => {
const errorHandler = jest.fn();
const context = { kind: 'user', key: 'test-user' };
const err = new Error('first-error');

// First error — no listener, should log
emitter.maybeReportError(context, err);
expect(logger.error).toHaveBeenCalledTimes(1);

// Register a listener
emitter.on('error', errorHandler);

// Second error — listener present, should emit not log
const err2 = new Error('second-error');
emitter.maybeReportError(context, err2);
expect(errorHandler).toHaveBeenCalledWith(context, err2);
expect(logger.error).toHaveBeenCalledTimes(1); // still just the first call
});

it('resumes logging when all listeners are removed', () => {
const errorHandler = jest.fn();
const context = { kind: 'user', key: 'test-user' };

emitter.on('error', errorHandler);
emitter.maybeReportError(context, new Error('emitted'));
expect(errorHandler).toHaveBeenCalledTimes(1);
expect(logger.error).not.toHaveBeenCalled();

emitter.off('error', errorHandler);
emitter.maybeReportError(context, new Error('logged'));
expect(errorHandler).toHaveBeenCalledTimes(1); // not called again
expect(logger.error).toHaveBeenCalledTimes(1);
});
});

it('logs a warning when a non-string event name is provided to on', () => {
const handler = jest.fn();
emitter.on(123 as any, handler);
Expand Down
4 changes: 2 additions & 2 deletions packages/shared/sdk-client/src/DataManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ export abstract class BaseDataManager implements DataManager {
identifyResolve?.();
},
(err) => {
this.emitter.emit('error', context, err);
this.emitter.maybeReportError(context, err);
this._dataSourceEventHandler.handlePollingError(err);
identifyReject?.(err);
},
Expand Down Expand Up @@ -205,7 +205,7 @@ export abstract class BaseDataManager implements DataManager {
pollingRequestor,
this.diagnosticsManager,
(e) => {
this.emitter.emit('error', context, e);
this.emitter.maybeReportError(context, e);
this._dataSourceEventHandler.handleStreamingError(e);
identifyReject?.(e);
},
Expand Down
9 changes: 3 additions & 6 deletions packages/shared/sdk-client/src/LDClientImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,7 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult {
this._baseHeaders,
this._diagnosticsManager,
);
this.emitter = new LDEmitter();
this.emitter.on('error', (c: LDContextStrict, err: any) => {
this.logger.error(`error: ${err}, context: ${JSON.stringify(c)}`);
});
this.emitter = new LDEmitter(this.logger);

this._flagManager.on((context, flagKeys, type) => {
this._handleInspectionChanged(flagKeys, type);
Expand Down Expand Up @@ -342,7 +339,7 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult {

if (!checkedContext.valid) {
const error = new Error('Context was unspecified or had no key');
this.emitter.emit('error', context, error);
this.emitter.maybeReportError(context, error);
return Promise.reject(error);
}
this._activeContextTracker.set(context, checkedContext);
Expand Down Expand Up @@ -577,7 +574,7 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult {
const error = new LDClientError(
`Wrong type "${type}" for feature flag "${flagKey}"; returning default value`,
);
this.emitter.emit('error', this._activeContextTracker.getUnwrappedContext(), error);
this.emitter.maybeReportError(this._activeContextTracker.getUnwrappedContext(), error);
return createErrorEvaluationDetail(ErrorKinds.WrongType, defaultValue);
}
}
Expand Down
17 changes: 17 additions & 0 deletions packages/shared/sdk-client/src/LDEmitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,23 @@ export default class LDEmitter {
listeners?.forEach((listener) => this._invokeListener(listener, name, ...detail));
}

/**
* Reports an error to registered error listeners, or logs it if none are registered.
*
* @remarks
* If a user has registered an error listener via `on('error', handler)`, the error
* is emitted to those listeners and internal logging is suppressed.
* If no listener is registered, the error is logged via the SDK logger.
*/
maybeReportError(...detail: any[]) {
if (this.listenerCount('error')) {
this.emit('error', ...detail);
} else {
const [context, error] = detail;
this._logger?.error(`error: ${error}, context: ${JSON.stringify(context)}`);
}
}

eventNames(): string[] {
return [...this._listeners.keys()];
}
Expand Down
Loading