From 7b1cd98618ed9e6a45e2f31c5c56fd90a6ef58cf Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Wed, 11 Mar 2026 19:44:24 +0100 Subject: [PATCH 01/16] Handle browser teardown startup aborts --- packages/browser-sdk/src/client.ts | 11 ++- packages/browser-sdk/src/feedback/feedback.ts | 13 +++- packages/browser-sdk/src/flag/flags.ts | 3 +- .../browser-sdk/src/utils/pageLifecycle.ts | 70 +++++++++++++++++++ packages/browser-sdk/test/flags.test.ts | 17 +++++ packages/browser-sdk/test/usage.test.ts | 35 ++++++++++ 6 files changed, 143 insertions(+), 6 deletions(-) create mode 100644 packages/browser-sdk/src/utils/pageLifecycle.ts diff --git a/packages/browser-sdk/src/client.ts b/packages/browser-sdk/src/client.ts index 461c350e..ab4dde55 100644 --- a/packages/browser-sdk/src/client.ts +++ b/packages/browser-sdk/src/client.ts @@ -16,7 +16,6 @@ import { RawFlags, } from "./flag/flags"; import { ToolbarPosition } from "./ui/types"; -import { logResponseError } from "./utils/responseError"; import { BulkEvent, BulkQueue } from "./bulkQueue"; import { API_BASE_URL, @@ -30,6 +29,8 @@ import { HttpClient } from "./httpClient"; import { Logger, loggerWithPrefix, quietConsoleLogger } from "./logger"; import { StorageAdapter } from "./storage"; import { showToolbarToggle } from "./toolbar"; +import { logResponseError } from "./utils/responseError"; +import { logLifecycleAwareNetworkError } from "./utils/pageLifecycle"; const isMobile = typeof window !== "undefined" && window.innerWidth < 768; const isNode = typeof document === "undefined"; // deno supports "window" but not "document" according to https://remix.run/docs/en/main/guides/gotchas @@ -571,13 +572,17 @@ export class ReflagClient { if (!this.config.bootstrapped) { if (this.context.user && this.config.enableTracking) { this.user().catch((e) => { - this.logger.error("error sending user", e); + logLifecycleAwareNetworkError(this.logger, "error sending user", e); }); } if (this.context.company && this.config.enableTracking) { this.company().catch((e) => { - this.logger.error("error sending company", e); + logLifecycleAwareNetworkError( + this.logger, + "error sending company", + e, + ); }); } } diff --git a/packages/browser-sdk/src/feedback/feedback.ts b/packages/browser-sdk/src/feedback/feedback.ts index 53272d76..ad99e945 100644 --- a/packages/browser-sdk/src/feedback/feedback.ts +++ b/packages/browser-sdk/src/feedback/feedback.ts @@ -4,6 +4,7 @@ import { Logger } from "../logger"; import { AblySSEChannel, openAblySSEChannel } from "../sse"; import { Position } from "../ui/types"; import { logResponseError } from "../utils/responseError"; +import { logLifecycleAwareNetworkError } from "../utils/pageLifecycle"; import { FeedbackSubmission, @@ -316,7 +317,11 @@ export class AutoFeedback { }); this.logger.debug(`automatic feedback connection established`); } catch (e) { - this.logger.error(`error initializing automatic feedback client`, e); + logLifecycleAwareNetworkError( + this.logger, + `error initializing automatic feedback client`, + e, + ); } } @@ -518,7 +523,11 @@ export class AutoFeedback { } } } catch (e) { - this.logger.error(`error initializing automatic feedback`, e); + logLifecycleAwareNetworkError( + this.logger, + `error initializing automatic feedback`, + e, + ); return; } return; diff --git a/packages/browser-sdk/src/flag/flags.ts b/packages/browser-sdk/src/flag/flags.ts index 0a131c46..1934a6a4 100644 --- a/packages/browser-sdk/src/flag/flags.ts +++ b/packages/browser-sdk/src/flag/flags.ts @@ -10,6 +10,7 @@ import { getDefaultStorageAdapter, StorageAdapter } from "../storage"; import { createAbortController } from "../utils/abortController"; import { createEventTarget } from "../utils/eventTarget"; import { logResponseError, parseResponseError } from "../utils/responseError"; +import { logLifecycleAwareNetworkError } from "../utils/pageLifecycle"; import { FlagCache, isObject, parseAPIFlagsResponse } from "./flagCache"; @@ -437,7 +438,7 @@ export class FlagsClient { return typeRes.flags; } catch (e) { - this.logger.error("error fetching flags: ", e); + logLifecycleAwareNetworkError(this.logger, "error fetching flags:", e); return; } } diff --git a/packages/browser-sdk/src/utils/pageLifecycle.ts b/packages/browser-sdk/src/utils/pageLifecycle.ts new file mode 100644 index 00000000..e6b54a45 --- /dev/null +++ b/packages/browser-sdk/src/utils/pageLifecycle.ts @@ -0,0 +1,70 @@ +import { Logger } from "../logger"; + +let pageIsTearingDown = false; +let listenersRegistered = false; + +function markPageActive() { + pageIsTearingDown = false; +} + +function markPageTearingDown() { + pageIsTearingDown = true; +} + +function ensureListenersRegistered() { + if ( + listenersRegistered || + typeof window === "undefined" || + typeof window.addEventListener !== "function" + ) { + return; + } + + window.addEventListener("pagehide", markPageTearingDown, { capture: true }); + window.addEventListener("beforeunload", markPageTearingDown, { + capture: true, + }); + window.addEventListener("pageshow", markPageActive, { capture: true }); + listenersRegistered = true; +} + +function isAbortLikeError(error: unknown) { + if ( + typeof DOMException !== "undefined" && + error instanceof DOMException && + error.name === "AbortError" + ) { + return true; + } + + if (!(error instanceof Error)) { + return false; + } + + return ( + error.name === "AbortError" || + error.message === "Failed to fetch" || + error.message === "Load failed" || + error.message === "Network request failed" + ); +} + +export function isPageLifecycleAbortError(error: unknown) { + ensureListenersRegistered(); + return pageIsTearingDown && isAbortLikeError(error); +} + +export function logLifecycleAwareNetworkError( + logger: Pick, + message: string, + error: unknown, +) { + if (isPageLifecycleAbortError(error)) { + logger.debug(`${message} (aborted during page teardown)`, error); + return; + } + + logger.error(message, error); +} + +ensureListenersRegistered(); diff --git a/packages/browser-sdk/test/flags.test.ts b/packages/browser-sdk/test/flags.test.ts index b531f353..d12883e2 100644 --- a/packages/browser-sdk/test/flags.test.ts +++ b/packages/browser-sdk/test/flags.test.ts @@ -12,6 +12,7 @@ import { testLogger } from "./testLogger"; beforeEach(() => { vi.useFakeTimers(); vi.resetAllMocks(); + window.dispatchEvent(new Event("pageshow")); }); afterAll(() => { @@ -196,6 +197,22 @@ describe("FlagsClient", () => { }); }); + test("downgrades page teardown fetch failures to debug logs", async () => { + const { newFlagsClient, httpClient } = flagsClientFactory(); + + vi.mocked(httpClient.get).mockRejectedValue(new TypeError("Failed to fetch")); + window.dispatchEvent(new Event("pagehide")); + + const flagsClient = newFlagsClient(); + await flagsClient.initialize(); + + expect(testLogger.error).not.toHaveBeenCalled(); + expect(testLogger.debug).toHaveBeenCalledWith( + "[Flags] error fetching flags: (aborted during page teardown)", + expect.any(TypeError), + ); + }); + test("caches response", async () => { const { newFlagsClient, httpClient } = flagsClientFactory(); diff --git a/packages/browser-sdk/test/usage.test.ts b/packages/browser-sdk/test/usage.test.ts index 274f311b..5da1b85b 100644 --- a/packages/browser-sdk/test/usage.test.ts +++ b/packages/browser-sdk/test/usage.test.ts @@ -46,6 +46,7 @@ window.innerWidth = 1024; afterEach(() => { server.resetHandlers(); + window.dispatchEvent(new Event("pageshow")); }); beforeEach(() => { @@ -201,6 +202,40 @@ describe("feedback prompting", () => { expect(openAblySSEChannel).toBeCalledTimes(0); }); + + test("downgrades prompting init fetch failures during page teardown", async () => { + const logger = { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }; + const post = vi + .spyOn(HttpClient.prototype, "post") + .mockRejectedValueOnce(new TypeError("Failed to fetch")); + + window.dispatchEvent(new Event("pagehide")); + + try { + const reflagInstance = new ReflagClient({ + publishableKey: KEY, + user: { id: "foo" }, + enableTracking: false, + logger, + }); + await reflagInstance.initialize(); + + expect(post).toHaveBeenCalled(); + expect(logger.error).not.toHaveBeenCalled(); + expect(logger.debug).toHaveBeenCalledWith( + "error initializing automatic feedback (aborted during page teardown)", + expect.any(TypeError), + ); + expect(openAblySSEChannel).toBeCalledTimes(0); + } finally { + post.mockRestore(); + } + }); }); describe("feedback state management", () => { From 59c96142bfaa6605c04bcdcd0cff391ff1e36168 Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Wed, 11 Mar 2026 20:26:31 +0100 Subject: [PATCH 02/16] Refine teardown handling for bulk queue --- packages/browser-sdk/src/bulkQueue.ts | 15 +++++-- packages/browser-sdk/src/client.ts | 11 ++--- packages/browser-sdk/src/feedback/feedback.ts | 2 +- packages/browser-sdk/src/flag/flags.ts | 2 +- packages/browser-sdk/test/bulkQueue.test.ts | 45 +++++++++++++++++++ packages/browser-sdk/test/flags.test.ts | 4 +- 6 files changed, 65 insertions(+), 14 deletions(-) diff --git a/packages/browser-sdk/src/bulkQueue.ts b/packages/browser-sdk/src/bulkQueue.ts index 0d83c611..1c0ebed9 100644 --- a/packages/browser-sdk/src/bulkQueue.ts +++ b/packages/browser-sdk/src/bulkQueue.ts @@ -6,6 +6,7 @@ import { BULK_QUEUE_RETRY_MAX_DELAY_MS, } from "./config"; import { Logger } from "./logger"; +import { isPageLifecycleAbortError } from "./utils/pageLifecycle"; const BULK_QUEUE_STORAGE_KEY = "__reflag_bulk_queue_v1"; const WARN_AFTER_CONSECUTIVE_FAILURES = 10; @@ -290,11 +291,19 @@ export class BulkQueue { this.retryCount += 1; const retryInMs = this.getRetryDelay(); nextDelayMs = retryInMs; - this.logger?.info("bulk retry scheduled", { + const logDetails = { retryInMs, queueSize: this.queue.length + this.getInFlightBatchSize(), consecutiveFailures: this.consecutiveFailures, - }); + }; + if (isPageLifecycleAbortError(error)) { + this.logger?.debug("bulk retry scheduled (aborted during page teardown)", { + ...logDetails, + error, + }); + } else { + this.logger?.info("bulk retry scheduled", logDetails); + } const outageMs = now - this.firstFailureAt; const shouldWarn = @@ -302,7 +311,7 @@ export class BulkQueue { outageMs >= WARN_AFTER_FAILURE_MS; const canWarnNow = this.lastWarnAt === null || now - this.lastWarnAt >= WARN_THROTTLE_MS; - if (shouldWarn && canWarnNow) { + if (shouldWarn && canWarnNow && !isPageLifecycleAbortError(error)) { this.logger?.warn("bulk delivery degraded", { consecutiveFailures: this.consecutiveFailures, outageMs, diff --git a/packages/browser-sdk/src/client.ts b/packages/browser-sdk/src/client.ts index ab4dde55..461c350e 100644 --- a/packages/browser-sdk/src/client.ts +++ b/packages/browser-sdk/src/client.ts @@ -16,6 +16,7 @@ import { RawFlags, } from "./flag/flags"; import { ToolbarPosition } from "./ui/types"; +import { logResponseError } from "./utils/responseError"; import { BulkEvent, BulkQueue } from "./bulkQueue"; import { API_BASE_URL, @@ -29,8 +30,6 @@ import { HttpClient } from "./httpClient"; import { Logger, loggerWithPrefix, quietConsoleLogger } from "./logger"; import { StorageAdapter } from "./storage"; import { showToolbarToggle } from "./toolbar"; -import { logResponseError } from "./utils/responseError"; -import { logLifecycleAwareNetworkError } from "./utils/pageLifecycle"; const isMobile = typeof window !== "undefined" && window.innerWidth < 768; const isNode = typeof document === "undefined"; // deno supports "window" but not "document" according to https://remix.run/docs/en/main/guides/gotchas @@ -572,17 +571,13 @@ export class ReflagClient { if (!this.config.bootstrapped) { if (this.context.user && this.config.enableTracking) { this.user().catch((e) => { - logLifecycleAwareNetworkError(this.logger, "error sending user", e); + this.logger.error("error sending user", e); }); } if (this.context.company && this.config.enableTracking) { this.company().catch((e) => { - logLifecycleAwareNetworkError( - this.logger, - "error sending company", - e, - ); + this.logger.error("error sending company", e); }); } } diff --git a/packages/browser-sdk/src/feedback/feedback.ts b/packages/browser-sdk/src/feedback/feedback.ts index ad99e945..3d404c3d 100644 --- a/packages/browser-sdk/src/feedback/feedback.ts +++ b/packages/browser-sdk/src/feedback/feedback.ts @@ -3,8 +3,8 @@ import { HttpClient } from "../httpClient"; import { Logger } from "../logger"; import { AblySSEChannel, openAblySSEChannel } from "../sse"; import { Position } from "../ui/types"; -import { logResponseError } from "../utils/responseError"; import { logLifecycleAwareNetworkError } from "../utils/pageLifecycle"; +import { logResponseError } from "../utils/responseError"; import { FeedbackSubmission, diff --git a/packages/browser-sdk/src/flag/flags.ts b/packages/browser-sdk/src/flag/flags.ts index 1934a6a4..3507794b 100644 --- a/packages/browser-sdk/src/flag/flags.ts +++ b/packages/browser-sdk/src/flag/flags.ts @@ -9,8 +9,8 @@ import RateLimiter from "../rateLimiter"; import { getDefaultStorageAdapter, StorageAdapter } from "../storage"; import { createAbortController } from "../utils/abortController"; import { createEventTarget } from "../utils/eventTarget"; -import { logResponseError, parseResponseError } from "../utils/responseError"; import { logLifecycleAwareNetworkError } from "../utils/pageLifecycle"; +import { logResponseError, parseResponseError } from "../utils/responseError"; import { FlagCache, isObject, parseAPIFlagsResponse } from "./flagCache"; diff --git a/packages/browser-sdk/test/bulkQueue.test.ts b/packages/browser-sdk/test/bulkQueue.test.ts index e3006297..4507546f 100644 --- a/packages/browser-sdk/test/bulkQueue.test.ts +++ b/packages/browser-sdk/test/bulkQueue.test.ts @@ -88,6 +88,51 @@ describe("BulkQueue", () => { expect(sendBulk).toHaveBeenNthCalledWith(2, [trackEvent]); }); + it("requeues and downgrades page teardown aborts for bulk sends", async () => { + const logger = { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }; + const sendBulk = vi + .fn<(events: BulkEvent[]) => Promise>() + .mockRejectedValueOnce(new TypeError("Failed to fetch")) + .mockResolvedValue(new Response("", { status: 200 })); + const queue = new BulkQueue(sendBulk, { + flushDelayMs: 10, + retryBaseDelayMs: 20, + retryMaxDelayMs: 20, + logger, + }); + + window.dispatchEvent(new Event("pagehide")); + await queue.enqueue(trackEvent); + + await vi.advanceTimersByTimeAsync(10); + expect(sendBulk).toHaveBeenCalledTimes(1); + expect(await queue.size()).toBe(1); + expect(logger.debug).toHaveBeenCalledWith( + "bulk retry scheduled (aborted during page teardown)", + expect.objectContaining({ + retryInMs: 20, + queueSize: 2, + consecutiveFailures: 1, + error: expect.any(TypeError), + }), + ); + expect(logger.info).not.toHaveBeenCalledWith( + "bulk retry scheduled", + expect.anything(), + ); + expect(logger.warn).not.toHaveBeenCalled(); + + window.dispatchEvent(new Event("pageshow")); + await vi.advanceTimersByTimeAsync(20); + expect(sendBulk).toHaveBeenCalledTimes(2); + expect(sendBulk).toHaveBeenNthCalledWith(2, [trackEvent]); + }); + it("drops 4xx responses, logs error, and does not retry", async () => { const sendBulk = vi .fn<(events: BulkEvent[]) => Promise>() diff --git a/packages/browser-sdk/test/flags.test.ts b/packages/browser-sdk/test/flags.test.ts index d12883e2..ba441251 100644 --- a/packages/browser-sdk/test/flags.test.ts +++ b/packages/browser-sdk/test/flags.test.ts @@ -200,7 +200,9 @@ describe("FlagsClient", () => { test("downgrades page teardown fetch failures to debug logs", async () => { const { newFlagsClient, httpClient } = flagsClientFactory(); - vi.mocked(httpClient.get).mockRejectedValue(new TypeError("Failed to fetch")); + vi.mocked(httpClient.get).mockRejectedValue( + new TypeError("Failed to fetch"), + ); window.dispatchEvent(new Event("pagehide")); const flagsClient = newFlagsClient(); From 111282184631fb40e2e0bcf341fbd1c70513737a Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Wed, 11 Mar 2026 20:43:34 +0100 Subject: [PATCH 03/16] prettiery --- packages/browser-sdk/src/bulkQueue.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/browser-sdk/src/bulkQueue.ts b/packages/browser-sdk/src/bulkQueue.ts index 1c0ebed9..049266c4 100644 --- a/packages/browser-sdk/src/bulkQueue.ts +++ b/packages/browser-sdk/src/bulkQueue.ts @@ -297,10 +297,13 @@ export class BulkQueue { consecutiveFailures: this.consecutiveFailures, }; if (isPageLifecycleAbortError(error)) { - this.logger?.debug("bulk retry scheduled (aborted during page teardown)", { - ...logDetails, - error, - }); + this.logger?.debug( + "bulk retry scheduled (aborted during page teardown)", + { + ...logDetails, + error, + }, + ); } else { this.logger?.info("bulk retry scheduled", logDetails); } From 84e8130cf8b8f7714d9ec6f5b57df1ada3305c91 Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Wed, 11 Mar 2026 20:47:37 +0100 Subject: [PATCH 04/16] fix: bump versions --- packages/browser-sdk/package.json | 2 +- .../openfeature-browser-provider/package.json | 4 +- packages/react-sdk/package.json | 4 +- packages/vue-sdk/package.json | 4 +- yarn.lock | 37 ++++++++++++++++--- 5 files changed, 39 insertions(+), 12 deletions(-) diff --git a/packages/browser-sdk/package.json b/packages/browser-sdk/package.json index b559b900..59b0290e 100644 --- a/packages/browser-sdk/package.json +++ b/packages/browser-sdk/package.json @@ -1,6 +1,6 @@ { "name": "@reflag/browser-sdk", - "version": "1.4.4", + "version": "1.4.5", "packageManager": "yarn@4.1.1", "license": "MIT", "repository": { diff --git a/packages/openfeature-browser-provider/package.json b/packages/openfeature-browser-provider/package.json index 1b9f4b15..df73f3d9 100644 --- a/packages/openfeature-browser-provider/package.json +++ b/packages/openfeature-browser-provider/package.json @@ -1,6 +1,6 @@ { "name": "@reflag/openfeature-browser-provider", - "version": "1.3.2", + "version": "1.3.3", "packageManager": "yarn@4.1.1", "license": "MIT", "repository": { @@ -35,7 +35,7 @@ } }, "dependencies": { - "@reflag/browser-sdk": "1.4.4" + "@reflag/browser-sdk": "1.4.5" }, "devDependencies": { "@openfeature/core": "1.5.0", diff --git a/packages/react-sdk/package.json b/packages/react-sdk/package.json index 2a6575a5..487b53fd 100644 --- a/packages/react-sdk/package.json +++ b/packages/react-sdk/package.json @@ -1,6 +1,6 @@ { "name": "@reflag/react-sdk", - "version": "1.4.4", + "version": "1.4.5", "license": "MIT", "repository": { "type": "git", @@ -37,7 +37,7 @@ } }, "dependencies": { - "@reflag/browser-sdk": "1.4.4" + "@reflag/browser-sdk": "1.4.5" }, "peerDependencies": { "react": "*", diff --git a/packages/vue-sdk/package.json b/packages/vue-sdk/package.json index fb39999b..d891ca78 100644 --- a/packages/vue-sdk/package.json +++ b/packages/vue-sdk/package.json @@ -1,6 +1,6 @@ { "name": "@reflag/vue-sdk", - "version": "1.3.2", + "version": "1.3.3", "license": "MIT", "repository": { "type": "git", @@ -35,7 +35,7 @@ } }, "dependencies": { - "@reflag/browser-sdk": "1.4.4" + "@reflag/browser-sdk": "1.4.5" }, "peerDependencies": { "vue": "^3.0.0" diff --git a/yarn.lock b/yarn.lock index b8d20bfa..39ab0f2c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5676,7 +5676,19 @@ __metadata: languageName: node linkType: hard -"@reflag/browser-sdk@npm:1.4.4, @reflag/browser-sdk@workspace:packages/browser-sdk": +"@reflag/browser-sdk@npm:1.4.3": + version: 1.4.3 + resolution: "@reflag/browser-sdk@npm:1.4.3" + dependencies: + "@floating-ui/dom": "npm:^1.6.8" + fast-equals: "npm:^5.2.2" + js-cookie: "npm:^3.0.5" + preact: "npm:^10.22.1" + checksum: 10c0/86d4799d9cf7dbdf1cd772a494e0ec153f14119bd252ff6ac60978003a528af5468804514dd365b300a44e0c1d4b8561451d9b23d090b050ee58980f08695e6b + languageName: node + linkType: hard + +"@reflag/browser-sdk@npm:1.4.5, @reflag/browser-sdk@workspace:packages/browser-sdk": version: 0.0.0-use.local resolution: "@reflag/browser-sdk@workspace:packages/browser-sdk" dependencies: @@ -5817,7 +5829,7 @@ __metadata: dependencies: "@openfeature/core": "npm:1.5.0" "@openfeature/web-sdk": "npm:^1.3.0" - "@reflag/browser-sdk": "npm:1.4.4" + "@reflag/browser-sdk": "npm:1.4.5" "@reflag/eslint-config": "npm:0.0.2" "@reflag/tsconfig": "npm:0.0.2" "@types/node": "npm:^22.12.0" @@ -5875,11 +5887,26 @@ __metadata: languageName: unknown linkType: soft -"@reflag/react-sdk@npm:1.4.4, @reflag/react-sdk@workspace:^, @reflag/react-sdk@workspace:packages/react-sdk": +"@reflag/react-sdk@npm:1.4.4": + version: 1.4.4 + resolution: "@reflag/react-sdk@npm:1.4.4" + dependencies: + "@reflag/browser-sdk": "npm:1.4.3" + peerDependencies: + react: "*" + react-dom: "*" + peerDependenciesMeta: + react-dom: + optional: true + checksum: 10c0/d87c9918444e69b872c26db34413721f8d2c7003fa201dad5c95bf0a69397f77d621c78180098a2b6031556a1ef1cfefbf37775dc0e128c5bcf92a1080e3ee6d + languageName: node + linkType: hard + +"@reflag/react-sdk@workspace:^, @reflag/react-sdk@workspace:packages/react-sdk": version: 0.0.0-use.local resolution: "@reflag/react-sdk@workspace:packages/react-sdk" dependencies: - "@reflag/browser-sdk": "npm:1.4.4" + "@reflag/browser-sdk": "npm:1.4.5" "@reflag/eslint-config": "npm:^0.0.2" "@reflag/tsconfig": "npm:^0.0.2" "@testing-library/react": "npm:^15.0.7" @@ -5939,7 +5966,7 @@ __metadata: version: 0.0.0-use.local resolution: "@reflag/vue-sdk@workspace:packages/vue-sdk" dependencies: - "@reflag/browser-sdk": "npm:1.4.4" + "@reflag/browser-sdk": "npm:1.4.5" "@reflag/eslint-config": "npm:^0.0.2" "@reflag/tsconfig": "npm:^0.0.2" "@types/jsdom": "npm:^21.1.6" From 81c26ce7daff159a2f7b0b2608ff41b8c0d4f835 Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Wed, 11 Mar 2026 20:59:46 +0100 Subject: [PATCH 05/16] Add E2E coverage for startup aborts --- packages/browser-sdk/src/bulkQueue.ts | 2 +- .../test/e2e/acceptance.browser.spec.ts | 77 +++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/packages/browser-sdk/src/bulkQueue.ts b/packages/browser-sdk/src/bulkQueue.ts index 049266c4..c70070ef 100644 --- a/packages/browser-sdk/src/bulkQueue.ts +++ b/packages/browser-sdk/src/bulkQueue.ts @@ -1,3 +1,4 @@ +import { isPageLifecycleAbortError } from "./utils/pageLifecycle"; import { logResponseError } from "./utils/responseError"; import { BULK_QUEUE_FLUSH_DELAY_MS, @@ -6,7 +7,6 @@ import { BULK_QUEUE_RETRY_MAX_DELAY_MS, } from "./config"; import { Logger } from "./logger"; -import { isPageLifecycleAbortError } from "./utils/pageLifecycle"; const BULK_QUEUE_STORAGE_KEY = "__reflag_bulk_queue_v1"; const WARN_AFTER_CONSECUTIVE_FAILURES = 10; diff --git a/packages/browser-sdk/test/e2e/acceptance.browser.spec.ts b/packages/browser-sdk/test/e2e/acceptance.browser.spec.ts index 92b51f49..44f28673 100644 --- a/packages/browser-sdk/test/e2e/acceptance.browser.spec.ts +++ b/packages/browser-sdk/test/e2e/acceptance.browser.spec.ts @@ -121,3 +121,80 @@ test("Acceptance", async ({ page }) => { expect(successfulRequests).toContain("EVENT"); expect(successfulRequests).toContain("FEEDBACK"); }); + +test("does not log startup fetch aborts as errors during fast full navigation", async ({ + page, +}) => { + await page.goto("http://localhost:8001/test/e2e/empty.html"); + + const consoleErrors: string[] = []; + page.on("console", (msg) => { + if (msg.type() === "error") { + consoleErrors.push(msg.text()); + } + }); + + await page.route(`${API_BASE_URL}/features/evaluated*`, async (route) => { + await new Promise((resolve) => setTimeout(resolve, 2_000)); + try { + await route.fulfill({ + status: 200, + body: JSON.stringify({ + success: true, + features: {}, + }), + }); + } catch { + // The original document may have been torn down before the response. + } + }); + + await page.route(`${API_BASE_URL}/feedback/prompting-init`, async (route) => { + await new Promise((resolve) => setTimeout(resolve, 2_000)); + try { + await route.fulfill({ + status: 200, + body: JSON.stringify({ success: false }), + }); + } catch { + // The original document may have been torn down before the response. + } + }); + + await page.route(`${API_BASE_URL}/bulk`, async (route) => { + await new Promise((resolve) => setTimeout(resolve, 2_000)); + try { + await route.fulfill({ + status: 200, + body: JSON.stringify({ success: true }), + }); + } catch { + // The original document may have been torn down before the response. + } + }); + + await page.evaluate(` + ;(async () => { + const { ReflagClient } = await import("/dist/reflag-browser-sdk.mjs"); + const reflagClient = new ReflagClient({ + publishableKey: "${KEY}", + user: { id: "foo" }, + company: { id: "bar" }, + }); + void reflagClient.initialize(); + })() + `); + + await page.goto("http://localhost:8001/test/e2e/give-feedback-button.html"); + await page.waitForTimeout(250); + + expect(consoleErrors).not.toContainEqual( + expect.stringContaining("error fetching flags"), + ); + expect(consoleErrors).not.toContainEqual( + expect.stringContaining("error initializing automatic feedback"), + ); + expect(consoleErrors).not.toContainEqual( + expect.stringContaining("bulk delivery degraded"), + ); +}); From 9713afb1129a36165880aa996574d4391ccc20f6 Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Wed, 11 Mar 2026 21:07:32 +0100 Subject: [PATCH 06/16] Strengthen startup abort E2E --- .../test/e2e/acceptance.browser.spec.ts | 73 +++++++++++++------ 1 file changed, 49 insertions(+), 24 deletions(-) diff --git a/packages/browser-sdk/test/e2e/acceptance.browser.spec.ts b/packages/browser-sdk/test/e2e/acceptance.browser.spec.ts index 44f28673..e1291863 100644 --- a/packages/browser-sdk/test/e2e/acceptance.browser.spec.ts +++ b/packages/browser-sdk/test/e2e/acceptance.browser.spec.ts @@ -134,9 +134,38 @@ test("does not log startup fetch aborts as errors during fast full navigation", } }); + let flagsStarted: () => void; + const flagsStartedPromise = new Promise((resolve) => { + flagsStarted = resolve; + }); + let promptingInitStarted: () => void; + const promptingInitStartedPromise = new Promise((resolve) => { + promptingInitStarted = resolve; + }); + + let releaseFlags: () => void; + const releaseFlagsPromise = new Promise((resolve) => { + releaseFlags = resolve; + }); + let releasePromptingInit: () => void; + const releasePromptingInitPromise = new Promise((resolve) => { + releasePromptingInit = resolve; + }); + + let flagsRequestCount = 0; + let promptingInitRequestCount = 0; + await page.route(`${API_BASE_URL}/features/evaluated*`, async (route) => { - await new Promise((resolve) => setTimeout(resolve, 2_000)); - try { + flagsRequestCount += 1; + if (flagsRequestCount === 1) { + flagsStarted(); + await releaseFlagsPromise; + try { + await route.abort("failed"); + } catch { + // The original document may already be gone. + } + } else { await route.fulfill({ status: 200, body: JSON.stringify({ @@ -144,32 +173,24 @@ test("does not log startup fetch aborts as errors during fast full navigation", features: {}, }), }); - } catch { - // The original document may have been torn down before the response. } }); await page.route(`${API_BASE_URL}/feedback/prompting-init`, async (route) => { - await new Promise((resolve) => setTimeout(resolve, 2_000)); - try { + promptingInitRequestCount += 1; + if (promptingInitRequestCount === 1) { + promptingInitStarted(); + await releasePromptingInitPromise; + try { + await route.abort("failed"); + } catch { + // The original document may already be gone. + } + } else { await route.fulfill({ status: 200, body: JSON.stringify({ success: false }), }); - } catch { - // The original document may have been torn down before the response. - } - }); - - await page.route(`${API_BASE_URL}/bulk`, async (route) => { - await new Promise((resolve) => setTimeout(resolve, 2_000)); - try { - await route.fulfill({ - status: 200, - body: JSON.stringify({ success: true }), - }); - } catch { - // The original document may have been torn down before the response. } }); @@ -185,16 +206,20 @@ test("does not log startup fetch aborts as errors during fast full navigation", })() `); - await page.goto("http://localhost:8001/test/e2e/give-feedback-button.html"); + await Promise.all([flagsStartedPromise, promptingInitStartedPromise]); + + const navigation = page.goto("http://localhost:8001/test/e2e/give-feedback-button.html"); + releaseFlags(); + releasePromptingInit(); + await navigation; await page.waitForTimeout(250); + expect(flagsRequestCount).toBeGreaterThanOrEqual(1); + expect(promptingInitRequestCount).toBeGreaterThanOrEqual(1); expect(consoleErrors).not.toContainEqual( expect.stringContaining("error fetching flags"), ); expect(consoleErrors).not.toContainEqual( expect.stringContaining("error initializing automatic feedback"), ); - expect(consoleErrors).not.toContainEqual( - expect.stringContaining("bulk delivery degraded"), - ); }); From e6633f39790f815efd5a4b0aa828c0b27e25ca17 Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Thu, 12 Mar 2026 07:54:29 +0100 Subject: [PATCH 07/16] Refine teardown fetch error handling --- packages/browser-sdk/src/bulkQueue.ts | 105 ++++++++++++------ packages/browser-sdk/src/feedback/feedback.ts | 54 +++++---- packages/browser-sdk/src/flag/flags.ts | 36 ++++-- .../browser-sdk/src/utils/pageLifecycle.ts | 32 +----- packages/browser-sdk/test/bulkQueue.test.ts | 31 ++++++ packages/browser-sdk/test/flags.test.ts | 19 ++++ packages/browser-sdk/test/usage.test.ts | 35 ++++++ 7 files changed, 219 insertions(+), 93 deletions(-) diff --git a/packages/browser-sdk/src/bulkQueue.ts b/packages/browser-sdk/src/bulkQueue.ts index c70070ef..95073552 100644 --- a/packages/browser-sdk/src/bulkQueue.ts +++ b/packages/browser-sdk/src/bulkQueue.ts @@ -1,4 +1,4 @@ -import { isPageLifecycleAbortError } from "./utils/pageLifecycle"; +import { isPageTearingDown } from "./utils/pageLifecycle"; import { logResponseError } from "./utils/responseError"; import { BULK_QUEUE_FLUSH_DELAY_MS, @@ -247,39 +247,9 @@ export class BulkQueue { private async sendBatch(batch: BulkEvent[]) { let nextDelayMs: number | null = null; + let res: Response; try { - const res = await this.sendBulk(batch); - if (!res.ok) { - if (res.status >= 400 && res.status < 500) { - this.retryCount = 0; - this.firstFailureAt = null; - this.consecutiveFailures = 0; - this.lastWarnAt = null; - if (this.logger) { - await logResponseError({ - logger: this.logger, - res, - message: - "bulk request failed with non-retriable status; dropping batch", - }); - } - nextDelayMs = this.flushDelayMs; - } else { - throw new Error(`unexpected status ${res.status}`); - } - } else { - this.retryCount = 0; - if (this.firstFailureAt !== null && this.consecutiveFailures > 0) { - this.logger?.info("bulk delivery recovered", { - outageMs: Date.now() - this.firstFailureAt, - failedAttempts: this.consecutiveFailures, - }); - } - this.firstFailureAt = null; - this.consecutiveFailures = 0; - this.lastWarnAt = null; - nextDelayMs = this.flushDelayMs; - } + res = await this.sendBulk(batch); } catch (error) { this.queue = batch.concat(this.queue); @@ -296,7 +266,7 @@ export class BulkQueue { queueSize: this.queue.length + this.getInFlightBatchSize(), consecutiveFailures: this.consecutiveFailures, }; - if (isPageLifecycleAbortError(error)) { + if (isPageTearingDown()) { this.logger?.debug( "bulk retry scheduled (aborted during page teardown)", { @@ -314,7 +284,7 @@ export class BulkQueue { outageMs >= WARN_AFTER_FAILURE_MS; const canWarnNow = this.lastWarnAt === null || now - this.lastWarnAt >= WARN_THROTTLE_MS; - if (shouldWarn && canWarnNow && !isPageLifecycleAbortError(error)) { + if (shouldWarn && canWarnNow && !isPageTearingDown()) { this.logger?.warn("bulk delivery degraded", { consecutiveFailures: this.consecutiveFailures, outageMs, @@ -324,7 +294,72 @@ export class BulkQueue { }); this.lastWarnAt = now; } + return nextDelayMs; + } + + if (!res.ok) { + if (res.status >= 400 && res.status < 500) { + this.retryCount = 0; + this.firstFailureAt = null; + this.consecutiveFailures = 0; + this.lastWarnAt = null; + if (this.logger) { + await logResponseError({ + logger: this.logger, + res, + message: + "bulk request failed with non-retriable status; dropping batch", + }); + } + return this.flushDelayMs; + } + + this.queue = batch.concat(this.queue); + + const now = Date.now(); + if (this.firstFailureAt === null) { + this.firstFailureAt = now; + } + this.consecutiveFailures += 1; + this.retryCount += 1; + const retryInMs = this.getRetryDelay(); + nextDelayMs = retryInMs; + this.logger?.info("bulk retry scheduled", { + retryInMs, + queueSize: this.queue.length + this.getInFlightBatchSize(), + consecutiveFailures: this.consecutiveFailures, + }); + + const outageMs = now - this.firstFailureAt; + const shouldWarn = + this.consecutiveFailures >= WARN_AFTER_CONSECUTIVE_FAILURES || + outageMs >= WARN_AFTER_FAILURE_MS; + const canWarnNow = + this.lastWarnAt === null || now - this.lastWarnAt >= WARN_THROTTLE_MS; + if (shouldWarn && canWarnNow) { + this.logger?.warn("bulk delivery degraded", { + consecutiveFailures: this.consecutiveFailures, + outageMs, + queueSize: this.queue.length + this.getInFlightBatchSize(), + retryInMs, + error: new Error(`unexpected status ${res.status}`), + }); + this.lastWarnAt = now; + } + return nextDelayMs; + } + + this.retryCount = 0; + if (this.firstFailureAt !== null && this.consecutiveFailures > 0) { + this.logger?.info("bulk delivery recovered", { + outageMs: Date.now() - this.firstFailureAt, + failedAttempts: this.consecutiveFailures, + }); } + this.firstFailureAt = null; + this.consecutiveFailures = 0; + this.lastWarnAt = null; + nextDelayMs = this.flushDelayMs; return nextDelayMs; } diff --git a/packages/browser-sdk/src/feedback/feedback.ts b/packages/browser-sdk/src/feedback/feedback.ts index 3d404c3d..d286ca17 100644 --- a/packages/browser-sdk/src/feedback/feedback.ts +++ b/packages/browser-sdk/src/feedback/feedback.ts @@ -3,7 +3,7 @@ import { HttpClient } from "../httpClient"; import { Logger } from "../logger"; import { AblySSEChannel, openAblySSEChannel } from "../sse"; import { Position } from "../ui/types"; -import { logLifecycleAwareNetworkError } from "../utils/pageLifecycle"; +import { logLifecycleAwareFetchError } from "../utils/pageLifecycle"; import { logResponseError } from "../utils/responseError"; import { @@ -317,11 +317,7 @@ export class AutoFeedback { }); this.logger.debug(`automatic feedback connection established`); } catch (e) { - logLifecycleAwareNetworkError( - this.logger, - `error initializing automatic feedback client`, - e, - ); + this.logger.error(`error initializing automatic feedback client`, e); } } @@ -498,32 +494,48 @@ export class AutoFeedback { return channel; } + let res: Response; try { - if (!channel) { - const res = await this.httpClient.post({ - path: `/feedback/prompting-init`, - body: { - userId: this.userId, - }, - }); + res = await this.httpClient.post({ + path: `/feedback/prompting-init`, + body: { + userId: this.userId, + }, + }); + } catch (e) { + logLifecycleAwareFetchError( + this.logger, + `error initializing automatic feedback`, + e, + ); + return; + } - this.logger.debug(`automatic feedback status sent`, res); - if (!res.ok) { + try { + this.logger.debug(`automatic feedback status sent`, res); + if (!res.ok) { + try { await logResponseError({ logger: this.logger, res, message: "automatic feedback init request failed", }); - return; + } catch (e) { + logLifecycleAwareFetchError( + this.logger, + `error initializing automatic feedback`, + e, + ); } + return; + } - const body: { success: boolean; channel?: string } = await res.json(); - if (body.success && body.channel) { - return body.channel; - } + const body: { success: boolean; channel?: string } = await res.json(); + if (body.success && body.channel) { + return body.channel; } } catch (e) { - logLifecycleAwareNetworkError( + logLifecycleAwareFetchError( this.logger, `error initializing automatic feedback`, e, diff --git a/packages/browser-sdk/src/flag/flags.ts b/packages/browser-sdk/src/flag/flags.ts index 3507794b..31651ee6 100644 --- a/packages/browser-sdk/src/flag/flags.ts +++ b/packages/browser-sdk/src/flag/flags.ts @@ -9,7 +9,7 @@ import RateLimiter from "../rateLimiter"; import { getDefaultStorageAdapter, StorageAdapter } from "../storage"; import { createAbortController } from "../utils/abortController"; import { createEventTarget } from "../utils/eventTarget"; -import { logLifecycleAwareNetworkError } from "../utils/pageLifecycle"; +import { logLifecycleAwareFetchError } from "../utils/pageLifecycle"; import { logResponseError, parseResponseError } from "../utils/responseError"; import { FlagCache, isObject, parseAPIFlagsResponse } from "./flagCache"; @@ -411,34 +411,52 @@ export class FlagsClient { async fetchFlags(): Promise { const params = this.fetchParams(); + let res: Response; try { - const res = await this.httpClient.get({ + res = await this.httpClient.get({ path: "/features/evaluated", timeoutMs: this.config.timeoutMs, params, }); + } catch (e) { + logLifecycleAwareFetchError(this.logger, "error fetching flags:", e); + return; + } - if (!res.ok) { + if (!res.ok) { + try { const { errorDetails, errorSummary } = await parseResponseError(res); const fallbackBody = errorDetails.responseBody ? ` - ${errorDetails.responseBody}` : ""; - throw new Error( - `unexpected response code: ${res.status}${ - errorSummary ? ` - ${errorSummary}` : fallbackBody - }`, + this.logger.error( + "error fetching flags:", + new Error( + `unexpected response code: ${res.status}${ + errorSummary ? ` - ${errorSummary}` : fallbackBody + }`, + ), ); + } catch (e) { + logLifecycleAwareFetchError(this.logger, "error fetching flags:", e); } + return; + } + try { const typeRes = validateFlagsResponse(await res.json()); if (!typeRes || !typeRes.success) { - throw new Error("unable to validate response"); + this.logger.error( + "error fetching flags:", + new Error("unable to validate response"), + ); + return; } return typeRes.flags; } catch (e) { - logLifecycleAwareNetworkError(this.logger, "error fetching flags:", e); + logLifecycleAwareFetchError(this.logger, "error fetching flags:", e); return; } } diff --git a/packages/browser-sdk/src/utils/pageLifecycle.ts b/packages/browser-sdk/src/utils/pageLifecycle.ts index e6b54a45..5a03b03c 100644 --- a/packages/browser-sdk/src/utils/pageLifecycle.ts +++ b/packages/browser-sdk/src/utils/pageLifecycle.ts @@ -21,45 +21,21 @@ function ensureListenersRegistered() { } window.addEventListener("pagehide", markPageTearingDown, { capture: true }); - window.addEventListener("beforeunload", markPageTearingDown, { - capture: true, - }); window.addEventListener("pageshow", markPageActive, { capture: true }); listenersRegistered = true; } -function isAbortLikeError(error: unknown) { - if ( - typeof DOMException !== "undefined" && - error instanceof DOMException && - error.name === "AbortError" - ) { - return true; - } - - if (!(error instanceof Error)) { - return false; - } - - return ( - error.name === "AbortError" || - error.message === "Failed to fetch" || - error.message === "Load failed" || - error.message === "Network request failed" - ); -} - -export function isPageLifecycleAbortError(error: unknown) { +export function isPageTearingDown() { ensureListenersRegistered(); - return pageIsTearingDown && isAbortLikeError(error); + return pageIsTearingDown; } -export function logLifecycleAwareNetworkError( +export function logLifecycleAwareFetchError( logger: Pick, message: string, error: unknown, ) { - if (isPageLifecycleAbortError(error)) { + if (isPageTearingDown()) { logger.debug(`${message} (aborted during page teardown)`, error); return; } diff --git a/packages/browser-sdk/test/bulkQueue.test.ts b/packages/browser-sdk/test/bulkQueue.test.ts index 4507546f..b5f0d715 100644 --- a/packages/browser-sdk/test/bulkQueue.test.ts +++ b/packages/browser-sdk/test/bulkQueue.test.ts @@ -35,6 +35,7 @@ describe("BulkQueue", () => { beforeEach(() => { vi.useFakeTimers(); sessionStorage.clear(); + window.dispatchEvent(new Event("pageshow")); }); afterEach(() => { @@ -133,6 +134,36 @@ describe("BulkQueue", () => { expect(sendBulk).toHaveBeenNthCalledWith(2, [trackEvent]); }); + it("keeps 5xx bulk retries on the normal degraded-delivery path during teardown", async () => { + const logger = { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }; + const sendBulk = vi + .fn<(events: BulkEvent[]) => Promise>() + .mockResolvedValue(new Response("", { status: 500 })); + const queue = new BulkQueue(sendBulk, { + flushDelayMs: 10, + retryBaseDelayMs: 20, + retryMaxDelayMs: 20, + logger, + }); + + window.dispatchEvent(new Event("pagehide")); + await queue.enqueue(trackEvent); + + await vi.advanceTimersByTimeAsync(10); + expect(sendBulk).toHaveBeenCalledTimes(1); + expect(logger.debug).not.toHaveBeenCalled(); + expect(logger.info).toHaveBeenCalledWith("bulk retry scheduled", { + retryInMs: 20, + queueSize: 2, + consecutiveFailures: 1, + }); + }); + it("drops 4xx responses, logs error, and does not retry", async () => { const sendBulk = vi .fn<(events: BulkEvent[]) => Promise>() diff --git a/packages/browser-sdk/test/flags.test.ts b/packages/browser-sdk/test/flags.test.ts index ba441251..e33f45cb 100644 --- a/packages/browser-sdk/test/flags.test.ts +++ b/packages/browser-sdk/test/flags.test.ts @@ -215,6 +215,25 @@ describe("FlagsClient", () => { ); }); + test("downgrades page teardown body-read failures to debug logs", async () => { + const { newFlagsClient, httpClient } = flagsClientFactory(); + + vi.mocked(httpClient.get).mockResolvedValue({ + ok: true, + json: vi.fn().mockRejectedValue(new TypeError("Failed to fetch")), + } as unknown as Response); + window.dispatchEvent(new Event("pagehide")); + + const flagsClient = newFlagsClient(); + await flagsClient.initialize(); + + expect(testLogger.error).not.toHaveBeenCalled(); + expect(testLogger.debug).toHaveBeenCalledWith( + "[Flags] error fetching flags: (aborted during page teardown)", + expect.any(TypeError), + ); + }); + test("caches response", async () => { const { newFlagsClient, httpClient } = flagsClientFactory(); diff --git a/packages/browser-sdk/test/usage.test.ts b/packages/browser-sdk/test/usage.test.ts index 5da1b85b..15d9e565 100644 --- a/packages/browser-sdk/test/usage.test.ts +++ b/packages/browser-sdk/test/usage.test.ts @@ -236,6 +236,41 @@ describe("feedback prompting", () => { post.mockRestore(); } }); + + test("downgrades prompting init body-read failures during page teardown", async () => { + const logger = { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }; + const post = vi.spyOn(HttpClient.prototype, "post").mockResolvedValueOnce({ + ok: true, + json: vi.fn().mockRejectedValue(new TypeError("Failed to fetch")), + } as unknown as Response); + + window.dispatchEvent(new Event("pagehide")); + + try { + const reflagInstance = new ReflagClient({ + publishableKey: KEY, + user: { id: "foo" }, + enableTracking: false, + logger, + }); + await reflagInstance.initialize(); + + expect(post).toHaveBeenCalled(); + expect(logger.error).not.toHaveBeenCalled(); + expect(logger.debug).toHaveBeenCalledWith( + "error initializing automatic feedback (aborted during page teardown)", + expect.any(TypeError), + ); + expect(openAblySSEChannel).toBeCalledTimes(0); + } finally { + post.mockRestore(); + } + }); }); describe("feedback state management", () => { From 94c275c458664b0bc75a0a198079ea6f6e04066a Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Thu, 12 Mar 2026 11:27:18 +0100 Subject: [PATCH 08/16] Retry startup fetch failures without unload detection --- packages/browser-sdk/src/bulkQueue.ts | 15 +-- packages/browser-sdk/src/feedback/feedback.ts | 74 ++++++------- packages/browser-sdk/src/flag/flags.ts | 88 +++++++-------- .../browser-sdk/src/utils/pageLifecycle.ts | 46 -------- packages/browser-sdk/src/utils/retry.ts | 31 ++++++ packages/browser-sdk/test/bulkQueue.test.ts | 50 +-------- .../test/e2e/acceptance.browser.spec.ts | 102 ------------------ packages/browser-sdk/test/flags.test.ts | 68 +++++++----- packages/browser-sdk/test/usage.test.ts | 62 ++++++----- 9 files changed, 190 insertions(+), 346 deletions(-) delete mode 100644 packages/browser-sdk/src/utils/pageLifecycle.ts create mode 100644 packages/browser-sdk/src/utils/retry.ts diff --git a/packages/browser-sdk/src/bulkQueue.ts b/packages/browser-sdk/src/bulkQueue.ts index 95073552..7572a564 100644 --- a/packages/browser-sdk/src/bulkQueue.ts +++ b/packages/browser-sdk/src/bulkQueue.ts @@ -1,4 +1,3 @@ -import { isPageTearingDown } from "./utils/pageLifecycle"; import { logResponseError } from "./utils/responseError"; import { BULK_QUEUE_FLUSH_DELAY_MS, @@ -266,17 +265,7 @@ export class BulkQueue { queueSize: this.queue.length + this.getInFlightBatchSize(), consecutiveFailures: this.consecutiveFailures, }; - if (isPageTearingDown()) { - this.logger?.debug( - "bulk retry scheduled (aborted during page teardown)", - { - ...logDetails, - error, - }, - ); - } else { - this.logger?.info("bulk retry scheduled", logDetails); - } + this.logger?.info("bulk retry scheduled", logDetails); const outageMs = now - this.firstFailureAt; const shouldWarn = @@ -284,7 +273,7 @@ export class BulkQueue { outageMs >= WARN_AFTER_FAILURE_MS; const canWarnNow = this.lastWarnAt === null || now - this.lastWarnAt >= WARN_THROTTLE_MS; - if (shouldWarn && canWarnNow && !isPageTearingDown()) { + if (shouldWarn && canWarnNow) { this.logger?.warn("bulk delivery degraded", { consecutiveFailures: this.consecutiveFailures, outageMs, diff --git a/packages/browser-sdk/src/feedback/feedback.ts b/packages/browser-sdk/src/feedback/feedback.ts index d286ca17..2b576f75 100644 --- a/packages/browser-sdk/src/feedback/feedback.ts +++ b/packages/browser-sdk/src/feedback/feedback.ts @@ -3,8 +3,8 @@ import { HttpClient } from "../httpClient"; import { Logger } from "../logger"; import { AblySSEChannel, openAblySSEChannel } from "../sse"; import { Position } from "../ui/types"; -import { logLifecycleAwareFetchError } from "../utils/pageLifecycle"; import { logResponseError } from "../utils/responseError"; +import { retryOnThrow } from "../utils/retry"; import { FeedbackSubmission, @@ -20,6 +20,8 @@ import { getAuthToken } from "./promptStorage"; import * as feedbackLib from "./ui"; import { DEFAULT_POSITION } from "./ui"; +const INITIAL_FETCH_RETRY_DELAYS_MS = [0, 5000]; + export type Key = string; export type FeedbackOptions = { @@ -494,54 +496,40 @@ export class AutoFeedback { return channel; } - let res: Response; try { - res = await this.httpClient.post({ - path: `/feedback/prompting-init`, - body: { - userId: this.userId, - }, - }); - } catch (e) { - logLifecycleAwareFetchError( - this.logger, - `error initializing automatic feedback`, - e, - ); - return; - } + return await retryOnThrow(INITIAL_FETCH_RETRY_DELAYS_MS, async () => { + const res = await this.httpClient.post({ + path: `/feedback/prompting-init`, + body: { + userId: this.userId, + }, + }); - try { - this.logger.debug(`automatic feedback status sent`, res); - if (!res.ok) { - try { - await logResponseError({ - logger: this.logger, - res, - message: "automatic feedback init request failed", - }); - } catch (e) { - logLifecycleAwareFetchError( - this.logger, - `error initializing automatic feedback`, - e, - ); + this.logger.debug(`automatic feedback status sent`, res); + if (!res.ok) { + try { + await logResponseError({ + logger: this.logger, + res, + message: "automatic feedback init request failed", + }); + } catch { + this.logger.error( + `error initializing automatic feedback`, + new Error(`unexpected response code: ${res.status}`), + ); + } + return; } - return; - } - const body: { success: boolean; channel?: string } = await res.json(); - if (body.success && body.channel) { - return body.channel; - } + const body: { success: boolean; channel?: string } = await res.json(); + if (body.success && body.channel) { + return body.channel; + } + }); } catch (e) { - logLifecycleAwareFetchError( - this.logger, - `error initializing automatic feedback`, - e, - ); + this.logger.error(`error initializing automatic feedback`, e); return; } - return; } } diff --git a/packages/browser-sdk/src/flag/flags.ts b/packages/browser-sdk/src/flag/flags.ts index 31651ee6..c00c6a9d 100644 --- a/packages/browser-sdk/src/flag/flags.ts +++ b/packages/browser-sdk/src/flag/flags.ts @@ -9,11 +9,13 @@ import RateLimiter from "../rateLimiter"; import { getDefaultStorageAdapter, StorageAdapter } from "../storage"; import { createAbortController } from "../utils/abortController"; import { createEventTarget } from "../utils/eventTarget"; -import { logLifecycleAwareFetchError } from "../utils/pageLifecycle"; import { logResponseError, parseResponseError } from "../utils/responseError"; +import { retryOnThrow } from "../utils/retry"; import { FlagCache, isObject, parseAPIFlagsResponse } from "./flagCache"; +const INITIAL_FETCH_RETRY_DELAYS_MS = [0, 5000]; + /** * A flag fetched from the server. */ @@ -411,52 +413,52 @@ export class FlagsClient { async fetchFlags(): Promise { const params = this.fetchParams(); - let res: Response; try { - res = await this.httpClient.get({ - path: "/features/evaluated", - timeoutMs: this.config.timeoutMs, - params, - }); - } catch (e) { - logLifecycleAwareFetchError(this.logger, "error fetching flags:", e); - return; - } - - if (!res.ok) { - try { - const { errorDetails, errorSummary } = await parseResponseError(res); - const fallbackBody = errorDetails.responseBody - ? ` - ${errorDetails.responseBody}` - : ""; - - this.logger.error( - "error fetching flags:", - new Error( - `unexpected response code: ${res.status}${ - errorSummary ? ` - ${errorSummary}` : fallbackBody - }`, - ), - ); - } catch (e) { - logLifecycleAwareFetchError(this.logger, "error fetching flags:", e); - } - return; - } + return await retryOnThrow(INITIAL_FETCH_RETRY_DELAYS_MS, async () => { + const res = await this.httpClient.get({ + path: "/features/evaluated", + timeoutMs: this.config.timeoutMs, + params, + }); - try { - const typeRes = validateFlagsResponse(await res.json()); - if (!typeRes || !typeRes.success) { - this.logger.error( - "error fetching flags:", - new Error("unable to validate response"), - ); - return; - } + if (!res.ok) { + let errorSummary = ""; + let fallbackBody = ""; + try { + const { errorDetails, errorSummary: parsedSummary } = + await parseResponseError(res); + errorSummary = parsedSummary ?? ""; + fallbackBody = errorDetails.responseBody + ? ` - ${errorDetails.responseBody}` + : ""; + } catch { + // Best-effort response parsing only; the response itself is enough to fail. + } + + this.logger.error( + "error fetching flags:", + new Error( + `unexpected response code: ${res.status}${ + errorSummary ? ` - ${errorSummary}` : fallbackBody + }`, + ), + ); + return; + } + + const typeRes = validateFlagsResponse(await res.json()); + if (!typeRes || !typeRes.success) { + this.logger.error( + "error fetching flags:", + new Error("unable to validate response"), + ); + return; + } - return typeRes.flags; + return typeRes.flags; + }); } catch (e) { - logLifecycleAwareFetchError(this.logger, "error fetching flags:", e); + this.logger.error("error fetching flags:", e); return; } } diff --git a/packages/browser-sdk/src/utils/pageLifecycle.ts b/packages/browser-sdk/src/utils/pageLifecycle.ts deleted file mode 100644 index 5a03b03c..00000000 --- a/packages/browser-sdk/src/utils/pageLifecycle.ts +++ /dev/null @@ -1,46 +0,0 @@ -import { Logger } from "../logger"; - -let pageIsTearingDown = false; -let listenersRegistered = false; - -function markPageActive() { - pageIsTearingDown = false; -} - -function markPageTearingDown() { - pageIsTearingDown = true; -} - -function ensureListenersRegistered() { - if ( - listenersRegistered || - typeof window === "undefined" || - typeof window.addEventListener !== "function" - ) { - return; - } - - window.addEventListener("pagehide", markPageTearingDown, { capture: true }); - window.addEventListener("pageshow", markPageActive, { capture: true }); - listenersRegistered = true; -} - -export function isPageTearingDown() { - ensureListenersRegistered(); - return pageIsTearingDown; -} - -export function logLifecycleAwareFetchError( - logger: Pick, - message: string, - error: unknown, -) { - if (isPageTearingDown()) { - logger.debug(`${message} (aborted during page teardown)`, error); - return; - } - - logger.error(message, error); -} - -ensureListenersRegistered(); diff --git a/packages/browser-sdk/src/utils/retry.ts b/packages/browser-sdk/src/utils/retry.ts new file mode 100644 index 00000000..5b038e16 --- /dev/null +++ b/packages/browser-sdk/src/utils/retry.ts @@ -0,0 +1,31 @@ +function delay(ms: number) { + if (ms <= 0) { + return Promise.resolve(); + } + + return new Promise((resolve) => { + setTimeout(resolve, ms); + }); +} + +export async function retryOnThrow( + retryDelaysMs: number[], + operation: () => Promise, +): Promise { + let lastError: unknown; + + for (let attempt = 0; attempt <= retryDelaysMs.length; attempt += 1) { + try { + return await operation(); + } catch (error) { + lastError = error; + if (attempt === retryDelaysMs.length) { + throw error; + } + + await delay(retryDelaysMs[attempt] ?? 0); + } + } + + throw lastError; +} diff --git a/packages/browser-sdk/test/bulkQueue.test.ts b/packages/browser-sdk/test/bulkQueue.test.ts index b5f0d715..2dbe6bb1 100644 --- a/packages/browser-sdk/test/bulkQueue.test.ts +++ b/packages/browser-sdk/test/bulkQueue.test.ts @@ -35,7 +35,6 @@ describe("BulkQueue", () => { beforeEach(() => { vi.useFakeTimers(); sessionStorage.clear(); - window.dispatchEvent(new Event("pageshow")); }); afterEach(() => { @@ -89,7 +88,7 @@ describe("BulkQueue", () => { expect(sendBulk).toHaveBeenNthCalledWith(2, [trackEvent]); }); - it("requeues and downgrades page teardown aborts for bulk sends", async () => { + it("logs normal retry scheduling for thrown bulk send failures", async () => { const logger = { debug: vi.fn(), info: vi.fn(), @@ -107,51 +106,6 @@ describe("BulkQueue", () => { logger, }); - window.dispatchEvent(new Event("pagehide")); - await queue.enqueue(trackEvent); - - await vi.advanceTimersByTimeAsync(10); - expect(sendBulk).toHaveBeenCalledTimes(1); - expect(await queue.size()).toBe(1); - expect(logger.debug).toHaveBeenCalledWith( - "bulk retry scheduled (aborted during page teardown)", - expect.objectContaining({ - retryInMs: 20, - queueSize: 2, - consecutiveFailures: 1, - error: expect.any(TypeError), - }), - ); - expect(logger.info).not.toHaveBeenCalledWith( - "bulk retry scheduled", - expect.anything(), - ); - expect(logger.warn).not.toHaveBeenCalled(); - - window.dispatchEvent(new Event("pageshow")); - await vi.advanceTimersByTimeAsync(20); - expect(sendBulk).toHaveBeenCalledTimes(2); - expect(sendBulk).toHaveBeenNthCalledWith(2, [trackEvent]); - }); - - it("keeps 5xx bulk retries on the normal degraded-delivery path during teardown", async () => { - const logger = { - debug: vi.fn(), - info: vi.fn(), - warn: vi.fn(), - error: vi.fn(), - }; - const sendBulk = vi - .fn<(events: BulkEvent[]) => Promise>() - .mockResolvedValue(new Response("", { status: 500 })); - const queue = new BulkQueue(sendBulk, { - flushDelayMs: 10, - retryBaseDelayMs: 20, - retryMaxDelayMs: 20, - logger, - }); - - window.dispatchEvent(new Event("pagehide")); await queue.enqueue(trackEvent); await vi.advanceTimersByTimeAsync(10); @@ -162,6 +116,8 @@ describe("BulkQueue", () => { queueSize: 2, consecutiveFailures: 1, }); + await vi.advanceTimersByTimeAsync(20); + expect(sendBulk).toHaveBeenCalledTimes(2); }); it("drops 4xx responses, logs error, and does not retry", async () => { diff --git a/packages/browser-sdk/test/e2e/acceptance.browser.spec.ts b/packages/browser-sdk/test/e2e/acceptance.browser.spec.ts index e1291863..92b51f49 100644 --- a/packages/browser-sdk/test/e2e/acceptance.browser.spec.ts +++ b/packages/browser-sdk/test/e2e/acceptance.browser.spec.ts @@ -121,105 +121,3 @@ test("Acceptance", async ({ page }) => { expect(successfulRequests).toContain("EVENT"); expect(successfulRequests).toContain("FEEDBACK"); }); - -test("does not log startup fetch aborts as errors during fast full navigation", async ({ - page, -}) => { - await page.goto("http://localhost:8001/test/e2e/empty.html"); - - const consoleErrors: string[] = []; - page.on("console", (msg) => { - if (msg.type() === "error") { - consoleErrors.push(msg.text()); - } - }); - - let flagsStarted: () => void; - const flagsStartedPromise = new Promise((resolve) => { - flagsStarted = resolve; - }); - let promptingInitStarted: () => void; - const promptingInitStartedPromise = new Promise((resolve) => { - promptingInitStarted = resolve; - }); - - let releaseFlags: () => void; - const releaseFlagsPromise = new Promise((resolve) => { - releaseFlags = resolve; - }); - let releasePromptingInit: () => void; - const releasePromptingInitPromise = new Promise((resolve) => { - releasePromptingInit = resolve; - }); - - let flagsRequestCount = 0; - let promptingInitRequestCount = 0; - - await page.route(`${API_BASE_URL}/features/evaluated*`, async (route) => { - flagsRequestCount += 1; - if (flagsRequestCount === 1) { - flagsStarted(); - await releaseFlagsPromise; - try { - await route.abort("failed"); - } catch { - // The original document may already be gone. - } - } else { - await route.fulfill({ - status: 200, - body: JSON.stringify({ - success: true, - features: {}, - }), - }); - } - }); - - await page.route(`${API_BASE_URL}/feedback/prompting-init`, async (route) => { - promptingInitRequestCount += 1; - if (promptingInitRequestCount === 1) { - promptingInitStarted(); - await releasePromptingInitPromise; - try { - await route.abort("failed"); - } catch { - // The original document may already be gone. - } - } else { - await route.fulfill({ - status: 200, - body: JSON.stringify({ success: false }), - }); - } - }); - - await page.evaluate(` - ;(async () => { - const { ReflagClient } = await import("/dist/reflag-browser-sdk.mjs"); - const reflagClient = new ReflagClient({ - publishableKey: "${KEY}", - user: { id: "foo" }, - company: { id: "bar" }, - }); - void reflagClient.initialize(); - })() - `); - - await Promise.all([flagsStartedPromise, promptingInitStartedPromise]); - - const navigation = page.goto("http://localhost:8001/test/e2e/give-feedback-button.html"); - releaseFlags(); - releasePromptingInit(); - await navigation; - await page.waitForTimeout(250); - - expect(flagsRequestCount).toBeGreaterThanOrEqual(1); - expect(promptingInitRequestCount).toBeGreaterThanOrEqual(1); - expect(consoleErrors).not.toContainEqual( - expect.stringContaining("error fetching flags"), - ); - expect(consoleErrors).not.toContainEqual( - expect.stringContaining("error initializing automatic feedback"), - ); -}); diff --git a/packages/browser-sdk/test/flags.test.ts b/packages/browser-sdk/test/flags.test.ts index e33f45cb..9a2261db 100644 --- a/packages/browser-sdk/test/flags.test.ts +++ b/packages/browser-sdk/test/flags.test.ts @@ -12,7 +12,6 @@ import { testLogger } from "./testLogger"; beforeEach(() => { vi.useFakeTimers(); vi.resetAllMocks(); - window.dispatchEvent(new Event("pageshow")); }); afterAll(() => { @@ -153,7 +152,9 @@ describe("FlagsClient", () => { fallbackFlags: ["huddle"], }); - await flagsClient.initialize(); + const initializePromise = flagsClient.initialize(); + await vi.advanceTimersByTimeAsync(5000); + await initializePromise; expect(flagsClient.getFlags()).toStrictEqual({ huddle: { isEnabled: true, @@ -180,7 +181,9 @@ describe("FlagsClient", () => { }, }); - await flagsClient.initialize(); + const initializePromise = flagsClient.initialize(); + await vi.advanceTimersByTimeAsync(5000); + await initializePromise; expect(flagsClient.getFlags()).toStrictEqual({ huddle: { isEnabled: true, @@ -197,41 +200,52 @@ describe("FlagsClient", () => { }); }); - test("downgrades page teardown fetch failures to debug logs", async () => { + test("retries thrown flag fetch failures before succeeding", async () => { const { newFlagsClient, httpClient } = flagsClientFactory(); - vi.mocked(httpClient.get).mockRejectedValue( - new TypeError("Failed to fetch"), - ); - window.dispatchEvent(new Event("pagehide")); + vi.mocked(httpClient.get) + .mockRejectedValueOnce(new TypeError("Failed to fetch")) + .mockRejectedValueOnce(new TypeError("Failed to fetch")) + .mockResolvedValue( + new Response(JSON.stringify(flagResponse), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), + ); const flagsClient = newFlagsClient(); - await flagsClient.initialize(); + const initializePromise = flagsClient.initialize(); + await vi.advanceTimersByTimeAsync(5000); + await initializePromise; + expect(httpClient.get).toHaveBeenCalledTimes(3); expect(testLogger.error).not.toHaveBeenCalled(); - expect(testLogger.debug).toHaveBeenCalledWith( - "[Flags] error fetching flags: (aborted during page teardown)", - expect.any(TypeError), - ); + expect(flagsClient.getFlags()).toEqual(flagsResult); }); - test("downgrades page teardown body-read failures to debug logs", async () => { + test("retries thrown flag body-read failures before succeeding", async () => { const { newFlagsClient, httpClient } = flagsClientFactory(); - vi.mocked(httpClient.get).mockResolvedValue({ - ok: true, - json: vi.fn().mockRejectedValue(new TypeError("Failed to fetch")), - } as unknown as Response); - window.dispatchEvent(new Event("pagehide")); + vi.mocked(httpClient.get) + .mockResolvedValueOnce({ + ok: true, + json: vi.fn().mockRejectedValue(new TypeError("Failed to fetch")), + } as unknown as Response) + .mockResolvedValue( + new Response(JSON.stringify(flagResponse), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), + ); const flagsClient = newFlagsClient(); - await flagsClient.initialize(); + const initializePromise = flagsClient.initialize(); + await vi.advanceTimersByTimeAsync(0); + await initializePromise; + expect(httpClient.get).toHaveBeenCalledTimes(2); expect(testLogger.error).not.toHaveBeenCalled(); - expect(testLogger.debug).toHaveBeenCalledWith( - "[Flags] error fetching flags: (aborted during page teardown)", - expect.any(TypeError), - ); + expect(flagsClient.getFlags()).toEqual(flagsResult); }); test("caches response", async () => { @@ -264,8 +278,10 @@ describe("FlagsClient", () => { vi.advanceTimersByTime(TEST_STALE_MS + 1); // fail this time - await flagsClient.fetchFlags(); - expect(httpClient.get).toBeCalledTimes(2); + const fetchPromise = flagsClient.fetchFlags(); + await vi.advanceTimersByTimeAsync(5000); + await fetchPromise; + expect(httpClient.get).toBeCalledTimes(4); const staleFlags = flagsClient.getFlags(); expect(staleFlags).toEqual(flagsResult); diff --git a/packages/browser-sdk/test/usage.test.ts b/packages/browser-sdk/test/usage.test.ts index 15d9e565..982798d3 100644 --- a/packages/browser-sdk/test/usage.test.ts +++ b/packages/browser-sdk/test/usage.test.ts @@ -46,7 +46,6 @@ window.innerWidth = 1024; afterEach(() => { server.resetHandlers(); - window.dispatchEvent(new Event("pageshow")); }); beforeEach(() => { @@ -203,18 +202,24 @@ describe("feedback prompting", () => { expect(openAblySSEChannel).toBeCalledTimes(0); }); - test("downgrades prompting init fetch failures during page teardown", async () => { + test("retries prompting init fetch failures before succeeding", async () => { + vi.useFakeTimers(); const logger = { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn(), }; - const post = vi - .spyOn(HttpClient.prototype, "post") - .mockRejectedValueOnce(new TypeError("Failed to fetch")); - - window.dispatchEvent(new Event("pagehide")); + const post = vi.spyOn(HttpClient.prototype, "post"); + post + .mockRejectedValueOnce(new TypeError("Failed to fetch")) + .mockRejectedValueOnce(new TypeError("Failed to fetch")) + .mockResolvedValueOnce( + new Response(JSON.stringify({ success: false }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), + ); try { const reflagInstance = new ReflagClient({ @@ -223,33 +228,39 @@ describe("feedback prompting", () => { enableTracking: false, logger, }); - await reflagInstance.initialize(); + const initializePromise = reflagInstance.initialize(); + await vi.advanceTimersByTimeAsync(5000); + await initializePromise; - expect(post).toHaveBeenCalled(); + expect(post).toHaveBeenCalledTimes(3); expect(logger.error).not.toHaveBeenCalled(); - expect(logger.debug).toHaveBeenCalledWith( - "error initializing automatic feedback (aborted during page teardown)", - expect.any(TypeError), - ); expect(openAblySSEChannel).toBeCalledTimes(0); } finally { + vi.useRealTimers(); post.mockRestore(); } }); - test("downgrades prompting init body-read failures during page teardown", async () => { + test("retries prompting init body-read failures before succeeding", async () => { + vi.useFakeTimers(); const logger = { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn(), }; - const post = vi.spyOn(HttpClient.prototype, "post").mockResolvedValueOnce({ - ok: true, - json: vi.fn().mockRejectedValue(new TypeError("Failed to fetch")), - } as unknown as Response); - - window.dispatchEvent(new Event("pagehide")); + const post = vi.spyOn(HttpClient.prototype, "post"); + post + .mockResolvedValueOnce({ + ok: true, + json: vi.fn().mockRejectedValue(new TypeError("Failed to fetch")), + } as unknown as Response) + .mockResolvedValueOnce( + new Response(JSON.stringify({ success: false }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), + ); try { const reflagInstance = new ReflagClient({ @@ -258,16 +269,15 @@ describe("feedback prompting", () => { enableTracking: false, logger, }); - await reflagInstance.initialize(); + const initializePromise = reflagInstance.initialize(); + await vi.advanceTimersByTimeAsync(0); + await initializePromise; - expect(post).toHaveBeenCalled(); + expect(post).toHaveBeenCalledTimes(2); expect(logger.error).not.toHaveBeenCalled(); - expect(logger.debug).toHaveBeenCalledWith( - "error initializing automatic feedback (aborted during page teardown)", - expect.any(TypeError), - ); expect(openAblySSEChannel).toBeCalledTimes(0); } finally { + vi.useRealTimers(); post.mockRestore(); } }); From bc384c53652da630d9fe8f069e3f0853512d8e98 Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Thu, 12 Mar 2026 11:38:07 +0100 Subject: [PATCH 09/16] Simplify bulk delivery to best effort --- packages/browser-sdk/src/bulkQueue.ts | 266 ++------------------ packages/browser-sdk/src/client.ts | 23 +- packages/browser-sdk/src/config.ts | 2 - packages/browser-sdk/src/httpClient.ts | 3 + packages/browser-sdk/test/bulkQueue.test.ts | 105 +++----- 5 files changed, 66 insertions(+), 333 deletions(-) diff --git a/packages/browser-sdk/src/bulkQueue.ts b/packages/browser-sdk/src/bulkQueue.ts index 7572a564..3c605319 100644 --- a/packages/browser-sdk/src/bulkQueue.ts +++ b/packages/browser-sdk/src/bulkQueue.ts @@ -1,16 +1,7 @@ import { logResponseError } from "./utils/responseError"; -import { - BULK_QUEUE_FLUSH_DELAY_MS, - BULK_QUEUE_MAX_SIZE, - BULK_QUEUE_RETRY_BASE_DELAY_MS, - BULK_QUEUE_RETRY_MAX_DELAY_MS, -} from "./config"; +import { BULK_QUEUE_FLUSH_DELAY_MS, BULK_QUEUE_MAX_SIZE } from "./config"; import { Logger } from "./logger"; -const BULK_QUEUE_STORAGE_KEY = "__reflag_bulk_queue_v1"; -const WARN_AFTER_CONSECUTIVE_FAILURES = 10; -const WARN_AFTER_FAILURE_MS = 5 * 60 * 1000; -const WARN_THROTTLE_MS = 15 * 60 * 1000; const DROP_ERROR_THROTTLE_MS = 15 * 60 * 1000; type PayloadContext = { @@ -61,84 +52,19 @@ export type BulkEvent = export type BulkQueueOptions = { flushDelayMs?: number; maxSize?: number; - retryBaseDelayMs?: number; - retryMaxDelayMs?: number; - storageKey?: string; logger?: Logger; }; -function getSessionStorage(): Storage | null { - try { - if (typeof sessionStorage === "undefined") { - return null; - } - return sessionStorage; - } catch { - return null; - } -} - -function isObject(value: unknown): value is Record { - return typeof value === "object" && value !== null; -} - -function isBulkEvent(value: unknown): value is BulkEvent { - if (!isObject(value) || typeof value.type !== "string") { - return false; - } - - if (value.type === "user") { - return typeof value.userId === "string"; - } - - if (value.type === "company") { - return typeof value.companyId === "string"; - } - - if (value.type === "event") { - return typeof value.userId === "string" && typeof value.event === "string"; - } - - if (value.type === "feature-flag-event") { - return ( - typeof value.key === "string" && - (value.action === "check-is-enabled" || value.action === "check-config") - ); - } - - if (value.type === "prompt-event") { - return ( - typeof value.featureId === "string" && - typeof value.promptId === "string" && - typeof value.userId === "string" && - typeof value.promptedQuestion === "string" && - (value.action === "received" || - value.action === "shown" || - value.action === "dismissed") - ); - } - - return false; -} - export class BulkQueue { private readonly flushDelayMs: number; private readonly maxSize: number; - private readonly retryBaseDelayMs: number; - private readonly retryMaxDelayMs: number; - private readonly storageKey: string; - private readonly storage: Storage | null; private readonly logger?: Logger; private readonly sendBulk: (events: BulkEvent[]) => Promise; private queue: BulkEvent[] = []; private timer: ReturnType | null = null; private inFlightBatch: BulkEvent[] | null = null; - private inFlightPromise: Promise | null = null; - private retryCount = 0; - private consecutiveFailures = 0; - private firstFailureAt: number | null = null; - private lastWarnAt: number | null = null; + private inFlightPromise: Promise | null = null; private lastDropErrorAt: number | null = null; private totalDroppedEvents = 0; private droppedSinceLastError = 0; @@ -150,24 +76,12 @@ export class BulkQueue { this.sendBulk = sendBulk; this.flushDelayMs = opts.flushDelayMs ?? BULK_QUEUE_FLUSH_DELAY_MS; this.maxSize = opts.maxSize ?? BULK_QUEUE_MAX_SIZE; - this.retryBaseDelayMs = - opts.retryBaseDelayMs ?? BULK_QUEUE_RETRY_BASE_DELAY_MS; - this.retryMaxDelayMs = - opts.retryMaxDelayMs ?? BULK_QUEUE_RETRY_MAX_DELAY_MS; - this.storageKey = opts.storageKey ?? BULK_QUEUE_STORAGE_KEY; - this.storage = getSessionStorage(); this.logger = opts.logger; - - this.restoreQueueFromStorage(); - if (this.queue.length > 0) { - this.schedule(this.flushDelayMs); - } } async enqueue(event: BulkEvent) { this.queue.push(event); this.trimPendingQueueToCapacity(); - this.persistQueueToStorage(); const maxPending = Math.max(0, this.maxSize - this.getInFlightBatchSize()); if (this.queue.length > 0 && this.queue.length >= maxPending) { @@ -198,19 +112,17 @@ export class BulkQueue { const sendPromise = this.sendBatch(batch); this.inFlightPromise = sendPromise; - let nextDelayMs: number | null = null; try { - nextDelayMs = await sendPromise; + await sendPromise; } finally { if (this.inFlightPromise === sendPromise) { this.inFlightPromise = null; } this.inFlightBatch = null; - this.persistQueueToStorage(); } - if (this.queue.length > 0 && !this.timer && nextDelayMs !== null) { - this.schedule(nextDelayMs); + if (this.queue.length > 0 && !this.timer) { + this.schedule(this.flushDelayMs); } } @@ -218,15 +130,6 @@ export class BulkQueue { return this.queue.length + this.getInFlightBatchSize(); } - private getRetryDelay() { - const maxExponent = 6; - const exponent = Math.min(this.retryCount - 1, maxExponent); - return Math.min( - this.retryBaseDelayMs * 2 ** exponent, - this.retryMaxDelayMs, - ); - } - private schedule(delayMs: number) { if (this.timer || this.inFlightPromise || this.queue.length === 0) { return; @@ -244,166 +147,27 @@ export class BulkQueue { } private async sendBatch(batch: BulkEvent[]) { - let nextDelayMs: number | null = null; - let res: Response; try { res = await this.sendBulk(batch); } catch (error) { - this.queue = batch.concat(this.queue); - - const now = Date.now(); - if (this.firstFailureAt === null) { - this.firstFailureAt = now; - } - this.consecutiveFailures += 1; - this.retryCount += 1; - const retryInMs = this.getRetryDelay(); - nextDelayMs = retryInMs; - const logDetails = { - retryInMs, - queueSize: this.queue.length + this.getInFlightBatchSize(), - consecutiveFailures: this.consecutiveFailures, - }; - this.logger?.info("bulk retry scheduled", logDetails); - - const outageMs = now - this.firstFailureAt; - const shouldWarn = - this.consecutiveFailures >= WARN_AFTER_CONSECUTIVE_FAILURES || - outageMs >= WARN_AFTER_FAILURE_MS; - const canWarnNow = - this.lastWarnAt === null || now - this.lastWarnAt >= WARN_THROTTLE_MS; - if (shouldWarn && canWarnNow) { - this.logger?.warn("bulk delivery degraded", { - consecutiveFailures: this.consecutiveFailures, - outageMs, - queueSize: this.queue.length + this.getInFlightBatchSize(), - retryInMs, - error, - }); - this.lastWarnAt = now; - } - return nextDelayMs; - } - - if (!res.ok) { - if (res.status >= 400 && res.status < 500) { - this.retryCount = 0; - this.firstFailureAt = null; - this.consecutiveFailures = 0; - this.lastWarnAt = null; - if (this.logger) { - await logResponseError({ - logger: this.logger, - res, - message: - "bulk request failed with non-retriable status; dropping batch", - }); - } - return this.flushDelayMs; - } - - this.queue = batch.concat(this.queue); - - const now = Date.now(); - if (this.firstFailureAt === null) { - this.firstFailureAt = now; - } - this.consecutiveFailures += 1; - this.retryCount += 1; - const retryInMs = this.getRetryDelay(); - nextDelayMs = retryInMs; - this.logger?.info("bulk retry scheduled", { - retryInMs, - queueSize: this.queue.length + this.getInFlightBatchSize(), - consecutiveFailures: this.consecutiveFailures, + this.logger?.error("bulk request failed; dropping batch", { + error, + batchSize: batch.length, }); - - const outageMs = now - this.firstFailureAt; - const shouldWarn = - this.consecutiveFailures >= WARN_AFTER_CONSECUTIVE_FAILURES || - outageMs >= WARN_AFTER_FAILURE_MS; - const canWarnNow = - this.lastWarnAt === null || now - this.lastWarnAt >= WARN_THROTTLE_MS; - if (shouldWarn && canWarnNow) { - this.logger?.warn("bulk delivery degraded", { - consecutiveFailures: this.consecutiveFailures, - outageMs, - queueSize: this.queue.length + this.getInFlightBatchSize(), - retryInMs, - error: new Error(`unexpected status ${res.status}`), - }); - this.lastWarnAt = now; - } - return nextDelayMs; - } - - this.retryCount = 0; - if (this.firstFailureAt !== null && this.consecutiveFailures > 0) { - this.logger?.info("bulk delivery recovered", { - outageMs: Date.now() - this.firstFailureAt, - failedAttempts: this.consecutiveFailures, - }); - } - this.firstFailureAt = null; - this.consecutiveFailures = 0; - this.lastWarnAt = null; - nextDelayMs = this.flushDelayMs; - - return nextDelayMs; - } - - private getPersistedQueue() { - const inFlight = this.inFlightBatch ?? []; - return inFlight.concat(this.queue).slice(-this.maxSize); - } - - private persistQueueToStorage() { - if (!this.storage) { return; } - try { - const persisted = this.getPersistedQueue(); - if (persisted.length === 0) { - this.storage.removeItem(this.storageKey); - return; + if (!res.ok) { + if (this.logger) { + await logResponseError({ + logger: this.logger, + res, + message: "bulk request failed; dropping batch", + }); } - - this.storage.setItem(this.storageKey, JSON.stringify(persisted)); - } catch { - // ignore persistence failures - } - } - - private restoreQueueFromStorage() { - if (!this.storage) { return; } - - try { - const raw = this.storage.getItem(this.storageKey); - if (!raw) { - return; - } - - const parsed: unknown = JSON.parse(raw); - if (!Array.isArray(parsed)) { - throw new Error("invalid stored bulk queue"); - } - - this.queue = parsed.filter(isBulkEvent).slice(-this.maxSize); - if (this.queue.length === 0) { - this.storage.removeItem(this.storageKey); - } - } catch { - this.queue = []; - try { - this.storage.removeItem(this.storageKey); - } catch { - // ignore cleanup failures - } - } } private getInFlightBatchSize() { diff --git a/packages/browser-sdk/src/client.ts b/packages/browser-sdk/src/client.ts index 461c350e..5decd61e 100644 --- a/packages/browser-sdk/src/client.ts +++ b/packages/browser-sdk/src/client.ts @@ -310,8 +310,7 @@ export type InitOptions = ReflagDeprecatedContext & { /** * Queue settings for tracking updates sent to `/bulk`. * Applies to user/company updates, check events, and prompt events. - * Queue data is persisted in `sessionStorage` and restored on reloads - * within the same browser tab. + * Events are buffered in memory and flushed in the background. */ trackingQueue?: { /** @@ -329,16 +328,8 @@ export type InitOptions = ReflagDeprecatedContext & { maxSize?: number; /** - * Base retry delay in milliseconds after a failed bulk request. - * Defaults to 5000ms. + * No retries are performed; failed batches are dropped. */ - retryBaseDelayMs?: number; - - /** - * Maximum retry delay in milliseconds after repeated failures. - * Defaults to 60000ms. - */ - retryMaxDelayMs?: number; }; }; @@ -476,13 +467,15 @@ export class ReflagClient { }); if (!this.config.offline && this.config.enableTracking) { this.bulkQueue = new BulkQueue( - (events) => this.httpClient.post({ path: "/bulk", body: events }), + (events) => + this.httpClient.post({ + path: "/bulk", + body: events, + keepalive: true, + }), { flushDelayMs: opts.trackingQueue?.flushDelayMs, maxSize: opts.trackingQueue?.maxSize, - retryBaseDelayMs: opts.trackingQueue?.retryBaseDelayMs, - retryMaxDelayMs: opts.trackingQueue?.retryMaxDelayMs, - storageKey: `__reflag_bulk_queue_v1:${this.config.apiBaseUrl}:${this.publishableKey}`, logger: this.logger, }, ); diff --git a/packages/browser-sdk/src/config.ts b/packages/browser-sdk/src/config.ts index b45ef204..ceec3b46 100644 --- a/packages/browser-sdk/src/config.ts +++ b/packages/browser-sdk/src/config.ts @@ -11,8 +11,6 @@ export const FLAG_EVENTS_PER_MIN = 1; export const FLAGS_EXPIRE_MS = 30 * 24 * 60 * 60 * 1000; // expire entirely after 30 days export const BULK_QUEUE_MAX_SIZE = 100; export const BULK_QUEUE_FLUSH_DELAY_MS = 2000; -export const BULK_QUEUE_RETRY_BASE_DELAY_MS = 5000; -export const BULK_QUEUE_RETRY_MAX_DELAY_MS = 60_000; export const IS_SERVER = typeof window === "undefined" || typeof document === "undefined"; diff --git a/packages/browser-sdk/src/httpClient.ts b/packages/browser-sdk/src/httpClient.ts index bf9f207a..0bfaf58c 100644 --- a/packages/browser-sdk/src/httpClient.ts +++ b/packages/browser-sdk/src/httpClient.ts @@ -78,14 +78,17 @@ export class HttpClient { async post({ path, body, + keepalive, }: { host?: string; path: string; body: any; + keepalive?: boolean; }): ReturnType { return fetch(this.getUrl(path), { ...this.fetchOptions, method: "POST", + keepalive, headers: { "Content-Type": "application/json", [SDK_VERSION_HEADER_NAME]: this.sdkVersion, diff --git a/packages/browser-sdk/test/bulkQueue.test.ts b/packages/browser-sdk/test/bulkQueue.test.ts index 2dbe6bb1..5b878859 100644 --- a/packages/browser-sdk/test/bulkQueue.test.ts +++ b/packages/browser-sdk/test/bulkQueue.test.ts @@ -34,13 +34,11 @@ const lateTrackEvent: BulkEvent = { describe("BulkQueue", () => { beforeEach(() => { vi.useFakeTimers(); - sessionStorage.clear(); }); afterEach(() => { vi.useRealTimers(); vi.clearAllMocks(); - sessionStorage.clear(); }); it("batches events and flushes after the delay", async () => { @@ -64,45 +62,18 @@ describe("BulkQueue", () => { expect(sendBulk).toHaveBeenCalledWith([userEvent, companyEvent]); }); - it("retries failed bulk requests later", async () => { + it("drops thrown bulk request failures without retrying", async () => { const sendBulk = vi .fn<(events: BulkEvent[]) => Promise>() - .mockRejectedValueOnce(new Error("network")) - .mockResolvedValue(new Response("", { status: 200 })); - const queue = new BulkQueue(sendBulk, { - flushDelayMs: 10, - retryBaseDelayMs: 20, - retryMaxDelayMs: 20, - }); - - await queue.enqueue(trackEvent); - - await vi.advanceTimersByTimeAsync(10); - expect(sendBulk).toHaveBeenCalledTimes(1); - - await vi.advanceTimersByTimeAsync(19); - expect(sendBulk).toHaveBeenCalledTimes(1); - - await vi.advanceTimersByTimeAsync(1); - expect(sendBulk).toHaveBeenCalledTimes(2); - expect(sendBulk).toHaveBeenNthCalledWith(2, [trackEvent]); - }); - - it("logs normal retry scheduling for thrown bulk send failures", async () => { + .mockRejectedValueOnce(new Error("network")); const logger = { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn(), }; - const sendBulk = vi - .fn<(events: BulkEvent[]) => Promise>() - .mockRejectedValueOnce(new TypeError("Failed to fetch")) - .mockResolvedValue(new Response("", { status: 200 })); const queue = new BulkQueue(sendBulk, { flushDelayMs: 10, - retryBaseDelayMs: 20, - retryMaxDelayMs: 20, logger, }); @@ -110,14 +81,14 @@ describe("BulkQueue", () => { await vi.advanceTimersByTimeAsync(10); expect(sendBulk).toHaveBeenCalledTimes(1); - expect(logger.debug).not.toHaveBeenCalled(); - expect(logger.info).toHaveBeenCalledWith("bulk retry scheduled", { - retryInMs: 20, - queueSize: 2, - consecutiveFailures: 1, - }); - await vi.advanceTimersByTimeAsync(20); - expect(sendBulk).toHaveBeenCalledTimes(2); + expect(await queue.size()).toBe(0); + expect(logger.error).toHaveBeenCalledWith( + "bulk request failed; dropping batch", + expect.objectContaining({ + error: expect.any(Error), + batchSize: 1, + }), + ); }); it("drops 4xx responses, logs error, and does not retry", async () => { @@ -132,8 +103,6 @@ describe("BulkQueue", () => { }; const queue = new BulkQueue(sendBulk, { flushDelayMs: 10, - retryBaseDelayMs: 20, - retryMaxDelayMs: 20, logger, }); @@ -143,7 +112,7 @@ describe("BulkQueue", () => { expect(sendBulk).toHaveBeenCalledTimes(1); expect(await queue.size()).toBe(0); expect(logger.error).toHaveBeenCalledWith( - "bulk request failed with non-retriable status; dropping batch", + "bulk request failed; dropping batch", expect.objectContaining({ status: 400, responseBody: "invalid payload", @@ -195,6 +164,35 @@ describe("BulkQueue", () => { ); }); + it("drops 5xx responses without retrying", async () => { + const sendBulk = vi + .fn<(events: BulkEvent[]) => Promise>() + .mockResolvedValue(new Response("server error", { status: 500 })); + const logger = { + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }; + const queue = new BulkQueue(sendBulk, { + flushDelayMs: 10, + logger, + }); + + await queue.enqueue(trackEvent); + await vi.advanceTimersByTimeAsync(10); + + expect(sendBulk).toHaveBeenCalledTimes(1); + expect(await queue.size()).toBe(0); + expect(logger.error).toHaveBeenCalledWith( + "bulk request failed; dropping batch", + expect.objectContaining({ + status: 500, + responseBody: "server error", + }), + ); + }); + it("does not drop newly queued events when an older batch completes", async () => { let resolveFirstSend: ((res: Response) => void) | undefined; const firstSend = new Promise((resolve) => { @@ -255,29 +253,6 @@ describe("BulkQueue", () => { resolveSend?.(new Response("", { status: 200 })); }); - it("restores queue state between instances in the same tab", async () => { - const firstSend = vi - .fn<(events: BulkEvent[]) => Promise>() - .mockResolvedValue(new Response("", { status: 200 })); - const firstQueue = new BulkQueue(firstSend, { - flushDelayMs: 10_000, - }); - - await firstQueue.enqueue(userEvent); - expect(await firstQueue.size()).toBe(1); - - const secondSend = vi - .fn<(events: BulkEvent[]) => Promise>() - .mockResolvedValue(new Response("", { status: 200 })); - const secondQueue = new BulkQueue(secondSend, { - flushDelayMs: 10_000, - }); - - expect(await secondQueue.size()).toBe(1); - await secondQueue.flush(); - expect(secondSend).toHaveBeenCalledWith([userEvent]); - }); - it("requires a second flush to send pending events after an in-flight batch", async () => { let resolveFirstSend: ((res: Response) => void) | undefined; const firstSend = new Promise((resolve) => { From 1294006e9949afa361c585167e10baef3bababcf Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Thu, 12 Mar 2026 12:25:31 +0100 Subject: [PATCH 10/16] Improve best-effort bulk flushing --- packages/browser-sdk/src/client.ts | 23 +++++++++++++++++++- packages/browser-sdk/src/httpClient.ts | 15 +++++++++++-- packages/browser-sdk/test/client.test.ts | 16 ++++++++++++++ packages/browser-sdk/test/httpClient.test.ts | 23 ++++++++++++++++++++ 4 files changed, 74 insertions(+), 3 deletions(-) diff --git a/packages/browser-sdk/src/client.ts b/packages/browser-sdk/src/client.ts index 5decd61e..af22a57f 100644 --- a/packages/browser-sdk/src/client.ts +++ b/packages/browser-sdk/src/client.ts @@ -328,8 +328,14 @@ export type InitOptions = ReflagDeprecatedContext & { maxSize?: number; /** - * No retries are performed; failed batches are dropped. + * Deprecated: retries are no longer performed for bulk delivery. */ + retryBaseDelayMs?: number; + + /** + * Deprecated: retries are no longer performed for bulk delivery. + */ + retryMaxDelayMs?: number; }; }; @@ -423,6 +429,7 @@ export class ReflagClient { private autoFeedbackInit: Promise | undefined; private readonly flagsClient: FlagsClient; private readonly bulkQueue: BulkQueue | undefined; + private readonly handleBeforeUnload?: () => void; public readonly logger: Logger; @@ -480,6 +487,14 @@ export class ReflagClient { }, ); } + if (this.bulkQueue && !IS_SERVER) { + this.handleBeforeUnload = () => { + void this.bulkQueue?.flush(); + }; + window.addEventListener("beforeunload", this.handleBeforeUnload, { + capture: true, + }); + } const bulkQueue = this.bulkQueue; @@ -590,6 +605,12 @@ export class ReflagClient { * **/ async stop() { + if (this.handleBeforeUnload && !IS_SERVER) { + window.removeEventListener("beforeunload", this.handleBeforeUnload, { + capture: true, + }); + } + if (this.bulkQueue) { await this.bulkQueue.flush(); let remaining = await this.bulkQueue.size(); diff --git a/packages/browser-sdk/src/httpClient.ts b/packages/browser-sdk/src/httpClient.ts index 0bfaf58c..1afc273d 100644 --- a/packages/browser-sdk/src/httpClient.ts +++ b/packages/browser-sdk/src/httpClient.ts @@ -1,12 +1,18 @@ import { createAbortController } from "./utils/abortController"; import { API_BASE_URL, SDK_VERSION, SDK_VERSION_HEADER_NAME } from "./config"; +const KEEPALIVE_MAX_BODY_BYTES = 60 * 1024; + export interface HttpClientOptions { baseUrl?: string; sdkVersion?: string; credentials?: RequestCredentials; } +function getBodyByteLength(value: string) { + return new TextEncoder().encode(value).length; +} + export class HttpClient { private readonly baseUrl: string; private readonly sdkVersion: string; @@ -85,16 +91,21 @@ export class HttpClient { body: any; keepalive?: boolean; }): ReturnType { + const serializedBody = JSON.stringify(body); + const shouldUseKeepalive = + keepalive && + getBodyByteLength(serializedBody) <= KEEPALIVE_MAX_BODY_BYTES; + return fetch(this.getUrl(path), { ...this.fetchOptions, method: "POST", - keepalive, + keepalive: shouldUseKeepalive, headers: { "Content-Type": "application/json", [SDK_VERSION_HEADER_NAME]: this.sdkVersion, Authorization: `Bearer ${this.publishableKey}`, }, - body: JSON.stringify(body), + body: serializedBody, }); } } diff --git a/packages/browser-sdk/test/client.test.ts b/packages/browser-sdk/test/client.test.ts index 382ea4ac..f3c6f8b7 100644 --- a/packages/browser-sdk/test/client.test.ts +++ b/packages/browser-sdk/test/client.test.ts @@ -46,6 +46,7 @@ describe("ReflagClient", () => { await vi.waitFor(() => expect(httpClientPost).toHaveBeenCalledWith({ path: "/bulk", + keepalive: true, body: [ { type: "user", @@ -73,6 +74,7 @@ describe("ReflagClient", () => { await vi.waitFor(() => expect(httpClientPost).toHaveBeenCalledWith({ path: "/bulk", + keepalive: true, body: [ { type: "company", @@ -197,6 +199,20 @@ describe("ReflagClient", () => { }); describe("stop", () => { + it("flushes the bulk queue on beforeunload and removes the listener on stop", async () => { + const bulkQueue = client["bulkQueue"]; + expect(bulkQueue).toBeDefined(); + + const flushSpy = vi.spyOn(bulkQueue!, "flush").mockResolvedValue(); + window.dispatchEvent(new Event("beforeunload")); + expect(flushSpy).toHaveBeenCalledTimes(1); + + await client.stop(); + + window.dispatchEvent(new Event("beforeunload")); + expect(flushSpy).toHaveBeenCalledTimes(2); + }); + it("throws if queued bulk events remain after final flush attempt", async () => { const bulkQueue = client["bulkQueue"]; expect(bulkQueue).toBeDefined(); diff --git a/packages/browser-sdk/test/httpClient.test.ts b/packages/browser-sdk/test/httpClient.test.ts index 9389b5ec..af7cef9a 100644 --- a/packages/browser-sdk/test/httpClient.test.ts +++ b/packages/browser-sdk/test/httpClient.test.ts @@ -64,6 +64,29 @@ describe("sets `credentials`", () => { ); }); + test("uses keepalive for small request bodies when requested", async () => { + const client = new HttpClient("publishableKey"); + + await client.post({ path: "/test", body: { ok: true }, keepalive: true }); + + expect(global.fetch).toHaveBeenCalledWith( + expect.any(URL), + expect.objectContaining({ keepalive: true }), + ); + }); + + test("does not use keepalive for large request bodies", async () => { + const client = new HttpClient("publishableKey"); + const largeBody = { payload: "x".repeat(70 * 1024) }; + + await client.post({ path: "/test", body: largeBody, keepalive: true }); + + expect(global.fetch).toHaveBeenCalledWith( + expect.any(URL), + expect.objectContaining({ keepalive: false }), + ); + }); + test("does not require a writable `URL.search` property", async () => { const OriginalURL = global.URL; From 263b959a71d042795aca58598cc82a258f126b04 Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Thu, 12 Mar 2026 12:42:19 +0100 Subject: [PATCH 11/16] Bound bulk keepalive usage --- packages/browser-sdk/src/httpClient.ts | 44 ++++++++--- packages/browser-sdk/test/httpClient.test.ts | 83 ++++++++++++++++++++ 2 files changed, 115 insertions(+), 12 deletions(-) diff --git a/packages/browser-sdk/src/httpClient.ts b/packages/browser-sdk/src/httpClient.ts index 1afc273d..2592b7e5 100644 --- a/packages/browser-sdk/src/httpClient.ts +++ b/packages/browser-sdk/src/httpClient.ts @@ -2,6 +2,8 @@ import { createAbortController } from "./utils/abortController"; import { API_BASE_URL, SDK_VERSION, SDK_VERSION_HEADER_NAME } from "./config"; const KEEPALIVE_MAX_BODY_BYTES = 60 * 1024; +const KEEPALIVE_MAX_IN_FLIGHT_BYTES = 60 * 1024; +const KEEPALIVE_MAX_IN_FLIGHT_REQUESTS = 15; export interface HttpClientOptions { baseUrl?: string; @@ -18,6 +20,8 @@ export class HttpClient { private readonly sdkVersion: string; private readonly fetchOptions: RequestInit; + private inFlightKeepaliveBytes = 0; + private inFlightKeepaliveRequests = 0; constructor( public publishableKey: string, @@ -92,20 +96,36 @@ export class HttpClient { keepalive?: boolean; }): ReturnType { const serializedBody = JSON.stringify(body); + const bodyBytes = getBodyByteLength(serializedBody); const shouldUseKeepalive = keepalive && - getBodyByteLength(serializedBody) <= KEEPALIVE_MAX_BODY_BYTES; + bodyBytes <= KEEPALIVE_MAX_BODY_BYTES && + this.inFlightKeepaliveBytes + bodyBytes <= + KEEPALIVE_MAX_IN_FLIGHT_BYTES && + this.inFlightKeepaliveRequests < KEEPALIVE_MAX_IN_FLIGHT_REQUESTS; + + if (shouldUseKeepalive) { + this.inFlightKeepaliveBytes += bodyBytes; + this.inFlightKeepaliveRequests += 1; + } - return fetch(this.getUrl(path), { - ...this.fetchOptions, - method: "POST", - keepalive: shouldUseKeepalive, - headers: { - "Content-Type": "application/json", - [SDK_VERSION_HEADER_NAME]: this.sdkVersion, - Authorization: `Bearer ${this.publishableKey}`, - }, - body: serializedBody, - }); + try { + return await fetch(this.getUrl(path), { + ...this.fetchOptions, + method: "POST", + keepalive: shouldUseKeepalive, + headers: { + "Content-Type": "application/json", + [SDK_VERSION_HEADER_NAME]: this.sdkVersion, + Authorization: `Bearer ${this.publishableKey}`, + }, + body: serializedBody, + }); + } finally { + if (shouldUseKeepalive) { + this.inFlightKeepaliveBytes -= bodyBytes; + this.inFlightKeepaliveRequests -= 1; + } + } } } diff --git a/packages/browser-sdk/test/httpClient.test.ts b/packages/browser-sdk/test/httpClient.test.ts index af7cef9a..2573802e 100644 --- a/packages/browser-sdk/test/httpClient.test.ts +++ b/packages/browser-sdk/test/httpClient.test.ts @@ -87,6 +87,89 @@ describe("sets `credentials`", () => { ); }); + test("does not use keepalive when in-flight keepalive bytes would exceed the budget", async () => { + let resolveFirstFetch: ((value: Response) => void) | undefined; + vi.mocked(global.fetch) + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveFirstFetch = resolve; + }), + ) + .mockResolvedValueOnce(new Response()); + + const client = new HttpClient("publishableKey"); + const body = { payload: "x".repeat(35 * 1024) }; + + const firstRequest = client.post({ + path: "/test", + body, + keepalive: true, + }); + await Promise.resolve(); + + await client.post({ path: "/test", body, keepalive: true }); + + expect(vi.mocked(global.fetch).mock.calls[0]?.[1]).toEqual( + expect.objectContaining({ keepalive: true }), + ); + expect(vi.mocked(global.fetch).mock.calls[1]?.[1]).toEqual( + expect.objectContaining({ keepalive: false }), + ); + + resolveFirstFetch?.(new Response()); + await firstRequest; + }); + + test("does not use keepalive when the in-flight request count is at the limit", async () => { + const pendingRequests: Array<{ + resolve: (value: Response) => void; + }> = []; + let callCount = 0; + vi.mocked(global.fetch).mockImplementation(() => { + callCount += 1; + if (callCount > 15) { + return Promise.resolve(new Response()); + } + + let resolve!: (value: Response) => void; + const promise = new Promise((promiseResolve) => { + resolve = promiseResolve; + }); + pendingRequests.push({ resolve }); + return promise; + }); + + const client = new HttpClient("publishableKey"); + const body = { payload: "ok" }; + + const inFlightRequests = Array.from({ length: 15 }, () => + client.post({ + path: "/test", + body, + keepalive: true, + }), + ); + await Promise.resolve(); + + await client.post({ path: "/test", body, keepalive: true }); + + expect( + vi + .mocked(global.fetch) + .mock.calls.slice(0, 15) + .every(([, init]) => (init as RequestInit | undefined)?.keepalive), + ).toBe(true); + expect(vi.mocked(global.fetch).mock.calls[15]?.[1]).toEqual( + expect.objectContaining({ keepalive: false }), + ); + + for (const request of pendingRequests) { + request.resolve(new Response()); + } + await Promise.all(inFlightRequests); + }); + test("does not require a writable `URL.search` property", async () => { const OriginalURL = global.URL; From e51e29909e9577fd6fd2655284c810ef865dce95 Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Thu, 12 Mar 2026 12:48:53 +0100 Subject: [PATCH 12/16] Simplify keepalive budgeting --- packages/browser-sdk/src/httpClient.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/browser-sdk/src/httpClient.ts b/packages/browser-sdk/src/httpClient.ts index 2592b7e5..ed243a19 100644 --- a/packages/browser-sdk/src/httpClient.ts +++ b/packages/browser-sdk/src/httpClient.ts @@ -1,7 +1,6 @@ import { createAbortController } from "./utils/abortController"; import { API_BASE_URL, SDK_VERSION, SDK_VERSION_HEADER_NAME } from "./config"; -const KEEPALIVE_MAX_BODY_BYTES = 60 * 1024; const KEEPALIVE_MAX_IN_FLIGHT_BYTES = 60 * 1024; const KEEPALIVE_MAX_IN_FLIGHT_REQUESTS = 15; @@ -99,7 +98,6 @@ export class HttpClient { const bodyBytes = getBodyByteLength(serializedBody); const shouldUseKeepalive = keepalive && - bodyBytes <= KEEPALIVE_MAX_BODY_BYTES && this.inFlightKeepaliveBytes + bodyBytes <= KEEPALIVE_MAX_IN_FLIGHT_BYTES && this.inFlightKeepaliveRequests < KEEPALIVE_MAX_IN_FLIGHT_REQUESTS; From 23db3a980eebf8b6b3979e2e99c2a61919c48dc6 Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Thu, 12 Mar 2026 12:50:20 +0100 Subject: [PATCH 13/16] Fix feedback build return path --- packages/browser-sdk/src/feedback/feedback.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/browser-sdk/src/feedback/feedback.ts b/packages/browser-sdk/src/feedback/feedback.ts index 2b576f75..20e6c939 100644 --- a/packages/browser-sdk/src/feedback/feedback.ts +++ b/packages/browser-sdk/src/feedback/feedback.ts @@ -526,6 +526,8 @@ export class AutoFeedback { if (body.success && body.channel) { return body.channel; } + + return undefined; }); } catch (e) { this.logger.error(`error initializing automatic feedback`, e); From 987ce93a2902cf2ba35f2e6c3cc6a07f7283b72d Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Thu, 12 Mar 2026 14:20:55 +0100 Subject: [PATCH 14/16] Bump package versions for browser SDK changes --- packages/browser-sdk/package.json | 2 +- packages/openfeature-browser-provider/package.json | 4 ++-- packages/react-sdk/package.json | 4 ++-- packages/vue-sdk/package.json | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/browser-sdk/package.json b/packages/browser-sdk/package.json index 59b0290e..06ad9048 100644 --- a/packages/browser-sdk/package.json +++ b/packages/browser-sdk/package.json @@ -1,6 +1,6 @@ { "name": "@reflag/browser-sdk", - "version": "1.4.5", + "version": "1.4.6", "packageManager": "yarn@4.1.1", "license": "MIT", "repository": { diff --git a/packages/openfeature-browser-provider/package.json b/packages/openfeature-browser-provider/package.json index df73f3d9..9c3da16c 100644 --- a/packages/openfeature-browser-provider/package.json +++ b/packages/openfeature-browser-provider/package.json @@ -1,6 +1,6 @@ { "name": "@reflag/openfeature-browser-provider", - "version": "1.3.3", + "version": "1.3.4", "packageManager": "yarn@4.1.1", "license": "MIT", "repository": { @@ -35,7 +35,7 @@ } }, "dependencies": { - "@reflag/browser-sdk": "1.4.5" + "@reflag/browser-sdk": "1.4.6" }, "devDependencies": { "@openfeature/core": "1.5.0", diff --git a/packages/react-sdk/package.json b/packages/react-sdk/package.json index 487b53fd..8301639b 100644 --- a/packages/react-sdk/package.json +++ b/packages/react-sdk/package.json @@ -1,6 +1,6 @@ { "name": "@reflag/react-sdk", - "version": "1.4.5", + "version": "1.4.6", "license": "MIT", "repository": { "type": "git", @@ -37,7 +37,7 @@ } }, "dependencies": { - "@reflag/browser-sdk": "1.4.5" + "@reflag/browser-sdk": "1.4.6" }, "peerDependencies": { "react": "*", diff --git a/packages/vue-sdk/package.json b/packages/vue-sdk/package.json index d891ca78..d94a6090 100644 --- a/packages/vue-sdk/package.json +++ b/packages/vue-sdk/package.json @@ -1,6 +1,6 @@ { "name": "@reflag/vue-sdk", - "version": "1.3.3", + "version": "1.3.4", "license": "MIT", "repository": { "type": "git", @@ -35,7 +35,7 @@ } }, "dependencies": { - "@reflag/browser-sdk": "1.4.5" + "@reflag/browser-sdk": "1.4.6" }, "peerDependencies": { "vue": "^3.0.0" From e291c1567682f50dec5805573274bc7456deeb69 Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Thu, 12 Mar 2026 15:26:27 +0100 Subject: [PATCH 15/16] fix yarn.lock --- yarn.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/yarn.lock b/yarn.lock index 39ab0f2c..cd8c368f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5688,7 +5688,7 @@ __metadata: languageName: node linkType: hard -"@reflag/browser-sdk@npm:1.4.5, @reflag/browser-sdk@workspace:packages/browser-sdk": +"@reflag/browser-sdk@npm:1.4.6, @reflag/browser-sdk@workspace:packages/browser-sdk": version: 0.0.0-use.local resolution: "@reflag/browser-sdk@workspace:packages/browser-sdk" dependencies: @@ -5829,7 +5829,7 @@ __metadata: dependencies: "@openfeature/core": "npm:1.5.0" "@openfeature/web-sdk": "npm:^1.3.0" - "@reflag/browser-sdk": "npm:1.4.5" + "@reflag/browser-sdk": "npm:1.4.6" "@reflag/eslint-config": "npm:0.0.2" "@reflag/tsconfig": "npm:0.0.2" "@types/node": "npm:^22.12.0" @@ -5906,7 +5906,7 @@ __metadata: version: 0.0.0-use.local resolution: "@reflag/react-sdk@workspace:packages/react-sdk" dependencies: - "@reflag/browser-sdk": "npm:1.4.5" + "@reflag/browser-sdk": "npm:1.4.6" "@reflag/eslint-config": "npm:^0.0.2" "@reflag/tsconfig": "npm:^0.0.2" "@testing-library/react": "npm:^15.0.7" @@ -5966,7 +5966,7 @@ __metadata: version: 0.0.0-use.local resolution: "@reflag/vue-sdk@workspace:packages/vue-sdk" dependencies: - "@reflag/browser-sdk": "npm:1.4.5" + "@reflag/browser-sdk": "npm:1.4.6" "@reflag/eslint-config": "npm:^0.0.2" "@reflag/tsconfig": "npm:^0.0.2" "@types/jsdom": "npm:^21.1.6" From c34feed7c3b58f31de1b6b76d5ecf5ab64b7f5f5 Mon Sep 17 00:00:00 2001 From: Ron Cohen Date: Thu, 12 Mar 2026 15:35:24 +0100 Subject: [PATCH 16/16] Fix init test expectations --- packages/browser-sdk/test/init.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/browser-sdk/test/init.test.ts b/packages/browser-sdk/test/init.test.ts index 5591344e..fdc093b7 100644 --- a/packages/browser-sdk/test/init.test.ts +++ b/packages/browser-sdk/test/init.test.ts @@ -68,6 +68,9 @@ describe("init", () => { user: { id: "foo" }, apiBaseUrl: "https://example.com", enableTracking: false, + feedback: { + enableAutoFeedback: false, + }, }); await reflagInstance.initialize(); @@ -97,7 +100,6 @@ describe("init", () => { const reflagInstance = new ReflagClient({ publishableKey: KEY, user: { id: "foo" }, - apiBaseUrl: "https://example.com", enableTracking: false, feedback: { enableAutoFeedback: false,