diff --git a/packages/core/src/domain/configuration/endpointBuilder.ts b/packages/core/src/domain/configuration/endpointBuilder.ts index b40566231a..274fcbc6e0 100644 --- a/packages/core/src/domain/configuration/endpointBuilder.ts +++ b/packages/core/src/domain/configuration/endpointBuilder.ts @@ -40,6 +40,7 @@ export function createEndpointBuilder( const tags = [`sdk_version:${__BUILD_ENV__SDK_VERSION__}`, `api:${api}`].concat(configurationTags) if (retry) { tags.push(`retry_count:${retry.count}`, `retry_after:${retry.lastFailureStatus}`) + retry.lastFailureType && tags.push(`retry_after_type:${retry.lastFailureType}`) } let parameters = 'ddsource=browser' + diff --git a/packages/core/src/transport/httpRequest.spec.ts b/packages/core/src/transport/httpRequest.spec.ts index 0038bf43d1..2d8acb29f8 100644 --- a/packages/core/src/transport/httpRequest.spec.ts +++ b/packages/core/src/transport/httpRequest.spec.ts @@ -105,14 +105,14 @@ describe('httpRequest', () => { pending('no fetch keepalive support') } - interceptor.withFetch(() => Promise.resolve({ status: 429 })) + interceptor.withFetch(() => Promise.resolve({ status: 429, type: 'cors' })) fetchKeepAliveStrategy( endpointBuilder, BATCH_BYTES_LIMIT, { data: '{"foo":"bar1"}\n{"foo":"bar2"}', bytesCount: 10 }, (response) => { - expect(response).toEqual({ status: 429 }) + expect(response).toEqual({ status: 429, type: 'cors' }) done() } ) diff --git a/packages/core/src/transport/httpRequest.ts b/packages/core/src/transport/httpRequest.ts index da92449854..7e28f72bb3 100644 --- a/packages/core/src/transport/httpRequest.ts +++ b/packages/core/src/transport/httpRequest.ts @@ -18,6 +18,7 @@ export type HttpRequest = ReturnType export interface HttpResponse extends Context { status: number + type?: ResponseType } export interface Payload { @@ -29,6 +30,7 @@ export interface Payload { export interface RetryInfo { count: number lastFailureStatus: number + lastFailureType?: ResponseType } export function createHttpRequest( @@ -91,8 +93,8 @@ export function fetchKeepAliveStrategy( const canUseKeepAlive = isKeepAliveSupported() && bytesCount < bytesLimit if (canUseKeepAlive) { const fetchUrl = endpointBuilder.build('fetch', retry) - fetch(fetchUrl, { method: 'POST', body: data, keepalive: true }).then( - monitor((response: Response) => onResponse?.({ status: response.status })), + fetch(fetchUrl, { method: 'POST', body: data, keepalive: true, mode: 'cors' }).then( + monitor((response: Response) => onResponse?.({ status: response.status, type: response.type })), monitor(() => { const xhrUrl = endpointBuilder.build('xhr', retry) // failed to queue the request diff --git a/packages/core/src/transport/sendWithRetryStrategy.spec.ts b/packages/core/src/transport/sendWithRetryStrategy.spec.ts index 595127856e..dff8378ba1 100644 --- a/packages/core/src/transport/sendWithRetryStrategy.spec.ts +++ b/packages/core/src/transport/sendWithRetryStrategy.spec.ts @@ -190,50 +190,68 @@ describe('sendWithRetryStrategy', () => { }) }) ;[ - { description: 'when the intake returns error:', status: 500 }, - { description: 'when the intake returns too many request:', status: 429 }, - { description: 'when the intake returns request timeout:', status: 408 }, - { description: 'when network is down:', status: 0 }, - ].forEach(({ description, status }) => { + { expectRetry: true, description: 'when the intake returns error:', status: 500 }, + { expectRetry: true, description: 'when the intake returns too many request:', status: 429 }, + { expectRetry: true, description: 'when the intake returns request timeout:', status: 408 }, + { expectRetry: true, description: 'when network error:', status: 0, type: undefined }, + { expectRetry: true, description: 'when network error with response type:', status: 0, type: 'cors' as const }, + { expectRetry: false, description: 'when the intake returns opaque response:', status: 0, type: 'opaque' as const }, + ].forEach(({ expectRetry, description, status, type }) => { describe(description, () => { - it('should start queueing following requests', () => { - sendRequest() - sendStub.respondWith(0, { status }) - expect(state.queuedPayloads.size()).toBe(1) + if (expectRetry) { + it('should start queueing following requests', () => { + sendRequest() + sendStub.respondWith(0, { status, type }) + expect(state.queuedPayloads.size()).toBe(1) + + sendRequest() + expect(state.queuedPayloads.size()).toBe(2) + sendRequest() + expect(state.queuedPayloads.size()).toBe(3) + }) - sendRequest() - expect(state.queuedPayloads.size()).toBe(2) - sendRequest() - expect(state.queuedPayloads.size()).toBe(3) - }) + it('should send queued requests if another ongoing request succeed', () => { + sendRequest() + sendRequest() + sendStub.respondWith(0, { status, type }) + expect(state.bandwidthMonitor.ongoingRequestCount).toBe(1) + expect(state.queuedPayloads.size()).toBe(1) - it('should send queued requests if another ongoing request succeed', () => { - sendRequest() - sendRequest() - sendStub.respondWith(0, { status }) - expect(state.bandwidthMonitor.ongoingRequestCount).toBe(1) - expect(state.queuedPayloads.size()).toBe(1) + sendRequest() + expect(state.bandwidthMonitor.ongoingRequestCount).toBe(1) + expect(state.queuedPayloads.size()).toBe(2) - sendRequest() - expect(state.bandwidthMonitor.ongoingRequestCount).toBe(1) - expect(state.queuedPayloads.size()).toBe(2) - - sendStub.respondWith(1, { status: 200 }) - expect(state.bandwidthMonitor.ongoingRequestCount).toBe(2) - expect(state.queuedPayloads.size()).toBe(0) - }) + sendStub.respondWith(1, { status: 200 }) + expect(state.bandwidthMonitor.ongoingRequestCount).toBe(2) + expect(state.queuedPayloads.size()).toBe(0) + }) - it('should add retry info to payloads', () => { - sendRequest() + it('should add retry info to payloads', () => { + sendRequest() - sendStub.respondWith(0, { status }) - expect(state.queuedPayloads.first().retry).toEqual({ count: 1, lastFailureStatus: status }) + sendStub.respondWith(0, { status, type }) + expect(state.queuedPayloads.first().retry).toEqual({ + count: 1, + lastFailureStatus: status, + lastFailureType: type, + }) - clock.tick(INITIAL_BACKOFF_TIME) + clock.tick(INITIAL_BACKOFF_TIME) - sendStub.respondWith(1, { status }) - expect(state.queuedPayloads.first().retry).toEqual({ count: 2, lastFailureStatus: status }) - }) + sendStub.respondWith(1, { status, type }) + expect(state.queuedPayloads.first().retry).toEqual({ + count: 2, + lastFailureStatus: status, + lastFailureType: type, + }) + }) + } else { + it('should not queue the payload for retry', () => { + sendRequest() + sendStub.respondWith(0, { status, type }) + expect(state.queuedPayloads.size()).toBe(0) + }) + } }) }) diff --git a/packages/core/src/transport/sendWithRetryStrategy.ts b/packages/core/src/transport/sendWithRetryStrategy.ts index bfef2a13a0..5a63e1f994 100644 --- a/packages/core/src/transport/sendWithRetryStrategy.ts +++ b/packages/core/src/transport/sendWithRetryStrategy.ts @@ -104,6 +104,7 @@ function send( payload.retry = { count: payload.retry ? payload.retry.count + 1 : 1, lastFailureStatus: response.status, + lastFailureType: response.type, } onFailure() } @@ -133,7 +134,10 @@ function retryQueuedPayloads( } function shouldRetryRequest(response: HttpResponse) { - return response.status === 0 || response.status === 408 || response.status === 429 || response.status >= 500 + return ( + response?.type !== 'opaque' && + (response.status === 0 || response.status === 408 || response.status === 429 || response.status >= 500) + ) } export function newRetryState(): RetryState {