From f796571c2d4e7bb8964f3b30f895344e0a5be579 Mon Sep 17 00:00:00 2001 From: Greg Leonard <45019882+greg-el@users.noreply.github.com> Date: Thu, 19 Oct 2023 14:22:41 +0100 Subject: [PATCH] Provide option not to retry on event send failure (close #1248) --- ...failure-retry-option_2023-10-20-15-33.json | 10 ++ ...failure-retry-option_2023-10-23-14-24.json | 10 ++ .../browser-tracker-core/src/tracker/index.ts | 3 +- .../src/tracker/out_queue.ts | 33 +++++-- .../browser-tracker-core/src/tracker/types.ts | 11 +++ .../test/out_queue.test.ts | 94 ++++++++++++++++++- .../docs/browser-tracker.api.md | 1 + 7 files changed, 148 insertions(+), 14 deletions(-) create mode 100644 common/changes/@snowplow/browser-tracker-core/issue-1248-send-failure-retry-option_2023-10-20-15-33.json create mode 100644 common/changes/@snowplow/browser-tracker/issue-1248-send-failure-retry-option_2023-10-23-14-24.json diff --git a/common/changes/@snowplow/browser-tracker-core/issue-1248-send-failure-retry-option_2023-10-20-15-33.json b/common/changes/@snowplow/browser-tracker-core/issue-1248-send-failure-retry-option_2023-10-20-15-33.json new file mode 100644 index 000000000..bcd68cb5c --- /dev/null +++ b/common/changes/@snowplow/browser-tracker-core/issue-1248-send-failure-retry-option_2023-10-20-15-33.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@snowplow/browser-tracker-core", + "comment": "Add retryFailures option", + "type": "none" + } + ], + "packageName": "@snowplow/browser-tracker-core" +} \ No newline at end of file diff --git a/common/changes/@snowplow/browser-tracker/issue-1248-send-failure-retry-option_2023-10-23-14-24.json b/common/changes/@snowplow/browser-tracker/issue-1248-send-failure-retry-option_2023-10-23-14-24.json new file mode 100644 index 000000000..4a400f6fc --- /dev/null +++ b/common/changes/@snowplow/browser-tracker/issue-1248-send-failure-retry-option_2023-10-23-14-24.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@snowplow/browser-tracker", + "comment": "Update API docs with retryFailedRequests", + "type": "none" + } + ], + "packageName": "@snowplow/browser-tracker" +} \ No newline at end of file diff --git a/libraries/browser-tracker-core/src/tracker/index.ts b/libraries/browser-tracker-core/src/tracker/index.ts index a6443a932..2183981c0 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.retryFailedRequests ), // 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 eb5d8894d..feca74837 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 retryFailedRequests - 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, + retryFailedRequests: boolean = true ): OutQueue { type PostEvent = { evt: Record; @@ -348,10 +350,18 @@ export function OutQueueManager( numberToSend = 1; } + const checkRetryFailedRequests = () => { + if (!retryFailedRequests) { + removeEventsFromQueue(numberToSend); + } + executingQueue = false; + }; + // Time out POST requests after connectionTimeout const xhrTimeout = setTimeout(function () { xhr.abort(); - executingQueue = false; + + checkRetryFailedRequests(); }, connectionTimeout); const removeEventsFromQueue = (numberToSend: number): void => { @@ -375,13 +385,19 @@ export function OutQueueManager( clearTimeout(xhrTimeout); if (xhr.status >= 200 && xhr.status < 300) { onPostSuccess(numberToSend); - } else { - if (!shouldRetryForStatusCode(xhr.status)) { - LOG.error(`Status ${xhr.status}, will not retry.`); - removeEventsFromQueue(numberToSend); - } - executingQueue = false; + return; + } + + if (xhr.status === 0) { + checkRetryFailedRequests(); + return; } + + if (!shouldRetryForStatusCode(xhr.status)) { + LOG.error(`Status ${xhr.status}, will not retry.`); + removeEventsFromQueue(numberToSend); + } + executingQueue = false; } }; @@ -490,6 +506,7 @@ export function OutQueueManager( xhr.setRequestHeader(header, customHeaders[header]); } } + return xhr; } diff --git a/libraries/browser-tracker-core/src/tracker/types.ts b/libraries/browser-tracker-core/src/tracker/types.ts index 0f5801c9d..20a7ce390 100755 --- a/libraries/browser-tracker-core/src/tracker/types.ts +++ b/libraries/browser-tracker-core/src/tracker/types.ts @@ -249,6 +249,17 @@ 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 + */ + retryFailedRequests?: 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..98f8b6be2 100644 --- a/libraries/browser-tracker-core/test/out_queue.test.ts +++ b/libraries/browser-tracker-core/test/out_queue.test.ts @@ -31,6 +31,12 @@ 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; @@ -45,6 +51,7 @@ describe('OutQueueManager', () => { send: jest.fn(), setRequestHeader: jest.fn(), withCredentials: true, + abort: jest.fn(), }; jest.spyOn(window, 'XMLHttpRequest').mockImplementation(() => xhrMock as XMLHttpRequest); @@ -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,86 @@ describe('OutQueueManager', () => { }); }); }); + + describe('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 remain in queue on failure', (done) => { + let outQueue = createOutQueue(); + outQueue.enqueueRequest(request, 'http://example.com'); + + let retrievedQueue = readPostQueue(); + expect(retrievedQueue).toHaveLength(1); + + respondMockRequest(0); + + setTimeout(() => { + retrievedQueue = readPostQueue(); + expect(retrievedQueue).toHaveLength(1); + done(); + }, 20); + }); + }); + + describe('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, + 0, + false, + {}, + true, + [], + [], + '', + false + ); + + it('should remove from queue on failure', (done) => { + let outQueue = createOutQueue(); + outQueue.enqueueRequest(request, 'http://example.com'); + + let retrievedQueue = readPostQueue(); + expect(retrievedQueue).toHaveLength(1); + + respondMockRequest(0); + + setTimeout(() => { + retrievedQueue = readPostQueue(); + expect(retrievedQueue).toHaveLength(0); + done(); + }, 20); + }); + }); }); diff --git a/trackers/browser-tracker/docs/browser-tracker.api.md b/trackers/browser-tracker/docs/browser-tracker.api.md index 70aabae95..4328b687e 100644 --- a/trackers/browser-tracker/docs/browser-tracker.api.md +++ b/trackers/browser-tracker/docs/browser-tracker.api.md @@ -369,6 +369,7 @@ export type TrackerConfiguration = { dontRetryStatusCodes?: number[]; onSessionUpdateCallback?: (updatedSession: ClientSession) => void; idService?: string; + retryFailedRequests?: boolean; }; // @public