Skip to content

Commit 59c9614

Browse files
committed
Refine teardown handling for bulk queue
1 parent 7b1cd98 commit 59c9614

File tree

6 files changed

+65
-14
lines changed

6 files changed

+65
-14
lines changed

packages/browser-sdk/src/bulkQueue.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
BULK_QUEUE_RETRY_MAX_DELAY_MS,
77
} from "./config";
88
import { Logger } from "./logger";
9+
import { isPageLifecycleAbortError } from "./utils/pageLifecycle";
910

1011
const BULK_QUEUE_STORAGE_KEY = "__reflag_bulk_queue_v1";
1112
const WARN_AFTER_CONSECUTIVE_FAILURES = 10;
@@ -290,19 +291,27 @@ export class BulkQueue {
290291
this.retryCount += 1;
291292
const retryInMs = this.getRetryDelay();
292293
nextDelayMs = retryInMs;
293-
this.logger?.info("bulk retry scheduled", {
294+
const logDetails = {
294295
retryInMs,
295296
queueSize: this.queue.length + this.getInFlightBatchSize(),
296297
consecutiveFailures: this.consecutiveFailures,
297-
});
298+
};
299+
if (isPageLifecycleAbortError(error)) {
300+
this.logger?.debug("bulk retry scheduled (aborted during page teardown)", {
301+
...logDetails,
302+
error,
303+
});
304+
} else {
305+
this.logger?.info("bulk retry scheduled", logDetails);
306+
}
298307

299308
const outageMs = now - this.firstFailureAt;
300309
const shouldWarn =
301310
this.consecutiveFailures >= WARN_AFTER_CONSECUTIVE_FAILURES ||
302311
outageMs >= WARN_AFTER_FAILURE_MS;
303312
const canWarnNow =
304313
this.lastWarnAt === null || now - this.lastWarnAt >= WARN_THROTTLE_MS;
305-
if (shouldWarn && canWarnNow) {
314+
if (shouldWarn && canWarnNow && !isPageLifecycleAbortError(error)) {
306315
this.logger?.warn("bulk delivery degraded", {
307316
consecutiveFailures: this.consecutiveFailures,
308317
outageMs,

packages/browser-sdk/src/client.ts

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
RawFlags,
1717
} from "./flag/flags";
1818
import { ToolbarPosition } from "./ui/types";
19+
import { logResponseError } from "./utils/responseError";
1920
import { BulkEvent, BulkQueue } from "./bulkQueue";
2021
import {
2122
API_BASE_URL,
@@ -29,8 +30,6 @@ import { HttpClient } from "./httpClient";
2930
import { Logger, loggerWithPrefix, quietConsoleLogger } from "./logger";
3031
import { StorageAdapter } from "./storage";
3132
import { showToolbarToggle } from "./toolbar";
32-
import { logResponseError } from "./utils/responseError";
33-
import { logLifecycleAwareNetworkError } from "./utils/pageLifecycle";
3433

3534
const isMobile = typeof window !== "undefined" && window.innerWidth < 768;
3635
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 {
572571
if (!this.config.bootstrapped) {
573572
if (this.context.user && this.config.enableTracking) {
574573
this.user().catch((e) => {
575-
logLifecycleAwareNetworkError(this.logger, "error sending user", e);
574+
this.logger.error("error sending user", e);
576575
});
577576
}
578577

579578
if (this.context.company && this.config.enableTracking) {
580579
this.company().catch((e) => {
581-
logLifecycleAwareNetworkError(
582-
this.logger,
583-
"error sending company",
584-
e,
585-
);
580+
this.logger.error("error sending company", e);
586581
});
587582
}
588583
}

packages/browser-sdk/src/feedback/feedback.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ import { HttpClient } from "../httpClient";
33
import { Logger } from "../logger";
44
import { AblySSEChannel, openAblySSEChannel } from "../sse";
55
import { Position } from "../ui/types";
6-
import { logResponseError } from "../utils/responseError";
76
import { logLifecycleAwareNetworkError } from "../utils/pageLifecycle";
7+
import { logResponseError } from "../utils/responseError";
88

99
import {
1010
FeedbackSubmission,

packages/browser-sdk/src/flag/flags.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ import RateLimiter from "../rateLimiter";
99
import { getDefaultStorageAdapter, StorageAdapter } from "../storage";
1010
import { createAbortController } from "../utils/abortController";
1111
import { createEventTarget } from "../utils/eventTarget";
12-
import { logResponseError, parseResponseError } from "../utils/responseError";
1312
import { logLifecycleAwareNetworkError } from "../utils/pageLifecycle";
13+
import { logResponseError, parseResponseError } from "../utils/responseError";
1414

1515
import { FlagCache, isObject, parseAPIFlagsResponse } from "./flagCache";
1616

packages/browser-sdk/test/bulkQueue.test.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,51 @@ describe("BulkQueue", () => {
8888
expect(sendBulk).toHaveBeenNthCalledWith(2, [trackEvent]);
8989
});
9090

91+
it("requeues and downgrades page teardown aborts for bulk sends", async () => {
92+
const logger = {
93+
debug: vi.fn(),
94+
info: vi.fn(),
95+
warn: vi.fn(),
96+
error: vi.fn(),
97+
};
98+
const sendBulk = vi
99+
.fn<(events: BulkEvent[]) => Promise<Response>>()
100+
.mockRejectedValueOnce(new TypeError("Failed to fetch"))
101+
.mockResolvedValue(new Response("", { status: 200 }));
102+
const queue = new BulkQueue(sendBulk, {
103+
flushDelayMs: 10,
104+
retryBaseDelayMs: 20,
105+
retryMaxDelayMs: 20,
106+
logger,
107+
});
108+
109+
window.dispatchEvent(new Event("pagehide"));
110+
await queue.enqueue(trackEvent);
111+
112+
await vi.advanceTimersByTimeAsync(10);
113+
expect(sendBulk).toHaveBeenCalledTimes(1);
114+
expect(await queue.size()).toBe(1);
115+
expect(logger.debug).toHaveBeenCalledWith(
116+
"bulk retry scheduled (aborted during page teardown)",
117+
expect.objectContaining({
118+
retryInMs: 20,
119+
queueSize: 2,
120+
consecutiveFailures: 1,
121+
error: expect.any(TypeError),
122+
}),
123+
);
124+
expect(logger.info).not.toHaveBeenCalledWith(
125+
"bulk retry scheduled",
126+
expect.anything(),
127+
);
128+
expect(logger.warn).not.toHaveBeenCalled();
129+
130+
window.dispatchEvent(new Event("pageshow"));
131+
await vi.advanceTimersByTimeAsync(20);
132+
expect(sendBulk).toHaveBeenCalledTimes(2);
133+
expect(sendBulk).toHaveBeenNthCalledWith(2, [trackEvent]);
134+
});
135+
91136
it("drops 4xx responses, logs error, and does not retry", async () => {
92137
const sendBulk = vi
93138
.fn<(events: BulkEvent[]) => Promise<Response>>()

packages/browser-sdk/test/flags.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,9 @@ describe("FlagsClient", () => {
200200
test("downgrades page teardown fetch failures to debug logs", async () => {
201201
const { newFlagsClient, httpClient } = flagsClientFactory();
202202

203-
vi.mocked(httpClient.get).mockRejectedValue(new TypeError("Failed to fetch"));
203+
vi.mocked(httpClient.get).mockRejectedValue(
204+
new TypeError("Failed to fetch"),
205+
);
204206
window.dispatchEvent(new Event("pagehide"));
205207

206208
const flagsClient = newFlagsClient();

0 commit comments

Comments
 (0)