From b767fc6abbd853265358f1fe0fe59b7c3b3290ba Mon Sep 17 00:00:00 2001 From: Donald Pipowitch Date: Mon, 7 Dec 2020 17:03:33 +0100 Subject: [PATCH 1/7] fixes #142 --- src/amplitude-client.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/amplitude-client.js b/src/amplitude-client.js index 14a91a8f..2cf9f9a7 100644 --- a/src/amplitude-client.js +++ b/src/amplitude-client.js @@ -1358,6 +1358,9 @@ AmplitudeClient.prototype._logEvent = function _logEvent( return eventId; } catch (e) { utils.log.error(e); + if (type(callback) === 'function') { + callback(0, 'No request sent', {reason: 'Request failed (e.g. it was blocked).'}); + } } }; From afa790745953cac291fcf7fc30c7586ed516290d Mon Sep 17 00:00:00 2001 From: Donald Pipowitch Date: Thu, 10 Dec 2020 15:54:55 +0100 Subject: [PATCH 2/7] fix error handling --- src/amplitude-client.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/amplitude-client.js b/src/amplitude-client.js index 2cf9f9a7..b5bd249e 100644 --- a/src/amplitude-client.js +++ b/src/amplitude-client.js @@ -1358,9 +1358,6 @@ AmplitudeClient.prototype._logEvent = function _logEvent( return eventId; } catch (e) { utils.log.error(e); - if (type(callback) === 'function') { - callback(0, 'No request sent', {reason: 'Request failed (e.g. it was blocked).'}); - } } }; @@ -1687,6 +1684,7 @@ AmplitudeClient.prototype.sendEvents = function sendEvents() { // here. // } } catch (e) { + scope.removeEvents(Infinity, Infinity, 0, 'No request sent', {reason: 'Request failed (e.g. it was blocked).'}); // utils.log('failed upload'); } }); From 30f52d87563ca8dbc5fdb68c86b249f0d8a58480 Mon Sep 17 00:00:00 2001 From: John Tran Date: Wed, 6 Jan 2021 22:51:25 -0800 Subject: [PATCH 3/7] revert commit --- src/amplitude-client.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/amplitude-client.js b/src/amplitude-client.js index b5bd249e..3210336f 100644 --- a/src/amplitude-client.js +++ b/src/amplitude-client.js @@ -1684,8 +1684,7 @@ AmplitudeClient.prototype.sendEvents = function sendEvents() { // here. // } } catch (e) { - scope.removeEvents(Infinity, Infinity, 0, 'No request sent', {reason: 'Request failed (e.g. it was blocked).'}); - // utils.log('failed upload'); + // utils.log.error('failed upload'); } }); }; From 8d67d5b9b171335d44854ae5da6422c8bdb5eeed Mon Sep 17 00:00:00 2001 From: John Tran Date: Wed, 6 Jan 2021 22:51:48 -0800 Subject: [PATCH 4/7] extend callback interface --- src/amplitude-client.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/amplitude-client.js b/src/amplitude-client.js index 3210336f..ef50e9c3 100644 --- a/src/amplitude-client.js +++ b/src/amplitude-client.js @@ -1394,6 +1394,7 @@ AmplitudeClient.prototype._limitEventsQueued = function _limitEventsQueued(queue * @callback Amplitude~eventCallback * @param {number} responseCode - Server response code for the event / identify upload request. * @param {string} responseBody - Server response body for the event / identify upload request. + * @param {object} details - (optional) Additional information associated with sending event. */ /** From 9fc5e5bd8bd949e240e002618108cb9bca74bcfc Mon Sep 17 00:00:00 2001 From: John Tran Date: Wed, 6 Jan 2021 22:52:06 -0800 Subject: [PATCH 5/7] fix callback call for deleted events --- src/amplitude-client.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/amplitude-client.js b/src/amplitude-client.js index ef50e9c3..e786855a 100644 --- a/src/amplitude-client.js +++ b/src/amplitude-client.js @@ -1384,7 +1384,14 @@ var _generateApiPropertiesTrackingConfig = function _generateApiPropertiesTracki */ AmplitudeClient.prototype._limitEventsQueued = function _limitEventsQueued(queue) { if (queue.length > this.options.savedMaxCount) { - queue.splice(0, queue.length - this.options.savedMaxCount); + const deletedEvents = queue.splice(0, queue.length - this.options.savedMaxCount); + deletedEvents.forEach((e) => { + if (type(e.callback) === 'function') { + e.callback(0, 'No request sent', { + reason: 'Event dropped because options.savedMaxCount exceeded. User may be offline or have a content blocker', + }); + } + }); } }; From 523543957b470e5b7672e5cb9797d23b45edaab5 Mon Sep 17 00:00:00 2001 From: John Tran Date: Wed, 6 Jan 2021 22:52:27 -0800 Subject: [PATCH 6/7] Added tests --- test/amplitude-client.js | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/amplitude-client.js b/test/amplitude-client.js index 9788feb5..fdc8df1d 100644 --- a/test/amplitude-client.js +++ b/test/amplitude-client.js @@ -1945,6 +1945,33 @@ describe('AmplitudeClient', function () { assert.equal(message, 'success'); }); + it.only('should run the callback even with a dropped unsent event', function () { + amplitude.init(apiKey, null, { savedMaxCount: 1 }); + var counter = 0; + var value = null; + var message = null; + var reason = null; + var callback = function (status, response, details) { + counter++; + value = status; + message = response; + reason = details.reason; + }; + amplitude.logEvent('DroppedEvent', {}, callback); + amplitude.logEvent('SavedEvent', {}, callback); + server.respondWith([0, {}, '']); + server.respond(); + + // verify callback fired + assert.equal(counter, 1); + assert.equal(value, 0); + assert.equal(message, 'No request sent'); + assert.equal( + reason, + 'Event dropped because options.savedMaxCount exceeded. User may be offline or have a content blocker', + ); + }); + it('should run callback once and only after 413 resolved', function () { var counter = 0; var value = -1; From 86d5e6ea94e3ac93a90d28326731432227f0762b Mon Sep 17 00:00:00 2001 From: John Tran Date: Thu, 7 Jan 2021 17:05:14 -0800 Subject: [PATCH 7/7] single letter variable nit --- src/amplitude-client.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/amplitude-client.js b/src/amplitude-client.js index e786855a..e5b49b24 100644 --- a/src/amplitude-client.js +++ b/src/amplitude-client.js @@ -1385,9 +1385,9 @@ var _generateApiPropertiesTrackingConfig = function _generateApiPropertiesTracki AmplitudeClient.prototype._limitEventsQueued = function _limitEventsQueued(queue) { if (queue.length > this.options.savedMaxCount) { const deletedEvents = queue.splice(0, queue.length - this.options.savedMaxCount); - deletedEvents.forEach((e) => { - if (type(e.callback) === 'function') { - e.callback(0, 'No request sent', { + deletedEvents.forEach((event) => { + if (type(event.callback) === 'function') { + event.callback(0, 'No request sent', { reason: 'Event dropped because options.savedMaxCount exceeded. User may be offline or have a content blocker', }); }