feat(session-replay-browser): custom transport hooks for authenticated proxies (SR-4497)#1862
Merged
Conversation
…d proxies (SR-4497) Add optional handleSendEvents / handleFetchConfig callbacks so customers can own the outbound HTTP call (attach a JWT, route through an authenticating proxy) while the SDK keeps batching, retry, URL resolution, compression and serialization. Covers every outbound Session Replay request: replay uploads, the page-exit beacon, the worker-delegated send, the remote-config GET, and the interaction/scroll beacon. handleFetchConfig is wired through RemoteConfigClient via a new optional, instance-scoped customFetch param in analytics-core. Rebuilt onto latest main (replaces the prior SR-4497-custom-transport-hooks branch); generated/version files excluded. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…R-4497) - Remote config: treat a 2xx status as success even when res.ok is absent, so a custom handleFetchConfig returning a Response-like (status/text/json, e.g. an axios adapter) isn't mistaken for a failure (Bugbot). - handleDelegatedFetch: warn on the defensive built-in-fetch fallback instead of silently falling back, so a regression that delegates without a transport is diagnosable (Bugbot). - Add beacon-transport tests covering the interaction/scroll custom-transport path (100% coverage). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
size-limit report 📦
|
Session Replay Browser E2E Results
Details
Flaky testschromium › e2e/idb.spec.ts › IDB transaction abort handling › getSequencesToSend cursor abort: AbortError logged at debug not warn |
…4497)
trc-url-rule.spec.ts:163 ("starts recording after SPA navigation") was
flaky independent of this branch — it fails ~2/3 attempts even on main
(main's last green run only passed it on retry 2). The full snapshot is
captured asynchronously after targeting flips recording on, so the single
flush(false) often ran before any rrweb event was queued and delivered
nothing, leaving getBodies() at 0.
Poll the flush until a batch actually reaches the track API (10s budget)
instead of one flush + fixed wait. flush(false) is a no-op on an empty
queue, so re-driving it is safe. Removes the snapshot-vs-flush race.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Worker delegation ignores abort signal
- Delegated worker requests now reject with AbortError on the send timeout signal, clear pending delegation state, and retry through the worker loop.
Or push these changes by commenting:
@cursor push 10246f4067
Preview (10246f4067)
diff --git a/packages/session-replay-browser/src/worker/track-destination.ts b/packages/session-replay-browser/src/worker/track-destination.ts
--- a/packages/session-replay-browser/src/worker/track-destination.ts
+++ b/packages/session-replay-browser/src/worker/track-destination.ts
@@ -51,8 +51,8 @@
// string | Uint8Array (not BodyInit) so it stays structured-cloneable across postMessage.
body: string | Uint8Array;
keepalive: boolean;
- // Abort signal for the send timeout. The built-in fetch path honors it; the delegation path
- // runs the request on the main thread, which has its own lifecycle, so it ignores the signal.
+ // Abort signal for the send timeout. Built-in fetch honors it natively; delegated requests
+ // reject in the worker when it fires so retry can continue even if the main-thread transport hangs.
signal?: AbortSignal;
}
@@ -88,13 +88,35 @@
const defaultRequest: DoRequest = (url, options) => fetch(url, options) as Promise<ResponseLike>;
+const createAbortError = () => {
+ if (typeof DOMException === 'function') {
+ return new DOMException('The operation was aborted.', 'AbortError');
+ }
+ const error = new Error('The operation was aborted.');
+ error.name = 'AbortError';
+ return error;
+};
+
// Posts a fetch-request to the main thread and resolves once the matching fetch-response
-// arrives. Rejects if the main-thread transport errored, so doFetch's catch surfaces it the
-// same way a thrown fetch would (no retry) — matching the non-worker custom-transport path.
+// arrives. Rejects if the main-thread transport errored or the worker-side timeout aborts it,
+// so doFetch's catch handles those cases the same way it handles a thrown fetch.
const delegateRequest: DoRequest = (url, options) => {
const requestId = `${++delegationCounter}`;
return new Promise<ResponseLike>((resolve, reject) => {
+ function cleanup() {
+ pendingDelegations.delete(requestId);
+ options.signal?.removeEventListener('abort', onAbort);
+ }
+ function onAbort() {
+ cleanup();
+ reject(createAbortError());
+ }
+ if (options.signal?.aborted) {
+ reject(createAbortError());
+ return;
+ }
pendingDelegations.set(requestId, (resp) => {
+ cleanup();
if (resp.error) {
reject(new Error(resp.body));
return;
@@ -110,6 +132,7 @@
text: () => Promise.resolve(resp.body),
});
});
+ options.signal?.addEventListener('abort', onAbort, { once: true });
postMessage({
type: 'fetch-request',
requestId,
@@ -169,7 +192,7 @@
// fetch() has no native timeout; abort a hung request so it surfaces as a retryable
// failure instead of silently wedging the worker (and the awaiting orchestrator). The
// request goes through doRequest so a custom transport (delegated to the main thread) is
- // honored; the built-in fetch path uses the abort signal, the delegated path ignores it.
+ // honored; both paths reject on abort so retry can continue after a timeout.
const controller = new AbortController();
const timeout =
sendTimeoutMs > 0
diff --git a/packages/session-replay-browser/test/worker/track-destination.test.ts b/packages/session-replay-browser/test/worker/track-destination.test.ts
--- a/packages/session-replay-browser/test/worker/track-destination.test.ts
+++ b/packages/session-replay-browser/test/worker/track-destination.test.ts
@@ -606,6 +606,54 @@
expect(mockPostMessage).toHaveBeenCalledWith({ type: 'complete', id: 'd3', skipCode: null });
});
+ test('times out a hung delegated attempt and retries through the worker retry loop', async () => {
+ const timers: Array<() => void> = [];
+ const setTimeoutSpy = jest.spyOn(global, 'setTimeout').mockImplementation((fn) => {
+ timers.push(fn as () => void);
+ return timers.length as unknown as ReturnType<typeof setTimeout>;
+ });
+ const randomSpy = jest.spyOn(global.Math, 'random').mockReturnValue(0);
+
+ try {
+ const sendPromise = invokeOnMessage({
+ type: 'send',
+ id: 'd3-timeout',
+ payload: basePayload,
+ context: { ...baseContext, flushMaxRetries: 2, sendTimeoutMs: 10 },
+ useRetry: true,
+ useCustomTransport: true,
+ });
+
+ let requests = postedOfType('fetch-request');
+ expect(requests).toHaveLength(1);
+
+ timers[0](); // send timeout aborts the hung delegated attempt
+ await Promise.resolve();
+ await Promise.resolve();
+
+ timers[1](); // retry backoff
+ await Promise.resolve();
+ await Promise.resolve();
+
+ requests = postedOfType('fetch-request');
+ expect(requests).toHaveLength(2);
+
+ await invokeOnMessage({
+ type: 'fetch-response',
+ requestId: requests[1].requestId,
+ status: 200,
+ skipHeader: null,
+ body: '',
+ });
+ await sendPromise;
+
+ expect(mockPostMessage).toHaveBeenCalledWith({ type: 'complete', id: 'd3-timeout', skipCode: null });
+ } finally {
+ setTimeoutSpy.mockRestore();
+ randomSpy.mockRestore();
+ }
+ });
+
test('a delegated 413 reads the response body and posts payload_too_large', async () => {
const sendPromise = invokeOnMessage({
type: 'send',You can send follow-ups to the cloud agent here.
…ation (SR-4497) In the useWebWorker + handleSendEvents path, doFetch armed the sendTimeoutMs AbortController and forwarded its signal to doRequest, but delegateRequest ignored it. A hung/never-settling custom transport therefore never aborted: the timer fired controller.abort() but nothing rejected the delegation promise, so sendWithRetry awaited forever and leaked the in-flight delegation (and the awaiting main-thread orchestrator). delegateRequest now listens for the abort and rejects with an AbortError — the same retryable path doFetch's catch already takes for the built-in fetch timeout — and removes the pending delegation so a late main-thread fetch-response is dropped. Adds a regression test that a hung delegated attempt settles instead of wedging the worker. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ath (SR-4497) The previous commit's delegateRequest abort handling used optional chaining on options.signal and a pre-aborted guard, but doFetch always supplies controller.signal and never aborts it synchronously — so those branches were unreachable and dropped global coverage below the 100% gate. Make RequestOptions.signal required (doFetch always sets it) and drop the dead pre-aborted check; the 'abort' listener already covers every real abort. Restores 100% statements/branches/lines. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Custom transport ignores send timeout
- The main-thread custom transport path now races the callback against the send timeout abort and rejects with AbortError so retry handling continues instead of hanging.
Or push these changes by commenting:
@cursor push 843be862da
Preview (843be862da)
diff --git a/packages/session-replay-browser/src/track-destination.ts b/packages/session-replay-browser/src/track-destination.ts
--- a/packages/session-replay-browser/src/track-destination.ts
+++ b/packages/session-replay-browser/src/track-destination.ts
@@ -81,6 +81,30 @@
// cap only guards the pathological case of a wedged worker that never replies at all.
const MAX_TIMED_OUT_WORKER_REQUESTS = 256;
+const awaitWithAbort = <T>(promise: Promise<T>, signal: AbortSignal): Promise<T> =>
+ new Promise<T>((resolve, reject) => {
+ const abort = () => {
+ const err = new Error('Session replay send aborted by send timeout');
+ err.name = 'AbortError';
+ reject(err);
+ };
+ if (signal.aborted) {
+ abort();
+ } else {
+ signal.addEventListener('abort', abort, { once: true });
+ }
+ promise.then(
+ (value) => {
+ signal.removeEventListener('abort', abort);
+ resolve(value);
+ },
+ (err) => {
+ signal.removeEventListener('abort', abort);
+ reject(err);
+ },
+ );
+ });
+
export class SessionReplayTrackDestination implements AmplitudeSessionReplayTrackDestination {
loggerProvider: ILogger;
storageKey = '';
@@ -807,10 +831,13 @@
try {
// When a custom transport is configured, hand it the fully-formed request and let it own
// the network call (e.g. to attach a JWT and route through a proxy). Otherwise use the
- // built-in fetch. Retry/response handling below is identical for both paths. The custom
- // transport owns its own timeout; controller.signal only aborts the built-in fetch.
+ // built-in fetch. Retry/response handling below is identical for both paths, and both
+ // observe the send timeout so a hung transport can't wedge the serial flush loop.
res = this.handleSendEvents
- ? await this.handleSendEvents({ url: serverUrl, method: 'POST', headers, body, keepalive })
+ ? await awaitWithAbort(
+ this.handleSendEvents({ url: serverUrl, method: 'POST', headers, body, keepalive }),
+ controller.signal,
+ )
: await fetch(serverUrl, options);
} finally {
// Clear on success and on error alike so a settled request never leaves an armed
diff --git a/packages/session-replay-browser/test/custom-transport.test.ts b/packages/session-replay-browser/test/custom-transport.test.ts
--- a/packages/session-replay-browser/test/custom-transport.test.ts
+++ b/packages/session-replay-browser/test/custom-transport.test.ts
@@ -107,6 +107,25 @@
// SDK-supplied headers are still present
expect((options.headers as Record<string, string>)['X-Client-Version']).toBe(VERSION);
});
+
+ test('times out a never-settling callback and retries', async () => {
+ const handleSendEvents = jest
+ .fn()
+ .mockImplementationOnce(() => new Promise<Response>(() => undefined))
+ .mockResolvedValueOnce({ status: 200 } as Response);
+ const trackDestination = new SessionReplayTrackDestination({
+ loggerProvider: mockLoggerProvider,
+ handleSendEvents,
+ sendTimeoutMs: 100,
+ });
+
+ const sendPromise = trackDestination.send(baseContext(), true);
+ await jest.runAllTimersAsync();
+ await sendPromise;
+
+ expect(handleSendEvents).toHaveBeenCalledTimes(2);
+ expect(fetch).not.toHaveBeenCalled();
+ });
});
describe('retry contract (callback sits below retry)', () => {You can send follow-ups to the cloud agent here.
…m transport (SR-4497) sendOnMainThread armed the sendTimeoutMs AbortController but, with a custom handleSendEvents, awaited the callback without tying the abort to it (and SendEventsRequest carries no signal). A hung/never-settling transport never rejected, so the serial flush loop could block indefinitely and skip the timeout retry — unlike the worker delegation path, which already rejects on abort. Wrap the callback in `abortable(...)`, which rejects with a retryable AbortError when the send-timeout fires (the transport's own promise keeps running; we just stop awaiting it). Matches the built-in fetch and worker paths. Adds a regression test that a hung callback settles via retry instead of wedging the flush loop. 100% coverage retained. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Config customFetch ignores timeout
- Wrapped remote-config customFetch in an abortable race so hung custom transports reject on timeout and allow retries/fetchPromise cleanup.
Or push these changes by commenting:
@cursor push 18cb6c3c6d
Preview (18cb6c3c6d)
diff --git a/packages/analytics-core/src/remote-config/remote-config.ts b/packages/analytics-core/src/remote-config/remote-config.ts
--- a/packages/analytics-core/src/remote-config/remote-config.ts
+++ b/packages/analytics-core/src/remote-config/remote-config.ts
@@ -42,6 +42,28 @@
*/
export type RemoteConfigCustomFetch = (request: RemoteConfigFetchRequest) => Promise<Response>;
+function abortable<T>(promise: Promise<T>, signal: AbortSignal): Promise<T> {
+ return new Promise<T>((resolve, reject) => {
+ const onAbort = () => {
+ const err = new Error('Custom remote config transport aborted by fetch timeout');
+ err.name = 'AbortError';
+ reject(err);
+ };
+
+ signal.addEventListener('abort', onAbort, { once: true });
+ promise.then(
+ (value) => {
+ signal.removeEventListener('abort', onAbort);
+ resolve(value);
+ },
+ (err) => {
+ signal.removeEventListener('abort', onAbort);
+ reject(err);
+ },
+ );
+ });
+}
+
export const US_SERVER_URL = 'https://sr-client-cfg.amplitude.com/config';
export const EU_SERVER_URL = 'https://sr-client-cfg.eu.amplitude.com/config';
export const DEFAULT_MAX_RETRIES = 3;
@@ -379,7 +401,10 @@
// When a custom transport is configured, hand it the fully-formed request and let it own
// the network call; otherwise use the built-in fetch. Retry/response handling is identical.
const res = this.customFetch
- ? await this.customFetch({ url, method: 'GET', headers, signal: abortController.signal })
+ ? await abortable(
+ this.customFetch({ url, method: 'GET', headers, signal: abortController.signal }),
+ abortController.signal,
+ )
: await fetch(url, {
method: 'GET',
headers,
diff --git a/packages/analytics-core/test/remote-config/remote-config.test.ts b/packages/analytics-core/test/remote-config/remote-config.test.ts
--- a/packages/analytics-core/test/remote-config/remote-config.test.ts
+++ b/packages/analytics-core/test/remote-config/remote-config.test.ts
@@ -1156,6 +1156,26 @@
expect(customFetch).toHaveBeenCalledTimes(2);
expect(result.remoteConfig).toEqual({ key: 'value' });
});
+
+ test('times out a hung custom transport and retries', async () => {
+ global.fetch = jest.fn();
+ const customFetch = jest
+ .fn()
+ .mockReturnValueOnce(new Promise<Response>(() => undefined))
+ .mockResolvedValueOnce({ ok: true, json: () => Promise.resolve({ key: 'value' }) } as Response);
+ const customClient = new RemoteConfigClient(mockApiKey, mockLogger, 'US', undefined, customFetch);
+
+ const result = await Promise.race([
+ customClient.fetch(2, 5),
+ new Promise<RemoteConfigInfo>((resolve) =>
+ setTimeout(() => resolve({ remoteConfig: { timedOut: true }, lastFetch: new Date(0) }), 100),
+ ),
+ ]);
+
+ expect(customFetch).toHaveBeenCalledTimes(2);
+ expect(result.remoteConfig).toEqual({ key: 'value' });
+ expect(loggerDebug).toHaveBeenCalledWith(expect.stringContaining('timed out after 5ms'));
+ });
});
describe('getUrlParams', () => {You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 8f2ecdc. Configure here.
jxiwang
approved these changes
Jun 29, 2026
…ransport (SR-4497) RemoteConfigClient.fetch passed the request-timeout signal to customFetch but awaited it directly, only calling AbortController.abort() on a timer. A transport that ignores the signal would block the await — and every retry — indefinitely. Race the callback against the abort (abortableFetch) so the client-side timeout always rejects a hung transport, matching the SR event- upload main-thread and worker paths. Adds hung-timeout and rejected-callback regression tests. 100% coverage retained. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds optional
handleSendEvents/handleFetchConfigcallbacks to the Session Replay Browser SDK so customers can fully own the outbound HTTP call — e.g. attach a JWTAuthorizationheader and route replay traffic through their own authenticating proxy — while the SDK keeps batching, retry/backoff, URL resolution, compression, serialization and error handling.Both default to the SDK's internal
fetch, so existing integrations are unchanged. The callback receives a fully-formed request (resolved URL respectingtrackServerUrl/configServerUrl, default headers, serialized body) and returns aResponse. It's invoked once per attempt; retry stays in the SDK around it.Closes SR-4497.
Covers all outbound Session Replay traffic
handleSendEventsis used for every outbound request, so nothing leaves the page unauthenticated:useWebWorker) — compression stays off the main thread; only the network call is delegated back to the main thread to run the callback, via afetch-request/fetch-responseprotocol, so retry semantics are identicalnavigator.sendBeaconcan't carry headers → routed through the callback withkeepalive)BeaconTransport)handleFetchConfig, wired throughRemoteConfigClientvia a new optional, instance-scopedcustomFetchparam inanalytics-core)API
Plugin and unified SDK accept the same two options. The README documents the browser side, the proxy side (which endpoints to forward to, preserve
Content-Encoding/body, keep the API-key auth), and all three integration paths.Notes / review feedback addressed
res.ok) so a Response-like adapter (e.g. axios) works consistently with the event path.warninstead of silently using the built-in fetch.string | Uint8Arrayto match the postMessage structured-clone constraint.fetch-requestand the main-thread upload then succeeds, the crash path keeps the store record for recovery (favoring no-data-loss over no-duplicate), so a rare duplicate batch is possible. Extremely narrow timing window; outcome is a possible duplicate, not loss. Proposed as a follow-up rather than a risky fix here.Testing
handleFetchConfigwire-through.200carrying the JWT, in both main-thread anduseWebWorkermodes; unauthenticated requests are rejected (no open relay).🤖 Generated with Claude Code
Note
Medium Risk
Changes every Session Replay outbound path and remote-config fetch when hooks are enabled; behavior is unchanged without them, but misconfigured proxies/auth can break capture or uploads.
Overview
Adds optional
handleSendEventsandhandleFetchConfigso customers can run outbound HTTP (e.g. JWT + authenticating proxy) while the SDK keeps batching, retry, compression, and URL resolution. Defaults stay on internalfetch.Session Replay routes all replay traffic through the callbacks when set: main-thread uploads, page-exit and interaction beacons (replacing
sendBeacon/XHR so headers work), and web-worker sends via afetch-request/fetch-responsedelegation to the main thread. Hung custom transports are raced against abort so timeouts and retries still apply.analytics-core extends
RemoteConfigClientwith an optional constructorcustomFetch(wired fromhandleFetchConfig); config success treats 2xx even ifres.okis missing.Docs/README cover proxy contracts; plugin/unified pass the same options; e2e TRC test polls flush to reduce flake; local sr-echo test harness added.
Reviewed by Cursor Bugbot for commit 99a096a. Bugbot is set up for automated code reviews on this repo. Configure here.