Skip to content

Commit a2f7fad

Browse files
harshit078chargome
andauthored
feat(replay): Update client report discard reason for invalid sessions (#18796)
This PR distinguishes replays that get dropped due to session length vs. transport/ratelimit reasons in the client report. closes #18316 --------- Co-authored-by: Charly Gomez <charly.gomez@sentry.io>
1 parent 6119cf2 commit a2f7fad

5 files changed

Lines changed: 157 additions & 8 deletions

File tree

packages/replay-internal/src/replay.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ import { debug } from './util/logger';
5555
import { resetReplayIdOnDynamicSamplingContext } from './util/resetReplayIdOnDynamicSamplingContext';
5656
import { closestElementOfNode } from './util/rrweb';
5757
import { sendReplay } from './util/sendReplay';
58-
import { RateLimitError } from './util/sendReplayRequest';
58+
import { RateLimitError, ReplayDurationLimitError } from './util/sendReplayRequest';
5959
import type { SKIPPED } from './util/throttle';
6060
import { throttle, THROTTLED } from './util/throttle';
6161

@@ -1185,7 +1185,7 @@ export class ReplayContainer implements ReplayContainerInterface {
11851185
// We leave 30s wiggle room to accommodate late flushing etc.
11861186
// This _could_ happen when the browser is suspended during flushing, in which case we just want to stop
11871187
if (timestamp - this._context.initialTimestamp > this._options.maxReplayDuration + 30_000) {
1188-
throw new Error('Session is too long, not sending replay');
1188+
throw new ReplayDurationLimitError();
11891189
}
11901190

11911191
const eventContext = this._popEventContext();
@@ -1218,7 +1218,14 @@ export class ReplayContainer implements ReplayContainerInterface {
12181218
const client = getClient();
12191219

12201220
if (client) {
1221-
const dropReason = err instanceof RateLimitError ? 'ratelimit_backoff' : 'send_error';
1221+
let dropReason: 'ratelimit_backoff' | 'send_error' | 'invalid';
1222+
if (err instanceof RateLimitError) {
1223+
dropReason = 'ratelimit_backoff';
1224+
} else if (err instanceof ReplayDurationLimitError) {
1225+
dropReason = 'invalid';
1226+
} else {
1227+
dropReason = 'send_error';
1228+
}
12221229
client.recordDroppedEvent(dropReason, 'replay');
12231230
}
12241231
}

packages/replay-internal/src/util/sendReplayRequest.ts

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -117,16 +117,17 @@ export async function sendReplayRequest({
117117
throw error;
118118
}
119119

120-
// If the status code is invalid, we want to immediately stop & not retry
121-
if (typeof response.statusCode === 'number' && (response.statusCode < 200 || response.statusCode >= 300)) {
122-
throw new TransportStatusCodeError(response.statusCode);
123-
}
124-
120+
// Check for rate limiting first (handles 429 and rate limit headers)
125121
const rateLimits = updateRateLimits({}, response);
126122
if (isRateLimited(rateLimits, 'replay')) {
127123
throw new RateLimitError(rateLimits);
128124
}
129125

126+
// If the status code is invalid, we want to immediately stop & not retry
127+
if (typeof response.statusCode === 'number' && (response.statusCode < 200 || response.statusCode >= 300)) {
128+
throw new TransportStatusCodeError(response.statusCode);
129+
}
130+
130131
return response;
131132
}
132133

@@ -150,3 +151,13 @@ export class RateLimitError extends Error {
150151
this.rateLimits = rateLimits;
151152
}
152153
}
154+
155+
/**
156+
* This error indicates that the replay duration limit was exceeded and the session is too long.
157+
*
158+
*/
159+
export class ReplayDurationLimitError extends Error {
160+
public constructor() {
161+
super('Session is too long, not sending replay');
162+
}
163+
}

packages/replay-internal/test/integration/flush.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,49 @@ describe('Integration | flush', () => {
489489
await replay.start();
490490
});
491491

492+
/**
493+
* This tests that when a replay exceeds maxReplayDuration,
494+
* the dropped event is recorded with the 'invalid' reason
495+
* to distinguish it from actual send errors.
496+
*/
497+
it('records dropped event with invalid reason when session exceeds maxReplayDuration', async () => {
498+
const client = SentryUtils.getClient()!;
499+
const recordDroppedEventSpy = vi.spyOn(client, 'recordDroppedEvent');
500+
501+
replay.getOptions().maxReplayDuration = 100_000;
502+
503+
sessionStorage.clear();
504+
clearSession(replay);
505+
replay['_initializeSessionForSampling']();
506+
replay.setInitialState();
507+
await new Promise(process.nextTick);
508+
vi.setSystemTime(BASE_TIMESTAMP);
509+
510+
replay.eventBuffer!.clear();
511+
512+
replay.eventBuffer!.hasCheckout = true;
513+
514+
replay['_addPerformanceEntries'] = () => {
515+
return new Promise(resolve => setTimeout(resolve, 140_000));
516+
};
517+
518+
const TEST_EVENT = getTestEventCheckout({ timestamp: BASE_TIMESTAMP + 100 });
519+
mockRecord._emitter(TEST_EVENT);
520+
521+
await vi.advanceTimersByTimeAsync(160_000);
522+
523+
expect(mockFlush).toHaveBeenCalledTimes(1);
524+
expect(mockSendReplay).toHaveBeenCalledTimes(0);
525+
expect(replay.isEnabled()).toBe(false);
526+
527+
expect(recordDroppedEventSpy).toHaveBeenCalledWith('invalid', 'replay');
528+
529+
replay.getOptions().maxReplayDuration = MAX_REPLAY_DURATION;
530+
recordDroppedEventSpy.mockRestore();
531+
532+
await replay.start();
533+
});
534+
492535
it('resets flush lock if runFlush rejects/throws', async () => {
493536
mockRunFlush.mockImplementation(
494537
() =>

packages/replay-internal/test/integration/rateLimiting.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,4 +113,42 @@ describe('Integration | rate-limiting behaviour', () => {
113113
expect(replay.session).toBeDefined();
114114
expect(replay.isEnabled()).toBe(true);
115115
});
116+
117+
it('records dropped event with ratelimit_backoff reason when rate limited', async () => {
118+
const client = getClient()!;
119+
const recordDroppedEventSpy = vi.spyOn(client, 'recordDroppedEvent');
120+
121+
mockTransportSend.mockImplementationOnce(() => {
122+
return Promise.resolve({ statusCode: 429, headers: { 'retry-after': '10' } } as TransportMakeRequestResponse);
123+
});
124+
125+
replay.start();
126+
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);
127+
128+
expect(replay.isEnabled()).toBe(false);
129+
expect(recordDroppedEventSpy).toHaveBeenCalledWith('ratelimit_backoff', 'replay');
130+
131+
recordDroppedEventSpy.mockRestore();
132+
});
133+
134+
it('records dropped event with send_error reason when transport fails', async () => {
135+
const client = getClient()!;
136+
const recordDroppedEventSpy = vi.spyOn(client, 'recordDroppedEvent');
137+
138+
mockTransportSend.mockImplementation(() => {
139+
return Promise.reject(new Error('Network error'));
140+
});
141+
142+
replay.start();
143+
await advanceTimers(DEFAULT_FLUSH_MIN_DELAY);
144+
145+
await advanceTimers(5000);
146+
await advanceTimers(10000);
147+
await advanceTimers(30000);
148+
149+
expect(replay.isEnabled()).toBe(false);
150+
expect(recordDroppedEventSpy).toHaveBeenCalledWith('send_error', 'replay');
151+
152+
recordDroppedEventSpy.mockRestore();
153+
});
116154
});
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import { describe, expect, it } from 'vitest';
2+
import {
3+
RateLimitError,
4+
ReplayDurationLimitError,
5+
TransportStatusCodeError,
6+
} from '../../../src/util/sendReplayRequest';
7+
8+
describe('Unit | util | sendReplayRequest', () => {
9+
describe('TransportStatusCodeError', () => {
10+
it('creates error with correct message', () => {
11+
const error = new TransportStatusCodeError(500);
12+
expect(error.message).toBe('Transport returned status code 500');
13+
expect(error).toBeInstanceOf(Error);
14+
});
15+
});
16+
17+
describe('RateLimitError', () => {
18+
it('creates error with correct message and stores rate limits', () => {
19+
const rateLimits = { all: 1234567890 };
20+
const error = new RateLimitError(rateLimits);
21+
expect(error.message).toBe('Rate limit hit');
22+
expect(error.rateLimits).toBe(rateLimits);
23+
expect(error).toBeInstanceOf(Error);
24+
});
25+
});
26+
27+
describe('ReplayDurationLimitError', () => {
28+
it('creates error with correct message', () => {
29+
const error = new ReplayDurationLimitError();
30+
expect(error.message).toBe('Session is too long, not sending replay');
31+
expect(error).toBeInstanceOf(Error);
32+
});
33+
34+
it('is distinguishable from other error types', () => {
35+
const durationError = new ReplayDurationLimitError();
36+
const rateLimitError = new RateLimitError({ all: 123 });
37+
const transportError = new TransportStatusCodeError(500);
38+
39+
expect(durationError instanceof ReplayDurationLimitError).toBe(true);
40+
expect(durationError instanceof RateLimitError).toBe(false);
41+
expect(durationError instanceof TransportStatusCodeError).toBe(false);
42+
43+
expect(rateLimitError instanceof ReplayDurationLimitError).toBe(false);
44+
expect(rateLimitError instanceof RateLimitError).toBe(true);
45+
46+
expect(transportError instanceof ReplayDurationLimitError).toBe(false);
47+
expect(transportError instanceof TransportStatusCodeError).toBe(true);
48+
});
49+
});
50+
});

0 commit comments

Comments
 (0)