fix(session-replay-browser): reduce snapshot size and handle 413s#1685
fix(session-replay-browser): reduce snapshot size and handle 413s#1685lewgordon-amplitude merged 12 commits intomainfrom
Conversation
Session Replay Browser E2E ResultsDetails
|
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit f4b94d0. Configure here.
71f8cb9 to
b920cfb
Compare
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: Non-WAF 413s bisected despite stated no-retry intent
- Added control-flow check to skip bisection for non-WAF 413 responses, immediately dropping the batch instead of wasting O(2N-1) network requests.
Or push these changes by commenting:
@cursor push 8dae6093b6
Preview (8dae6093b6)
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
@@ -396,6 +396,14 @@
const source = isWaf ? 'WAF (compressed payload too large)' : 'server (event too large)';
const totalSizeKB = Math.round(context.events.reduce((sum, e) => sum + e.length, 0) / KB_SIZE);
+ if (!isWaf) {
+ this.completeRequest({
+ context,
+ err: `Session replay event batch dropped: ${source} rejected payload (${context.events.length} events, ${totalSizeKB} KB) — not retrying non-WAF 413`,
+ });
+ return;
+ }
+
if (context.events.length === 1) {
this.completeRequest({
context,You can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
1 issue from previous review remains unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit ab7931b. Configure here.
ab7931b to
cf60ed8
Compare
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: E2E test response body missing WAF signature string
- Updated both test cases to use the correct WAF 413 response body containing 'Payload exceeds' substring to properly trigger bisect-retry behavior
Or push these changes by commenting:
@cursor push 4bc4ffcc2b
Preview (4bc4ffcc2b)
diff --git a/packages/session-replay-browser/e2e/capture.spec.ts b/packages/session-replay-browser/e2e/capture.spec.ts
--- a/packages/session-replay-browser/e2e/capture.spec.ts
+++ b/packages/session-replay-browser/e2e/capture.spec.ts
@@ -1308,7 +1308,7 @@
callCount++;
if (callCount === 1) {
// Simulate the server rejecting the initial payload as too large
- await route.fulfill({ status: 413, contentType: 'application/json', body: '{"code":413}' });
+ await route.fulfill({ status: 413, contentType: 'application/json', body: '{"error":"Payload exceeds the maximum allowed size of 10MB"}' });
} else {
deliveredBodies.push(readRouteBody(route));
await route.fulfill({ status: 200, contentType: 'application/json', body: JSON.stringify(SR_API_SUCCESS) });
@@ -1339,7 +1339,7 @@
await page.route('https://api-sr.amplitude.com/**', async (route: Route) => {
callCount++;
if (callCount <= 2) {
- await route.fulfill({ status: 413, contentType: 'application/json', body: '{"code":413}' });
+ await route.fulfill({ status: 413, contentType: 'application/json', body: '{"error":"Payload exceeds the maximum allowed size of 10MB"}' });
} else {
deliveredBodies.push(readRouteBody(route));
await route.fulfill({ status: 200, contentType: 'application/json', body: JSON.stringify(SR_API_SUCCESS) });You can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 6a3431e. Configure here.
f9d9f21 to
4aaab66
Compare
4aaab66 to
276355e
Compare
…p oversized events at capture - Add MAX_SINGLE_EVENT_SIZE guard in EventCompressor to silently drop events that exceed the server's size limit before sending - Bisect-retry logic in track-destination: on a WAF 413 (body contains "Payload exceeds"), split the batch in half and retry each half independently; bottom out at single events to avoid infinite loops - Add MAX_SINGLE_EVENT_SIZE constant (9 MB) - Add e2e tests covering oversized capture-time drop and 413 retry - Move e2e test server to port 5174 to isolate worktree from main-branch Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… 413 useRetry inconsistency - Enable all rrweb slimDOMOptions by default (scripts, comments, all head meta/favicon/whitespace tags) — none affect replay fidelity - Remove omitElementTags from public config; stripping is now unconditional - Fix worker sendWithRetry: only bisect on 413 when useRetry=true, matching main-thread behavior for flush(false) calls Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…is always enabled Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…in oversized-event warnings Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Non-WAF (app-layer) 413s are rejected by the server at any payload size, so bisecting and retrying wastes O(2N-1) network requests before every event is individually dropped. Guard handlePayloadTooLargeResponse with an isWaf check to immediately drop the batch for non-WAF 413s. Update unit and integration tests to reflect the new non-WAF immediate-drop behavior; WAF 413 bisection behavior is unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The bisect-retry path is now only triggered for WAF 413s (body contains 'Payload exceeds'). Update the E2E test route mocks to return a proper WAF response body so the bisect-retry tests actually exercise the WAF code path rather than the immediate-drop non-WAF path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…r unconditional slim DOM After removing the omitElementTags config, the rrweb record options now hard-code the full slim DOM block. Update the child-mode test to assert the new shape instead of the removed config flag. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Broaden WAF detection to a case-insensitive /payload.*exceed/i regex so vendor wording tweaks don't silently disable bisect-retry. Centralised as WAF_PAYLOAD_TOO_LARGE_PATTERN in constants and used from both the main-thread and worker paths. - Compare UTF-8 byte size (Blob) against MAX_SINGLE_EVENT_SIZE instead of JS char count, so multi-byte payloads (CJK, emoji) are gated correctly. - size-limits e2e: derive OVERSIZED_ATTR_LENGTH from MAX_SINGLE_EVENT_SIZE so the test threshold tracks the constant if it's bumped. - size-limits e2e: tighten the app-layer 413 "no infinite loop" assertion to <= 2 (was < 20) — the immediate-drop path produces at most one tracking call, with slack for an unrelated follow-up flush. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The port change was a local-dev convenience to coexist with another worktree. It broke the urlMaskLevels e2e suite because privacy.spec.ts hardcodes localhost:5173 in MATCHING_PATTERN, so URL rules never matched on 5174 and inputs fell back to the default mask level. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ter rebase After rebasing onto main (which now contains #1703), the 20MB MAX_FULL_SNAPSHOT_SIZE guard is redundant and incorrect: - The 9MB MAX_SINGLE_EVENT_SIZE per-event guard already drops any oversized event at capture, including FullSnapshots, before they reach the network. - 20MB also exceeds the server's 10MB per-event limit, so allowing it through never actually helped. Also removes the duplicate `case Status.PayloadTooLarge` left behind by an auto-merge, and drops the obsolete worker bisect tests — the worker no longer bisects internally; it reports 413 to the main thread which handles the retry. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6044820 to
3275363
Compare
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 3275363. Configure here.
…atch handler Address PR review feedback: attach a .catch to store.cleanUpSessionEventsStore in the all-oversized branch, matching the surrounding error-handling pattern in events-manager.ts (addEvent etc.) instead of silently swallowing rejections. Co-Authored-By: Claude Opus 4.7 (1M context) <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: Bisect-retry log reports char count as KB size
- Changed totalSizeKB calculation from e.length (char count) to new Blob([e]).size (UTF-8 byte count) for accurate size reporting.
Or push these changes by commenting:
@cursor push dbb3658492
Preview (dbb3658492)
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
@@ -394,7 +394,7 @@
handlePayloadTooLargeResponse(context: SessionReplayDestinationContext, isWaf: boolean): void {
const source = isWaf ? 'WAF (compressed payload too large)' : 'server (event too large)';
- const totalSizeKB = Math.round(context.events.reduce((sum, e) => sum + e.length, 0) / KB_SIZE);
+ const totalSizeKB = Math.round(context.events.reduce((sum, e) => sum + new Blob([e]).size, 0) / KB_SIZE);
if (!isWaf) {
this.completeRequest({You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit c6639b3. Configure here.


Summary
Reduce snapshot payload size
slimDOMOptionsby default: scripts, comments, favicons, head whitespace, and all head meta tags (description/keywords, social, robots, http-equiv, authorship, verification) are now stripped from snapshots. None of these affect replay fidelity.omitElementTagsconfig option (was only wiring upscriptandcomment; now all slim-DOM stripping is unconditional)WAF 413 bisect-retry
"Payload exceeds"), split the event batch in half and retry each half independently; recurse until batches reach a single event, then stop and warnCapture-time drop for oversized single events
MAX_SINGLE_EVENT_SIZEconstant (9 MB)EventCompressor.addCompressedEventToManagerdrops any serialized event exceeding the limit before it's queued — avoids a wasted network round trip for snapshots that would fail the server's ~10 MB limit anywayTests
size-limits.spec.ts): oversized event not delivered, SDK recovers after drop, WAF 413 triggers bisect-retry, app-layer 413 doesn't loopChecklist
omitElementTagsremoved from public config (was undocumented/unused in practice)Note
Medium Risk
Changes core replay delivery behavior (per-event dropping and 413 retry/bisect logic) across both main-thread and web-worker send paths, which could affect data loss vs. retry behavior if misclassified. Also removes the
omitElementTagsoption, which is a small breaking config change.Overview
Reduces session replay payload size and adds stricter size limits. rrweb
slimDOMOptionsare now enabled broadly by default (scripts/comments plus various<head>artifacts), and the publicomitElementTagsoption is removed from both plugin and standalone configs.Adds per-event size guarding and safer 413 handling. A new
MAX_SINGLE_EVENT_SIZE(9MB, UTF-8 bytes) drops oversized serialized events at capture time and again before send (to protect against older/IDB-stored events). 413 responses are now classified as WAF vs app-layer viaWAF_PAYLOAD_TOO_LARGE_PATTERN: WAF 413 triggers split-and-retry of the batch, while non-WAF 413 is dropped without retry to avoid loops, in both main-thread and worker delivery.Expands test coverage. Adds unit/e2e coverage for oversized-event dropping, WAF bisect-retry, and ensuring the SDK remains functional after 413s (including new
size-limits.spec.ts).Reviewed by Cursor Bugbot for commit c6639b3. Bugbot is set up for automated code reviews on this repo. Configure here.