From 0b85e273098fede8540b5f79af807a3f9dec10d6 Mon Sep 17 00:00:00 2001 From: Stanley Phu Date: Mon, 19 Oct 2026 15:50:25 -0700 Subject: [PATCH 1/2] fix(feature-flags): Keep polling when no error listener is attached Co-Authored-By: Claude Opus 5.5 --- README.md | 3 +- src/feature-flags/runtime-client.spec.ts | 107 +++++++++++++++++++++-- src/feature-flags/runtime-client.ts | 13 ++- 3 files changed, 113 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index dce77ddd2..b68d34beb 100644 --- a/README.md +++ b/README.md @@ -162,7 +162,8 @@ polling populates the cache afterward. A failed poll, including one that returns a payload the SDK can't read, emits `error`, keeps the last good snapshot, and retries with backoff. A 401 stops -polling and also emits `failed`. +polling and also emits `failed`. Attach an `error` listener or a `logger` to see +failures; with neither, the client warns once and keeps polling. ## SDK Versioning diff --git a/src/feature-flags/runtime-client.spec.ts b/src/feature-flags/runtime-client.spec.ts index 5e9d0cda4..3e246b61e 100644 --- a/src/feature-flags/runtime-client.spec.ts +++ b/src/feature-flags/runtime-client.spec.ts @@ -558,14 +558,9 @@ describe('FeatureFlagsRuntimeClient', () => { }); it('throws on an error event when no listener is attached', () => { - // poll()'s catch block emits 'error'. With no listener, the emitter must - // throw (Node semantics) rather than silently drop it — the behavior - // eventemitter3 lacked and the old emit() override restored. Every other - // error test attaches a listener first, so this is the only guard against - // re-introducing silent error-dropping. Asserted on the inherited emit - // directly: via the real poll path the throw would surface as an - // unhandled promise rejection (poll runs in a setTimeout), which can't be - // observed deterministically. + // With no listener, the emitter must throw (Node semantics) rather than + // silently drop an 'error' — the behavior eventemitter3 lacked. poll() + // checks for listeners before emitting; the tests below cover that path. const client = workos.featureFlags.createRuntimeClient(); const boom = new Error('boom'); @@ -573,5 +568,101 @@ describe('FeatureFlagsRuntimeClient', () => { client.close(); }); + + it('keeps polling when a poll fails with no error listener', async () => { + const emitWarning = jest + .spyOn(workos, 'emitWarning') + .mockImplementation(() => {}); + fetch.mockRejectOnce(new Error('Network error')); + + const client = workos.featureFlags.createRuntimeClient(); + await jest.advanceTimersByTimeAsync(0); + + fetchOnce(pollResponse); + await jest.advanceTimersByTimeAsync(35_000); + + expect(client.getStats()).toMatchObject({ + pollCount: 2, + pollErrorCount: 1, + flagCount: 2, + }); + expect(client.isEnabled('flag-a')).toBe(true); + + client.close(); + emitWarning.mockRestore(); + }); + + it('rejects waitUntilReady and emits failed on 401 with no error listener', async () => { + const emitWarning = jest + .spyOn(workos, 'emitWarning') + .mockImplementation(() => {}); + fetchOnce( + { message: 'Unauthorized' }, + { status: 401, headers: { 'X-Request-ID': 'req_123' } }, + ); + + const client = workos.featureFlags.createRuntimeClient(); + const failures: unknown[] = []; + client.on('failed', (err) => failures.push(err)); + let readyError: unknown; + client.waitUntilReady().catch((err) => { + readyError = err; + }); + + await jest.advanceTimersByTimeAsync(0); + + expect(readyError).toBeInstanceOf(UnauthorizedException); + expect(failures).toHaveLength(1); + expect(fetch.mock.calls.length).toBe(1); + + client.close(); + emitWarning.mockRestore(); + }); + + it('warns once when polls fail with no error listener or logger', async () => { + const emitWarning = jest + .spyOn(workos, 'emitWarning') + .mockImplementation(() => {}); + fetch.mockRejectOnce(new Error('Network error')); + fetch.mockRejectOnce(new Error('Network error')); + + const client = workos.featureFlags.createRuntimeClient(); + await jest.advanceTimersByTimeAsync(0); + await jest.advanceTimersByTimeAsync(35_000); + + expect(client.getStats().pollErrorCount).toBe(2); + expect(emitWarning).toHaveBeenCalledTimes(1); + expect(emitWarning).toHaveBeenCalledWith( + expect.stringContaining("no 'error' listener or logger"), + ); + + client.close(); + emitWarning.mockRestore(); + }); + + it('logs instead of warning when a logger is attached', async () => { + const emitWarning = jest + .spyOn(workos, 'emitWarning') + .mockImplementation(() => {}); + const logger = { + debug: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + }; + fetch.mockRejectOnce(new Error('Network error')); + + const client = workos.featureFlags.createRuntimeClient({ logger }); + await jest.advanceTimersByTimeAsync(0); + + expect(logger.error).toHaveBeenCalledWith( + 'Poll failed', + expect.any(Error), + ); + expect(emitWarning).not.toHaveBeenCalled(); + + client.close(); + emitWarning.mockRestore(); + }); }); }); diff --git a/src/feature-flags/runtime-client.ts b/src/feature-flags/runtime-client.ts index 616522863..705f8b1d2 100644 --- a/src/feature-flags/runtime-client.ts +++ b/src/feature-flags/runtime-client.ts @@ -39,6 +39,7 @@ export class FeatureFlagsRuntimeClient extends EventEmitter private closed = false; private initialized = false; private consecutiveErrors = 0; + private warnedUnobservedError = false; private pollTimer: ReturnType | null = null; private pollAbortController: AbortController | null = null; @@ -196,7 +197,17 @@ export class FeatureFlagsRuntimeClient extends EventEmitter this.consecutiveErrors++; this.stats.pollErrorCount++; - this.emit('error', err); + // A background poll has no caller to throw to. Emitting 'error' with no + // listener throws, which would skip the retry below and surface as an + // unhandled rejection, so an unobserved failure is reported instead. + if (this.listenerCount('error') > 0) { + this.emit('error', err); + } else if (!this.logger && !this.warnedUnobservedError) { + this.warnedUnobservedError = true; + this.workos.emitWarning( + `Feature flag sync failed and no 'error' listener or logger is attached; keeping the last good flags and retrying. ${err.message}`, + ); + } this.logger?.error('Poll failed', err); if (err instanceof UnauthorizedException) { From a2bdfbe0fd66a0740df3866778744021ed5e3afe Mon Sep 17 00:00:00 2001 From: Stanley Phu Date: Mon, 19 Oct 2026 15:57:14 -0700 Subject: [PATCH 2/2] fix(feature-flags): Warn when a 401 stops unobserved syncing Co-Authored-By: Claude Opus 5.5 --- README.md | 3 ++- src/feature-flags/runtime-client.spec.ts | 34 ++++++++++++++++++++++++ src/feature-flags/runtime-client.ts | 16 +++++++---- 3 files changed, 47 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index b68d34beb..b12735521 100644 --- a/README.md +++ b/README.md @@ -163,7 +163,8 @@ polling populates the cache afterward. A failed poll, including one that returns a payload the SDK can't read, emits `error`, keeps the last good snapshot, and retries with backoff. A 401 stops polling and also emits `failed`. Attach an `error` listener or a `logger` to see -failures; with neither, the client warns once and keeps polling. +failures; with neither, the client warns once and keeps polling, and warns again +if a 401 stops it. ## SDK Versioning diff --git a/src/feature-flags/runtime-client.spec.ts b/src/feature-flags/runtime-client.spec.ts index 3e246b61e..6b4d2a82c 100644 --- a/src/feature-flags/runtime-client.spec.ts +++ b/src/feature-flags/runtime-client.spec.ts @@ -614,6 +614,13 @@ describe('FeatureFlagsRuntimeClient', () => { expect(readyError).toBeInstanceOf(UnauthorizedException); expect(failures).toHaveLength(1); expect(fetch.mock.calls.length).toBe(1); + expect(emitWarning).toHaveBeenCalledTimes(1); + expect(emitWarning).toHaveBeenCalledWith( + expect.stringContaining('stopped after a 401'), + ); + expect(emitWarning).not.toHaveBeenCalledWith( + expect.stringContaining('retrying'), + ); client.close(); emitWarning.mockRestore(); @@ -640,6 +647,33 @@ describe('FeatureFlagsRuntimeClient', () => { emitWarning.mockRestore(); }); + it('warns about a 401 after an earlier unobserved failure', async () => { + const emitWarning = jest + .spyOn(workos, 'emitWarning') + .mockImplementation(() => {}); + fetch.mockRejectOnce(new Error('Network error')); + fetchOnce( + { message: 'Unauthorized' }, + { status: 401, headers: { 'X-Request-ID': 'req_123' } }, + ); + + const client = workos.featureFlags.createRuntimeClient(); + await jest.advanceTimersByTimeAsync(0); + await jest.advanceTimersByTimeAsync(35_000); + + expect(emitWarning).toHaveBeenCalledTimes(2); + expect(emitWarning).toHaveBeenLastCalledWith( + expect.stringContaining('stopped after a 401'), + ); + + fetchOnce(pollResponse); + await jest.advanceTimersByTimeAsync(60_000); + expect(fetch.mock.calls.length).toBe(2); + + client.close(); + emitWarning.mockRestore(); + }); + it('logs instead of warning when a logger is attached', async () => { const emitWarning = jest .spyOn(workos, 'emitWarning') diff --git a/src/feature-flags/runtime-client.ts b/src/feature-flags/runtime-client.ts index 705f8b1d2..cd2a99c03 100644 --- a/src/feature-flags/runtime-client.ts +++ b/src/feature-flags/runtime-client.ts @@ -202,11 +202,17 @@ export class FeatureFlagsRuntimeClient extends EventEmitter // unhandled rejection, so an unobserved failure is reported instead. if (this.listenerCount('error') > 0) { this.emit('error', err); - } else if (!this.logger && !this.warnedUnobservedError) { - this.warnedUnobservedError = true; - this.workos.emitWarning( - `Feature flag sync failed and no 'error' listener or logger is attached; keeping the last good flags and retrying. ${err.message}`, - ); + } else if (!this.logger) { + if (err instanceof UnauthorizedException) { + this.workos.emitWarning( + `Feature flag sync stopped after a 401 and no 'error' listener or logger is attached; keeping the last good flags. Recreate the client with a valid API key to resume. ${err.message}`, + ); + } else if (!this.warnedUnobservedError) { + this.warnedUnobservedError = true; + this.workos.emitWarning( + `Feature flag sync failed and no 'error' listener or logger is attached; keeping the last good flags and retrying. ${err.message}`, + ); + } } this.logger?.error('Poll failed', err);