Skip to content

Commit cf60ed8

Browse files
fix(session-replay-browser): skip bisect-retry for non-WAF 413 responses
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>
1 parent 71f8cb9 commit cf60ed8

3 files changed

Lines changed: 30 additions & 7 deletions

File tree

‎packages/session-replay-browser/src/track-destination.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,14 @@ export class SessionReplayTrackDestination implements AmplitudeSessionReplayTrac
385385
const source = isWaf ? 'WAF (compressed payload too large)' : 'server (event too large)';
386386
const totalSizeKB = Math.round(context.events.reduce((sum, e) => sum + e.length, 0) / KB_SIZE);
387387

388+
if (!isWaf) {
389+
this.completeRequest({
390+
context,
391+
err: `Session replay event batch dropped: ${source} rejected payload (${context.events.length} events, ${totalSizeKB} KB) — not retrying non-WAF 413`,
392+
});
393+
return;
394+
}
395+
388396
if (context.events.length === 1) {
389397
this.completeRequest({
390398
context,

‎packages/session-replay-browser/test/integration.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ describe('module level integration', () => {
247247
await runScheduleTimers();
248248
expect(fetch).toHaveBeenCalledTimes(1);
249249
// eslint-disable-next-line @typescript-eslint/unbound-method
250-
expect(mockLoggerProvider.warn).toHaveBeenCalledWith(expect.stringMatching(/single event.*cannot split further/));
250+
expect(mockLoggerProvider.warn).toHaveBeenCalledWith(expect.stringContaining('not retrying non-WAF 413'));
251251
});
252252
test('should handle retry for 500 error', async () => {
253253
const sessionReplay = new SessionReplay();

‎packages/session-replay-browser/test/track-destination.test.ts‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -585,13 +585,28 @@ describe('SessionReplayTrackDestination', () => {
585585
...overrides,
586586
});
587587

588-
test('drops single event and logs error with size', () => {
588+
test('drops non-WAF batch immediately without bisecting', () => {
589589
const trackDestination = new SessionReplayTrackDestination({ loggerProvider: mockLoggerProvider });
590590
const completeRequest = jest.spyOn(trackDestination, 'completeRequest').mockReturnValueOnce(undefined);
591-
const context = baseContext();
591+
const sendEventsList = jest.spyOn(trackDestination, 'sendEventsList');
592+
const context = baseContext({ events: ['event1', 'event2'] });
592593

593594
trackDestination.handlePayloadTooLargeResponse(context, false);
594595

596+
expect(completeRequest).toHaveBeenCalledWith({
597+
context,
598+
err: expect.stringContaining('not retrying non-WAF 413'),
599+
});
600+
expect(sendEventsList).not.toHaveBeenCalled();
601+
});
602+
603+
test('drops single WAF event and logs error with size', () => {
604+
const trackDestination = new SessionReplayTrackDestination({ loggerProvider: mockLoggerProvider });
605+
const completeRequest = jest.spyOn(trackDestination, 'completeRequest').mockReturnValueOnce(undefined);
606+
const context = baseContext();
607+
608+
trackDestination.handlePayloadTooLargeResponse(context, true);
609+
595610
expect(completeRequest).toHaveBeenCalledWith({
596611
context,
597612
err: expect.stringContaining('single event'),
@@ -611,15 +626,15 @@ describe('SessionReplayTrackDestination', () => {
611626
expect(errArg).toContain('WAF');
612627
});
613628

614-
test('bisects multi-event batch and re-enqueues both halves', () => {
629+
test('bisects multi-event WAF batch and re-enqueues both halves', () => {
615630
const trackDestination = new SessionReplayTrackDestination({ loggerProvider: mockLoggerProvider });
616631
const capturedOnCompletes: Array<() => Promise<void>> = [];
617632
jest.spyOn(trackDestination, 'sendEventsList').mockImplementation((dest) => {
618633
capturedOnCompletes.push(dest.onComplete);
619634
});
620635
const context = baseContext({ events: ['event1', 'event2', 'event3', 'event4'] });
621636

622-
trackDestination.handlePayloadTooLargeResponse(context, false);
637+
trackDestination.handlePayloadTooLargeResponse(context, true);
623638

624639
expect((trackDestination.sendEventsList as jest.Mock).mock.calls).toHaveLength(2);
625640
expect((trackDestination.sendEventsList as jest.Mock).mock.calls[0][0].events).toEqual(['event1', 'event2']);
@@ -630,12 +645,12 @@ describe('SessionReplayTrackDestination', () => {
630645
void Promise.all(capturedOnCompletes.map((fn) => fn()));
631646
});
632647

633-
test('logs warn with event count and size when bisecting', () => {
648+
test('logs warn with event count and size when bisecting WAF batch', () => {
634649
const trackDestination = new SessionReplayTrackDestination({ loggerProvider: mockLoggerProvider });
635650
jest.spyOn(trackDestination, 'sendEventsList').mockReturnValue(undefined);
636651
const context = baseContext({ events: ['event1', 'event2'] });
637652

638-
trackDestination.handlePayloadTooLargeResponse(context, false);
653+
trackDestination.handlePayloadTooLargeResponse(context, true);
639654

640655
// eslint-disable-next-line @typescript-eslint/unbound-method
641656
expect(mockLoggerProvider.warn).toHaveBeenCalledWith(expect.stringContaining('splitting'));

0 commit comments

Comments
 (0)