diff --git a/packages/replay/src/replay.ts b/packages/replay/src/replay.ts index ba55e339a611..c415754fd7a8 100644 --- a/packages/replay/src/replay.ts +++ b/packages/replay/src/replay.ts @@ -2,8 +2,7 @@ import { EventType, record } from '@sentry-internal/rrweb'; import { captureException } from '@sentry/core'; import type { Breadcrumb, ReplayRecordingMode } from '@sentry/types'; -import type { RateLimits } from '@sentry/utils'; -import { disabledUntil, logger } from '@sentry/utils'; +import { logger } from '@sentry/utils'; import { ERROR_CHECKOUT_TIME, @@ -40,7 +39,6 @@ import { isExpired } from './util/isExpired'; import { isSessionExpired } from './util/isSessionExpired'; import { overwriteRecordDroppedEvent, restoreRecordDroppedEvent } from './util/monkeyPatchRecordDroppedEvent'; import { sendReplay } from './util/sendReplay'; -import { RateLimitError } from './util/sendReplayRequest'; /** * The main replay container class, which holds all the state and methods for recording and sending replays. @@ -809,11 +807,6 @@ export class ReplayContainer implements ReplayContainerInterface { } catch (err) { this._handleException(err); - if (err instanceof RateLimitError) { - this._handleRateLimit(err.rateLimits); - return; - } - // This means we retried 3 times, and all of them failed // In this case, we want to completely stop the replay - otherwise, we may get inconsistent segments this.stop(); @@ -873,29 +866,4 @@ export class ReplayContainer implements ReplayContainerInterface { saveSession(this.session); } } - - /** - * Pauses the replay and resumes it after the rate-limit duration is over. - */ - private _handleRateLimit(rateLimits: RateLimits): void { - // in case recording is already paused, we don't need to do anything, as we might have already paused because of a - // rate limit - if (this.isPaused()) { - return; - } - - const rateLimitEnd = disabledUntil(rateLimits, 'replay'); - const rateLimitDuration = rateLimitEnd - Date.now(); - - if (rateLimitDuration > 0) { - __DEBUG_BUILD__ && logger.warn('[Replay]', `Rate limit hit, pausing replay for ${rateLimitDuration}ms`); - this.pause(); - this._debouncedFlush.cancel(); - - setTimeout(() => { - __DEBUG_BUILD__ && logger.info('[Replay]', 'Resuming replay after rate limit'); - this.resume(); - }, rateLimitDuration); - } - } } diff --git a/packages/replay/src/util/sendReplay.ts b/packages/replay/src/util/sendReplay.ts index 464dca008886..e393c8611866 100644 --- a/packages/replay/src/util/sendReplay.ts +++ b/packages/replay/src/util/sendReplay.ts @@ -2,7 +2,7 @@ import { captureException, setContext } from '@sentry/core'; import { RETRY_BASE_INTERVAL, RETRY_MAX_COUNT, UNABLE_TO_SEND_REPLAY } from '../constants'; import type { SendReplayData } from '../types'; -import { RateLimitError, sendReplayRequest, TransportStatusCodeError } from './sendReplayRequest'; +import { sendReplayRequest, TransportStatusCodeError } from './sendReplayRequest'; /** * Finalize and send the current replay event to Sentry @@ -25,7 +25,7 @@ export async function sendReplay( await sendReplayRequest(replayData); return true; } catch (err) { - if (err instanceof RateLimitError || err instanceof TransportStatusCodeError) { + if (err instanceof TransportStatusCodeError) { throw err; } diff --git a/packages/replay/src/util/sendReplayRequest.ts b/packages/replay/src/util/sendReplayRequest.ts index c98bc211d0b8..06e35330937e 100644 --- a/packages/replay/src/util/sendReplayRequest.ts +++ b/packages/replay/src/util/sendReplayRequest.ts @@ -1,7 +1,6 @@ import { getCurrentHub } from '@sentry/core'; import type { ReplayEvent, TransportMakeRequestResponse } from '@sentry/types'; -import type { RateLimits } from '@sentry/utils'; -import { isRateLimited, logger, updateRateLimits } from '@sentry/utils'; +import { logger } from '@sentry/utils'; import { REPLAY_EVENT_NAME, UNABLE_TO_SEND_REPLAY } from '../constants'; import type { SendReplayData } from '../types'; @@ -125,11 +124,6 @@ export async function sendReplayRequest({ return response; } - const rateLimits = updateRateLimits({}, response); - if (isRateLimited(rateLimits, 'replay')) { - throw new RateLimitError(rateLimits); - } - // If the status code is invalid, we want to immediately stop & not retry if (typeof response.statusCode === 'number' && (response.statusCode < 200 || response.statusCode >= 300)) { throw new TransportStatusCodeError(response.statusCode); @@ -138,18 +132,6 @@ export async function sendReplayRequest({ return response; } -/** - * This error indicates that we hit a rate limit API error. - */ -export class RateLimitError extends Error { - public rateLimits: RateLimits; - - public constructor(rateLimits: RateLimits) { - super('Rate limit hit'); - this.rateLimits = rateLimits; - } -} - /** * This error indicates that the transport returned an invalid status code. */ diff --git a/packages/replay/test/integration/rateLimiting.test.ts b/packages/replay/test/integration/rateLimiting.test.ts index ad8f810604b2..ad0d6cfbdb22 100644 --- a/packages/replay/test/integration/rateLimiting.test.ts +++ b/packages/replay/test/integration/rateLimiting.test.ts @@ -1,5 +1,5 @@ import { getCurrentHub } from '@sentry/core'; -import type { Transport, TransportMakeRequestResponse } from '@sentry/types'; +import type { Transport } from '@sentry/types'; import { DEFAULT_FLUSH_MIN_DELAY, SESSION_IDLE_DURATION } from '../../src/constants'; import type { ReplayContainer } from '../../src/replay'; @@ -64,120 +64,9 @@ describe('Integration | rate-limiting behaviour', () => { replay && replay.stop(); }); - it.each([ - { - statusCode: 429, - headers: { - 'x-sentry-rate-limits': '30', - 'retry-after': null, - }, - }, - { - statusCode: 429, - headers: { - 'x-sentry-rate-limits': '30:replay', - 'retry-after': null, - }, - }, - { - statusCode: 429, - headers: { - 'x-sentry-rate-limits': null, - 'retry-after': '30', - }, - }, - ] as TransportMakeRequestResponse[])( - 'pauses recording and flushing a rate limit is hit and resumes both after the rate limit duration is over %j', - async rateLimitResponse => { - expect(replay.session?.segmentId).toBe(0); - jest.spyOn(replay, 'pause'); - jest.spyOn(replay, 'resume'); - // @ts-ignore private API - jest.spyOn(replay, '_handleRateLimit'); - - const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 2 }; - - mockTransportSend.mockImplementationOnce(() => { - return Promise.resolve(rateLimitResponse); - }); - - mockRecord._emitter(TEST_EVENT); - - // T = base + 5 - await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); - - expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); - expect(mockTransportSend).toHaveBeenCalledTimes(1); - expect(replay).toHaveLastSentReplay({ recordingData: JSON.stringify([TEST_EVENT]) }); - - expect(replay['_handleRateLimit']).toHaveBeenCalledTimes(1); - // resume() was called once before we even started - expect(replay.resume).not.toHaveBeenCalled(); - expect(replay.pause).toHaveBeenCalledTimes(1); - - // No user activity to trigger an update - expect(replay.session?.lastActivity).toBe(BASE_TIMESTAMP); - expect(replay.session?.segmentId).toBe(1); - - // let's simulate the rate-limit time of inactivity (30secs) and check that we don't do anything in the meantime - const TEST_EVENT2 = { data: {}, timestamp: BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY, type: 3 }; - for (let i = 0; i < 5; i++) { - const ev = { - ...TEST_EVENT2, - timestamp: BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY * (i + 1), - }; - mockRecord._emitter(ev); - await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); - expect(replay.isPaused()).toBe(true); - expect(mockSendReplayRequest).toHaveBeenCalledTimes(1); - expect(mockTransportSend).toHaveBeenCalledTimes(1); - } - - // T = base + 35 - await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); - - // now, recording should resume and first, we expect a checkout event to be sent, as resume() - // should trigger a full snapshot - expect(replay.resume).toHaveBeenCalledTimes(1); - expect(replay.isPaused()).toBe(false); - - expect(mockSendReplayRequest).toHaveBeenCalledTimes(2); - expect(replay).toHaveLastSentReplay({ - recordingData: JSON.stringify([ - { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY * 7, type: 2 }, - ]), - }); - - // and let's also emit a new event and check that it is recorded - const TEST_EVENT3 = { - data: {}, - timestamp: BASE_TIMESTAMP + 7 * DEFAULT_FLUSH_MIN_DELAY, - type: 3, - }; - mockRecord._emitter(TEST_EVENT3); - - // T = base + 40 - await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); - expect(mockSendReplayRequest).toHaveBeenCalledTimes(3); - expect(replay).toHaveLastSentReplay({ recordingData: JSON.stringify([TEST_EVENT3]) }); - - // nothing should happen afterwards - // T = base + 60 - await advanceTimers(20_000); - expect(mockSendReplayRequest).toHaveBeenCalledTimes(3); - expect(replay).toHaveLastSentReplay({ recordingData: JSON.stringify([TEST_EVENT3]) }); - - // events array should be empty - expect(replay.eventBuffer?.hasEvents).toBe(false); - }, - ); - - it('handles rate-limits from a plain 429 response without any retry time', async () => { + it('handles rate-limit 429 responses by stopping the replay', async () => { expect(replay.session?.segmentId).toBe(0); - jest.spyOn(replay, 'pause'); - jest.spyOn(replay, 'resume'); - // @ts-ignore private API - jest.spyOn(replay, '_handleRateLimit'); + jest.spyOn(replay, 'stop'); const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 2 }; @@ -194,46 +83,23 @@ describe('Integration | rate-limiting behaviour', () => { expect(mockTransportSend).toHaveBeenCalledTimes(1); expect(replay).toHaveLastSentReplay({ recordingData: JSON.stringify([TEST_EVENT]) }); - expect(replay['_handleRateLimit']).toHaveBeenCalledTimes(1); - // resume() was called once before we even started - expect(replay.resume).not.toHaveBeenCalled(); - expect(replay.pause).toHaveBeenCalledTimes(1); + expect(replay.stop).toHaveBeenCalledTimes(1); // No user activity to trigger an update expect(replay.session?.lastActivity).toBe(BASE_TIMESTAMP); expect(replay.session?.segmentId).toBe(1); - // let's simulate the rate-limit time of inactivity (60secs) and check that we don't do anything in the meantime + // let's simulate the default rate-limit time of inactivity (60secs) and check that we + // don't do anything in the meantime or after the time has passed // 60secs are the default we fall back to in the plain 429 case in updateRateLimits() - const TEST_EVENT2 = { data: {}, timestamp: BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY, type: 3 }; - for (let i = 0; i < 11; i++) { - const ev = { - ...TEST_EVENT2, - timestamp: BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY * (i + 1), - }; - mockRecord._emitter(ev); - await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); - expect(replay.isPaused()).toBe(true); - expect(mockSendReplayRequest).toHaveBeenCalledTimes(1); - expect(mockTransportSend).toHaveBeenCalledTimes(1); - } // T = base + 60 - await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); - - // now, recording should resume and first, we expect a checkout event to be sent, as resume() - // should trigger a full snapshot - expect(replay.resume).toHaveBeenCalledTimes(1); - expect(replay.isPaused()).toBe(false); + await advanceTimers(DEFAULT_FLUSH_MIN_DELAY * 12); - expect(mockSendReplayRequest).toHaveBeenCalledTimes(2); - expect(replay).toHaveLastSentReplay({ - recordingData: JSON.stringify([ - { data: { isCheckout: true }, timestamp: BASE_TIMESTAMP + DEFAULT_FLUSH_MIN_DELAY * 13, type: 2 }, - ]), - }); + expect(mockSendReplayRequest).toHaveBeenCalledTimes(1); + expect(mockTransportSend).toHaveBeenCalledTimes(1); - // and let's also emit a new event and check that it is recorded + // and let's also emit a new event and check that it is not recorded const TEST_EVENT3 = { data: {}, timestamp: BASE_TIMESTAMP + 7 * DEFAULT_FLUSH_MIN_DELAY, @@ -241,52 +107,9 @@ describe('Integration | rate-limiting behaviour', () => { }; mockRecord._emitter(TEST_EVENT3); - // T = base + 65 - await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); - expect(mockSendReplayRequest).toHaveBeenCalledTimes(3); - expect(replay).toHaveLastSentReplay({ recordingData: JSON.stringify([TEST_EVENT3]) }); - - // nothing should happen afterwards - // T = base + 85 + // T = base + 80 await advanceTimers(20_000); - expect(mockSendReplayRequest).toHaveBeenCalledTimes(3); - expect(replay).toHaveLastSentReplay({ recordingData: JSON.stringify([TEST_EVENT3]) }); - - // events array should be empty - expect(replay.eventBuffer?.hasEvents).toBe(false); - }); - - it("doesn't do anything, if a rate limit is hit and recording is already paused", async () => { - let paused = false; - expect(replay.session?.segmentId).toBe(0); - jest.spyOn(replay, 'isPaused').mockImplementation(() => { - return paused; - }); - jest.spyOn(replay, 'pause'); - jest.spyOn(replay, 'resume'); - // @ts-ignore private API - jest.spyOn(replay, '_handleRateLimit'); - - const TEST_EVENT = { data: {}, timestamp: BASE_TIMESTAMP, type: 2 }; - - mockTransportSend.mockImplementationOnce(() => { - return Promise.resolve({ statusCode: 429 }); - }); - - mockRecord._emitter(TEST_EVENT); - paused = true; - - // T = base + 5 - await advanceTimers(DEFAULT_FLUSH_MIN_DELAY); - - expect(mockRecord.takeFullSnapshot).not.toHaveBeenCalled(); + expect(mockSendReplayRequest).toHaveBeenCalledTimes(1); expect(mockTransportSend).toHaveBeenCalledTimes(1); - - expect(replay).toHaveLastSentReplay({ recordingData: JSON.stringify([TEST_EVENT]) }); - - expect(replay['_handleRateLimit']).toHaveBeenCalledTimes(1); - expect(replay.resume).not.toHaveBeenCalled(); - expect(replay.isPaused).toHaveBeenCalledTimes(2); - expect(replay.pause).not.toHaveBeenCalled(); }); });