diff --git a/packages/browser/src/transports/fetch.ts b/packages/browser/src/transports/fetch.ts index 83b75c82ba39..e996b6f8277a 100644 --- a/packages/browser/src/transports/fetch.ts +++ b/packages/browser/src/transports/fetch.ts @@ -13,7 +13,14 @@ export function makeFetchTransport( options: BrowserTransportOptions, nativeFetch: FetchImpl = getNativeFetchImplementation(), ): Transport { + let pendingBodySize = 0; + let pendingCount = 0; + function makeRequest(request: TransportRequest): PromiseLike { + const requestSize = request.body.length; + pendingBodySize += requestSize; + pendingCount++; + const requestOptions: RequestInit = { body: request.body, method: 'POST', @@ -25,23 +32,31 @@ export function makeFetchTransport( // frequently sending events right before the user is switching pages (eg. whenfinishing navigation transactions). // Gotchas: // - `keepalive` isn't supported by Firefox - // - As per spec (https://fetch.spec.whatwg.org/#http-network-or-cache-fetch), a request with `keepalive: true` - // and a content length of > 64 kibibytes returns a network error. We will therefore only activate the flag when - // we're below that limit. - keepalive: request.body.length <= 65536, + // - As per spec (https://fetch.spec.whatwg.org/#http-network-or-cache-fetch): + // If the sum of contentLength and inflightKeepaliveBytes is greater than 64 kibibytes, then return a network error. + // We will therefore only activate the flag when we're below that limit. + // There is also a limit of requests that can be open at the same time, so we also limit this to 15 + // See https://github.com/getsentry/sentry-javascript/pull/7553 for details + keepalive: pendingBodySize <= 60_000 && pendingCount < 15, ...options.fetchOptions, }; try { - return nativeFetch(options.url, requestOptions).then(response => ({ - statusCode: response.status, - headers: { - 'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'), - 'retry-after': response.headers.get('Retry-After'), - }, - })); + return nativeFetch(options.url, requestOptions).then(response => { + pendingBodySize -= requestSize; + pendingCount--; + return { + statusCode: response.status, + headers: { + 'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'), + 'retry-after': response.headers.get('Retry-After'), + }, + }; + }); } catch (e) { clearCachedFetchImplementation(); + pendingBodySize -= requestSize; + pendingCount--; return rejectedSyncPromise(e); } } diff --git a/packages/browser/test/unit/transports/fetch.test.ts b/packages/browser/test/unit/transports/fetch.test.ts index 53ed5c34e21b..e6b58eeaa110 100644 --- a/packages/browser/test/unit/transports/fetch.test.ts +++ b/packages/browser/test/unit/transports/fetch.test.ts @@ -16,6 +16,11 @@ const ERROR_ENVELOPE = createEnvelope({ event_id: 'aa3ff046696b4b [{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }] as EventItem, ]); +const LARGE_ERROR_ENVELOPE = createEnvelope( + { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, + [[{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', message: 'x'.repeat(10 * 900) }] as EventItem], +); + class Headers { headers: { [key: string]: string } = {}; get(key: string) { @@ -107,4 +112,54 @@ describe('NewFetchTransport', () => { await expect(() => transport.send(ERROR_ENVELOPE)).not.toThrow(); expect(mockFetch).toHaveBeenCalledTimes(1); }); + + it('correctly sets keepalive flag', async () => { + const mockFetch = jest.fn(() => + Promise.resolve({ + headers: new Headers(), + status: 200, + text: () => Promise.resolve({}), + }), + ) as unknown as FetchImpl; + + const REQUEST_OPTIONS: RequestInit = { + referrerPolicy: 'strict-origin', + referrer: 'http://example.org', + }; + + const transport = makeFetchTransport( + { ...DEFAULT_FETCH_TRANSPORT_OPTIONS, fetchOptions: REQUEST_OPTIONS }, + mockFetch, + ); + + const promises: PromiseLike[] = []; + for (let i = 0; i < 30; i++) { + promises.push(transport.send(LARGE_ERROR_ENVELOPE)); + } + + await Promise.all(promises); + + for (let i = 1; i <= 30; i++) { + // After 7 requests, we hit the total limit of >64kb of size + // Starting there, keepalive should be false + const keepalive = i < 7; + expect(mockFetch).toHaveBeenNthCalledWith(i, expect.any(String), expect.objectContaining({ keepalive })); + } + + (mockFetch as jest.Mock).mockClear(); + + // Limit resets when requests have resolved + // Now try based on # of pending requests + const promises2 = []; + for (let i = 0; i < 20; i++) { + promises2.push(transport.send(ERROR_ENVELOPE)); + } + + await Promise.all(promises2); + + for (let i = 1; i <= 20; i++) { + const keepalive = i < 15; + expect(mockFetch).toHaveBeenNthCalledWith(i, expect.any(String), expect.objectContaining({ keepalive })); + } + }); });