From 01a6d809ecb327a977a2d2b0e7ba86a67ca84a5a Mon Sep 17 00:00:00 2001 From: Greg Leonard <45019882+greg-el@users.noreply.github.com> Date: Tue, 10 Oct 2023 15:41:11 +0100 Subject: [PATCH] Provide option not to retry on event send failure (close #1248) --- .../browser-tracker-core/src/tracker/index.ts | 3 +- .../src/tracker/out_queue.ts | 23 ++- .../browser-tracker-core/src/tracker/types.ts | 12 ++ .../test/out_queue.test.ts | 144 +++++++++++++++++- 4 files changed, 168 insertions(+), 14 deletions(-) diff --git a/libraries/browser-tracker-core/src/tracker/index.ts b/libraries/browser-tracker-core/src/tracker/index.ts index a6443a932..af536aa2a 100755 --- a/libraries/browser-tracker-core/src/tracker/index.ts +++ b/libraries/browser-tracker-core/src/tracker/index.ts @@ -303,7 +303,8 @@ export function Tracker( trackerConfiguration.withCredentials ?? true, trackerConfiguration.retryStatusCodes ?? [], (trackerConfiguration.dontRetryStatusCodes ?? []).concat([400, 401, 403, 410, 422]), - trackerConfiguration.idService + trackerConfiguration.idService, + trackerConfiguration.retryFailures ), // Whether pageViewId should be regenerated after each trackPageView. Affect web_page context preservePageViewId = false, diff --git a/libraries/browser-tracker-core/src/tracker/out_queue.ts b/libraries/browser-tracker-core/src/tracker/out_queue.ts index 92e3ad968..23e59a5f8 100644 --- a/libraries/browser-tracker-core/src/tracker/out_queue.ts +++ b/libraries/browser-tracker-core/src/tracker/out_queue.ts @@ -64,6 +64,7 @@ export interface OutQueue { * @param retryStatusCodes – Failure HTTP response status codes from Collector for which sending events should be retried (they can override the `dontRetryStatusCodes`) * @param dontRetryStatusCodes – Failure HTTP response status codes from Collector for which sending events should not be retried * @param idService - Id service full URL. This URL will be added to the queue and will be called using a GET method. + * @param retryFailures - Whether to retry failed requests * @returns object OutQueueManager instance */ export function OutQueueManager( @@ -83,7 +84,8 @@ export function OutQueueManager( withCredentials: boolean, retryStatusCodes: number[], dontRetryStatusCodes: number[], - idService?: string + idService?: string, + retryFailures: boolean = true ): OutQueue { type PostEvent = { evt: Record; @@ -349,10 +351,7 @@ export function OutQueueManager( } // Time out POST requests after connectionTimeout - const xhrTimeout = setTimeout(function () { - xhr.abort(); - executingQueue = false; - }, connectionTimeout); + xhr.timeout = connectionTimeout; const removeEventsFromQueue = (numberToSend: number): void => { for (let deleteCount = 0; deleteCount < numberToSend; deleteCount++) { @@ -370,9 +369,21 @@ export function OutQueueManager( executeQueue(); }; + const failureHandler = (e: ProgressEvent) => { + // `e.type` will be "timeout", "abort", or "error" + if (!retryFailures) { + LOG.error(`Failed to send events due to: ${e.type}, retry failed is disabled.`); + removeEventsFromQueue(numberToSend); + executingQueue = false; + } + }; + + xhr.onerror = failureHandler; + xhr.ontimeout = failureHandler; + xhr.onabort = failureHandler; + xhr.onreadystatechange = function () { if (xhr.readyState === 4 && xhr.status >= 200) { - clearTimeout(xhrTimeout); if (xhr.status < 300) { onPostSuccess(numberToSend); } else { diff --git a/libraries/browser-tracker-core/src/tracker/types.ts b/libraries/browser-tracker-core/src/tracker/types.ts index 0f5801c9d..6bc42a3fe 100755 --- a/libraries/browser-tracker-core/src/tracker/types.ts +++ b/libraries/browser-tracker-core/src/tracker/types.ts @@ -249,6 +249,18 @@ export type TrackerConfiguration = { * The request respects the `anonymousTracking` option, including the SP-Anonymous header if needed, and any additional custom headers from the customHeaders option. */ idService?: string; + + /** + * Whether to retry failed requests to the collector. + * + * Failed requests are requests that failed due to + * [timeouts](https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/timeout_event), + * [network errors](https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/error_event), + * and [abort events](https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/abort_event). + * + * @defaultValue true + */ + retryFailures?: boolean; }; /** diff --git a/libraries/browser-tracker-core/test/out_queue.test.ts b/libraries/browser-tracker-core/test/out_queue.test.ts index bab53486b..b5e8668f9 100644 --- a/libraries/browser-tracker-core/test/out_queue.test.ts +++ b/libraries/browser-tracker-core/test/out_queue.test.ts @@ -31,23 +31,30 @@ import { OutQueueManager, OutQueue } from '../src/tracker/out_queue'; import { SharedState } from '../src/state'; +const readPostQueue = () => { + return JSON.parse( + window.localStorage.getItem('snowplowOutQueue_sp_post2') ?? fail('Unable to find local storage queue') + ); +}; + describe('OutQueueManager', () => { const maxQueueSize = 2; - var xhrMock: Partial; + var xhrMock: XMLHttpRequest; var xhrOpenMock: jest.Mock; beforeEach(() => { localStorage.clear(); xhrOpenMock = jest.fn(); xhrMock = { + ...new XMLHttpRequest(), open: xhrOpenMock, send: jest.fn(), setRequestHeader: jest.fn(), withCredentials: true, }; - jest.spyOn(window, 'XMLHttpRequest').mockImplementation(() => xhrMock as XMLHttpRequest); + jest.spyOn(window, 'XMLHttpRequest').mockImplementation(() => xhrMock); }); const respondMockRequest = (status: number) => { @@ -219,11 +226,6 @@ describe('OutQueueManager', () => { describe('idService requests', () => { const idServiceEndpoint = 'http://example.com/id'; - const readPostQueue = () => { - return JSON.parse( - window.localStorage.getItem('snowplowOutQueue_sp_post2') ?? fail('Unable to find local storage queue') - ); - }; const readGetQueue = () => JSON.parse(window.localStorage.getItem('snowplowOutQueue_sp_get') ?? fail('Unable to find local storage queue')); @@ -337,4 +339,132 @@ describe('OutQueueManager', () => { }); }); }); + + describe('retry failed events when retryFailures = true', () => { + const request = { e: 'pv', eid: '65cb78de-470c-4764-8c10-02bd79477a3a' }; + let createOutQueue = () => + OutQueueManager( + 'sp', + new SharedState(), + true, + 'post', + '/com.snowplowanalytics.snowplow/tp2', + 1, + 40000, + 0, + false, + maxQueueSize, + 10, + false, + {}, + true, + [], + [], + '', + true + ); + + it('should retry after abort', () => { + let outQueue = createOutQueue(); + outQueue.enqueueRequest(request, 'http://example.com'); + + let retrievedQueue = readPostQueue(); + expect(retrievedQueue).toHaveLength(1); + + xhrMock.onabort?.(new ProgressEvent('abort')); + + retrievedQueue = readPostQueue(); + expect(retrievedQueue).toHaveLength(1); + }); + + it('should retry after timeout', () => { + let outQueue = createOutQueue(); + outQueue.enqueueRequest(request, 'http://example.com'); + + let retrievedQueue = readPostQueue(); + expect(retrievedQueue).toHaveLength(1); + + xhrMock.ontimeout?.(new ProgressEvent('timeout')); + + retrievedQueue = readPostQueue(); + expect(retrievedQueue).toHaveLength(1); + }); + + it('should retry after error', () => { + let outQueue = createOutQueue(); + outQueue.enqueueRequest(request, 'http://example.com'); + + let retrievedQueue = readPostQueue(); + expect(retrievedQueue).toHaveLength(1); + + xhrMock.onerror?.(new ProgressEvent('error')); + + retrievedQueue = readPostQueue(); + expect(retrievedQueue).toHaveLength(1); + }); + }); + + describe('not retry failed events when retryFailures = false', () => { + const request = { e: 'pv', eid: '65cb78de-470c-4764-8c10-02bd79477a3a' }; + let createOutQueue = () => + OutQueueManager( + 'sp', + new SharedState(), + true, + 'post', + '/com.snowplowanalytics.snowplow/tp2', + 1, + 40000, + 0, + false, + maxQueueSize, + 10, + false, + {}, + true, + [], + [], + '', + false + ); + + it('should not retry after abort', () => { + let outQueue = createOutQueue(); + outQueue.enqueueRequest(request, 'http://example.com'); + + let retrievedQueue = readPostQueue(); + expect(retrievedQueue).toHaveLength(1); + + xhrMock.onabort?.(new ProgressEvent('abort')); + + retrievedQueue = readPostQueue(); + expect(retrievedQueue).toHaveLength(0); + }); + + it('should not retry after timeout', () => { + let outQueue = createOutQueue(); + outQueue.enqueueRequest(request, 'http://example.com'); + + let retrievedQueue = readPostQueue(); + expect(retrievedQueue).toHaveLength(1); + + xhrMock.ontimeout?.(new ProgressEvent('timeout')); + + retrievedQueue = readPostQueue(); + expect(retrievedQueue).toHaveLength(0); + }); + + it('should not retry after error', () => { + let outQueue = createOutQueue(); + outQueue.enqueueRequest(request, 'http://example.com'); + + let retrievedQueue = readPostQueue(); + expect(retrievedQueue).toHaveLength(1); + + xhrMock.onerror?.(new ProgressEvent('error')); + + retrievedQueue = readPostQueue(); + expect(retrievedQueue).toHaveLength(0); + }); + }); });