From 551bdfcbfd8967770e633fcba7552b82ee44b379 Mon Sep 17 00:00:00 2001
From: AJ Horst <AJ_Horst@intuit.com>
Date: Thu, 29 Jul 2021 22:15:04 -0700
Subject: [PATCH 1/6] Add error callback to log methods

---
 src/amplitude-client.js | 68 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 61 insertions(+), 7 deletions(-)

diff --git a/src/amplitude-client.js b/src/amplitude-client.js
index aa67d0f0..1e6f56d5 100644
--- a/src/amplitude-client.js
+++ b/src/amplitude-client.js
@@ -1164,6 +1164,7 @@ AmplitudeClient.prototype._logEvent = function _logEvent(
   groupProperties,
   timestamp,
   callback,
+  errorCallback,
 ) {
   _loadCookieData(this); // reload cookie before each log event to sync event meta-data between windows and tabs
 
@@ -1171,12 +1172,18 @@ AmplitudeClient.prototype._logEvent = function _logEvent(
     if (type(callback) === 'function') {
       callback(0, 'No request sent', { reason: 'Missing eventType' });
     }
+    if (type(errorCallback) === 'function') {
+      errorCallback(0, 'No request sent', { reason: 'Missing eventType' });
+    }
     return;
   }
   if (this.options.optOut) {
     if (type(callback) === 'function') {
       callback(0, 'No request sent', { reason: 'optOut is set to true' });
     }
+    if (type(errorCallback) === 'function') {
+      errorCallback(0, 'No request sent', { reason: 'optOut is set to true' });
+    }
     return;
   }
 
@@ -1233,10 +1240,10 @@ AmplitudeClient.prototype._logEvent = function _logEvent(
     };
 
     if (eventType === Constants.IDENTIFY_EVENT || eventType === Constants.GROUP_IDENTIFY_EVENT) {
-      this._unsentIdentifys.push({ event, callback });
+      this._unsentIdentifys.push({ event, callback, errorCallback });
       this._limitEventsQueued(this._unsentIdentifys);
     } else {
-      this._unsentEvents.push({ event, callback });
+      this._unsentEvents.push({ event, callback, errorCallback });
       this._limitEventsQueued(this._unsentEvents);
     }
 
@@ -1282,6 +1289,11 @@ AmplitudeClient.prototype._limitEventsQueued = function _limitEventsQueued(queue
           reason: 'Event dropped because options.savedMaxCount exceeded. User may be offline or have a content blocker',
         });
       }
+      if (type(event.errorCallback) === 'function') {
+        event.errorCallback(0, 'No request sent', {
+          reason: 'Event dropped because options.savedMaxCount exceeded. User may be offline or have a content blocker',
+        });
+      }
     });
   }
 };
@@ -1302,13 +1314,16 @@ AmplitudeClient.prototype._limitEventsQueued = function _limitEventsQueued(queue
  * @param {object} eventProperties - (optional) an object with string keys and values for the event properties.
  * @param {Amplitude~eventCallback} opt_callback - (optional) a callback function to run after the event is logged.
  * Note: the server response code and response body from the event upload are passed to the callback function.
+ * @param {Amplitude~eventCallback} opt_error_callback - (optional) a callback function to run after the event logging
+ * fails. The failure can be from the request being malformed or from a network failure
+ * Note: the server response code and response body from the event upload are passed to the callback function.
  * @example amplitudeClient.logEvent('Clicked Homepage Button', {'finished_flow': false, 'clicks': 15});
  */
-AmplitudeClient.prototype.logEvent = function logEvent(eventType, eventProperties, opt_callback) {
+AmplitudeClient.prototype.logEvent = function logEvent(eventType, eventProperties, opt_callback, opt_error_callback) {
   if (this._shouldDeferCall()) {
     return this._q.push(['logEvent'].concat(Array.prototype.slice.call(arguments, 0)));
   }
-  return this.logEventWithTimestamp(eventType, eventProperties, null, opt_callback);
+  return this.logEventWithTimestamp(eventType, eventProperties, null, opt_callback, opt_error_callback);
 };
 
 /**
@@ -1319,6 +1334,9 @@ AmplitudeClient.prototype.logEvent = function logEvent(eventType, eventPropertie
  * @param {number} timestamp - (optional) the custom timestamp as milliseconds since epoch.
  * @param {Amplitude~eventCallback} opt_callback - (optional) a callback function to run after the event is logged.
  * Note: the server response code and response body from the event upload are passed to the callback function.
+ * @param {Amplitude~eventCallback} opt_error_callback - (optional) a callback function to run after the event logging
+ * fails. The failure can be from the request being malformed or from a network failure
+ * Note: the server response code and response body from the event upload are passed to the callback function.
  * @example amplitudeClient.logEvent('Clicked Homepage Button', {'finished_flow': false, 'clicks': 15});
  */
 AmplitudeClient.prototype.logEventWithTimestamp = function logEvent(
@@ -1326,6 +1344,7 @@ AmplitudeClient.prototype.logEventWithTimestamp = function logEvent(
   eventProperties,
   timestamp,
   opt_callback,
+  opt_error_callback,
 ) {
   if (this._shouldDeferCall()) {
     return this._q.push(['logEventWithTimestamp'].concat(Array.prototype.slice.call(arguments, 0)));
@@ -1334,21 +1353,41 @@ AmplitudeClient.prototype.logEventWithTimestamp = function logEvent(
     if (type(opt_callback) === 'function') {
       opt_callback(0, 'No request sent', { reason: 'API key not set' });
     }
+    if (type(opt_error_callback) === 'function') {
+      opt_error_callback(0, 'No request sent', { reason: 'API key not set' });
+    }
+
     return -1;
   }
   if (!utils.validateInput(eventType, 'eventType', 'string')) {
     if (type(opt_callback) === 'function') {
       opt_callback(0, 'No request sent', { reason: 'Invalid type for eventType' });
     }
+    if (type(opt_error_callback) === 'function') {
+      opt_error_callback(0, 'No request sent', { reason: 'Invalid type for eventType' });
+    }
     return -1;
   }
   if (utils.isEmptyString(eventType)) {
     if (type(opt_callback) === 'function') {
       opt_callback(0, 'No request sent', { reason: 'Missing eventType' });
     }
+    if (type(opt_error_callback) === 'function') {
+      opt_error_callback(0, 'No request sent', { reason: 'Missing eventType' });
+    }
     return -1;
   }
-  return this._logEvent(eventType, eventProperties, null, null, null, null, timestamp, opt_callback);
+  return this._logEvent(
+    eventType,
+    eventProperties,
+    null,
+    null,
+    null,
+    null,
+    timestamp,
+    opt_callback,
+    opt_error_callback,
+  );
 };
 
 /**
@@ -1365,9 +1404,18 @@ AmplitudeClient.prototype.logEventWithTimestamp = function logEvent(
  * groupName can be a string or an array of strings.
  * @param {Amplitude~eventCallback} opt_callback - (optional) a callback function to run after the event is logged.
  * Note: the server response code and response body from the event upload are passed to the callback function.
+ * @param {Amplitude~eventCallback} opt_error_callback - (optional) a callback function to run after the event logging
+ * fails. The failure can be from the request being malformed or from a network failure
+ * Note: the server response code and response body from the event upload are passed to the callback function.
  * @example amplitudeClient.logEventWithGroups('Clicked Button', null, {'orgId': 24});
  */
-AmplitudeClient.prototype.logEventWithGroups = function (eventType, eventProperties, groups, opt_callback) {
+AmplitudeClient.prototype.logEventWithGroups = function (
+  eventType,
+  eventProperties,
+  groups,
+  opt_callback,
+  opt_error_callback,
+) {
   if (this._shouldDeferCall()) {
     return this._q.push(['logEventWithGroups'].concat(Array.prototype.slice.call(arguments, 0)));
   }
@@ -1375,15 +1423,21 @@ AmplitudeClient.prototype.logEventWithGroups = function (eventType, eventPropert
     if (type(opt_callback) === 'function') {
       opt_callback(0, 'No request sent', { reason: 'API key not set' });
     }
+    if (type(opt_error_callback) === 'function') {
+      opt_error_callback(0, 'No request sent', { reason: 'API key not set' });
+    }
     return -1;
   }
   if (!utils.validateInput(eventType, 'eventType', 'string')) {
     if (type(opt_callback) === 'function') {
       opt_callback(0, 'No request sent', { reason: 'Invalid type for eventType' });
     }
+    if (type(opt_error_callback) === 'function') {
+      opt_error_callback(0, 'No request sent', { reason: 'Invalid type for eventType' });
+    }
     return -1;
   }
-  return this._logEvent(eventType, eventProperties, null, null, groups, null, null, opt_callback);
+  return this._logEvent(eventType, eventProperties, null, null, groups, null, null, opt_callback, opt_error_callback);
 };
 
 /**

From ee300142d67d06b2ab5910f1afcd7a7dca020680 Mon Sep 17 00:00:00 2001
From: AJ Horst <AJ_Horst@intuit.com>
Date: Thu, 29 Jul 2021 22:21:55 -0700
Subject: [PATCH 2/6] add logic to call error callbacks on event log failures

---
 src/amplitude-client.js | 42 ++++++++++++++++++++++++++++++++---------
 1 file changed, 33 insertions(+), 9 deletions(-)

diff --git a/src/amplitude-client.js b/src/amplitude-client.js
index 1e6f56d5..d043f9f8 100644
--- a/src/amplitude-client.js
+++ b/src/amplitude-client.js
@@ -1522,6 +1522,27 @@ if (BUILD_COMPAT_2_0) {
   };
 }
 
+/**
+ * Calls error callback on unsent events
+ * @private
+ */
+AmplitudeClient.prototype.logErrorsOnEvents = function logErrorsOnEvents(maxEventId, maxIdentifyId, status, response) {
+  const queues = ['_unsentEvents', '_unsentIdentifys'];
+
+  for (let queue in queues) {
+    const maxId = queue === '_unsentEvents' ? maxEventId : maxIdentifyId;
+    for (var i = 0; i < this[queue].length || 0; i++) {
+      const unsentEvent = this[queue][i];
+
+      if (unsentEvent.event.event_id <= maxId) {
+        if (unsentEvent.errorCallback) {
+          unsentEvent.errorCallback(status, response);
+        }
+      }
+    }
+  }
+};
+
 /**
  * Remove events in storage with event ids up to and including maxEventId.
  * @private
@@ -1619,16 +1640,19 @@ AmplitudeClient.prototype.sendEvents = function sendEvents() {
         scope._sendEventsIfReady();
 
         // handle payload too large
-      } else if (status === 413) {
-        // utils.log('request too large');
-        // Can't even get this one massive event through. Drop it, even if it is an identify.
-        if (scope.options.uploadBatchSize === 1) {
-          scope.removeEvents(maxEventId, maxIdentifyId, status, response);
+      } else {
+        scope.logErrorsOnEvents(maxEventId, maxIdentifyId, status, response);
+        if (status === 413) {
+          // utils.log('request too large');
+          // Can't even get this one massive event through. Drop it, even if it is an identify.
+          if (scope.options.uploadBatchSize === 1) {
+            scope.removeEvents(maxEventId, maxIdentifyId, status, response);
+          }
+
+          // The server complained about the length of the request. Backoff and try again.
+          scope.options.uploadBatchSize = Math.ceil(numEvents / 2);
+          scope.sendEvents();
         }
-
-        // The server complained about the length of the request. Backoff and try again.
-        scope.options.uploadBatchSize = Math.ceil(numEvents / 2);
-        scope.sendEvents();
       }
       // else {
       //  all the events are still queued, and will be retried when the next

From f9fb5828079ec5525bc010969d139043a106c45f Mon Sep 17 00:00:00 2001
From: AJ Horst <AJ_Horst@intuit.com>
Date: Thu, 29 Jul 2021 22:43:19 -0700
Subject: [PATCH 3/6] add error callback to log identify methods

---
 src/amplitude-client.js | 42 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 39 insertions(+), 3 deletions(-)

diff --git a/src/amplitude-client.js b/src/amplitude-client.js
index d043f9f8..1f86d4d5 100644
--- a/src/amplitude-client.js
+++ b/src/amplitude-client.js
@@ -1032,11 +1032,14 @@ var _convertProxyObjectToRealObject = function _convertProxyObjectToRealObject(i
  * @param {Identify} identify_obj - the Identify object containing the user property operations to send.
  * @param {Amplitude~eventCallback} opt_callback - (optional) callback function to run when the identify event has been sent.
  * Note: the server response code and response body from the identify event upload are passed to the callback function.
+ * @param {Amplitude~eventCallback} opt_error_callback - (optional) a callback function to run after the event logging
+ * fails. The failure can be from the request being malformed or from a network failure
+ * Note: the server response code and response body from the event upload are passed to the callback function.
  * @example
  * var identify = new amplitude.Identify().set('colors', ['rose', 'gold']).add('karma', 1).setOnce('sign_up_date', '2016-03-31');
  * amplitude.identify(identify);
  */
-AmplitudeClient.prototype.identify = function (identify_obj, opt_callback) {
+AmplitudeClient.prototype.identify = function (identify_obj, opt_callback, opt_error_callback) {
   if (this._shouldDeferCall()) {
     return this._q.push(['identify'].concat(Array.prototype.slice.call(arguments, 0)));
   }
@@ -1044,6 +1047,9 @@ AmplitudeClient.prototype.identify = function (identify_obj, opt_callback) {
     if (type(opt_callback) === 'function') {
       opt_callback(0, 'No request sent', { reason: 'API key is not set' });
     }
+    if (type(opt_error_callback) === 'function') {
+      opt_error_callback(0, 'No request sent', { reason: 'API key is not set' });
+    }
     return;
   }
 
@@ -1064,21 +1070,34 @@ AmplitudeClient.prototype.identify = function (identify_obj, opt_callback) {
         null,
         null,
         opt_callback,
+        opt_error_callback,
       );
     } else {
       if (type(opt_callback) === 'function') {
         opt_callback(0, 'No request sent', { reason: 'No user property operations' });
       }
+      if (type(opt_error_callback) === 'function') {
+        opt_error_callback(0, 'No request sent', { reason: 'No user property operations' });
+      }
     }
   } else {
     utils.log.error('Invalid identify input type. Expected Identify object but saw ' + type(identify_obj));
     if (type(opt_callback) === 'function') {
       opt_callback(0, 'No request sent', { reason: 'Invalid identify input type' });
     }
+    if (type(opt_error_callback) === 'function') {
+      opt_error_callback(0, 'No request sent', { reason: 'No user property operations' });
+    }
   }
 };
 
-AmplitudeClient.prototype.groupIdentify = function (group_type, group_name, identify_obj, opt_callback) {
+AmplitudeClient.prototype.groupIdentify = function (
+  group_type,
+  group_name,
+  identify_obj,
+  opt_callback,
+  opt_error_callback,
+) {
   if (this._shouldDeferCall()) {
     return this._q.push(['groupIdentify'].concat(Array.prototype.slice.call(arguments, 0)));
   }
@@ -1086,6 +1105,9 @@ AmplitudeClient.prototype.groupIdentify = function (group_type, group_name, iden
     if (type(opt_callback) === 'function') {
       opt_callback(0, 'No request sent', { reason: 'API key is not set' });
     }
+    if (type(opt_error_callback) === 'function') {
+      opt_error_callback(0, 'No request sent', { reason: 'API key is not set' });
+    }
     return;
   }
 
@@ -1093,6 +1115,9 @@ AmplitudeClient.prototype.groupIdentify = function (group_type, group_name, iden
     if (type(opt_callback) === 'function') {
       opt_callback(0, 'No request sent', { reason: 'Invalid group type' });
     }
+    if (type(opt_error_callback) === 'function') {
+      opt_error_callback(0, 'No request sent', { reason: 'Invalid group type' });
+    }
     return;
   }
 
@@ -1100,6 +1125,9 @@ AmplitudeClient.prototype.groupIdentify = function (group_type, group_name, iden
     if (type(opt_callback) === 'function') {
       opt_callback(0, 'No request sent', { reason: 'Invalid group name' });
     }
+    if (type(opt_error_callback) === 'function') {
+      opt_error_callback(0, 'No request sent', { reason: 'Invalid group name' });
+    }
     return;
   }
 
@@ -1120,17 +1148,24 @@ AmplitudeClient.prototype.groupIdentify = function (group_type, group_name, iden
         identify_obj.userPropertiesOperations,
         null,
         opt_callback,
+        opt_error_callback,
       );
     } else {
       if (type(opt_callback) === 'function') {
         opt_callback(0, 'No request sent', { reason: 'No group property operations' });
       }
+      if (type(opt_error_callback) === 'function') {
+        opt_error_callback(0, 'No request sent', { reason: 'No group property operations' });
+      }
     }
   } else {
     utils.log.error('Invalid identify input type. Expected Identify object but saw ' + type(identify_obj));
     if (type(opt_callback) === 'function') {
       opt_callback(0, 'No request sent', { reason: 'Invalid identify input type' });
     }
+    if (type(opt_error_callback) === 'function') {
+      opt_error_callback(0, 'No request sent', { reason: 'No group property operations' });
+    }
   }
 };
 
@@ -1529,7 +1564,8 @@ if (BUILD_COMPAT_2_0) {
 AmplitudeClient.prototype.logErrorsOnEvents = function logErrorsOnEvents(maxEventId, maxIdentifyId, status, response) {
   const queues = ['_unsentEvents', '_unsentIdentifys'];
 
-  for (let queue in queues) {
+  for (var j = 0; j < queues.length; j++) {
+    const queue = queues[j];
     const maxId = queue === '_unsentEvents' ? maxEventId : maxIdentifyId;
     for (var i = 0; i < this[queue].length || 0; i++) {
       const unsentEvent = this[queue][i];

From 18ae7de5ae4158dd81e29ccdcce7ec259b5aaca4 Mon Sep 17 00:00:00 2001
From: AJ Horst <AJ_Horst@intuit.com>
Date: Thu, 29 Jul 2021 23:05:33 -0700
Subject: [PATCH 4/6] add tests

---
 test/amplitude-client.js | 199 +++++++++++++++++++++++++++++++++++----
 1 file changed, 179 insertions(+), 20 deletions(-)

diff --git a/test/amplitude-client.js b/test/amplitude-client.js
index 5c8fee58..aaacabd3 100644
--- a/test/amplitude-client.js
+++ b/test/amplitude-client.js
@@ -1216,6 +1216,7 @@ describe('AmplitudeClient', function () {
 
     it('should run the callback after making the identify call', function () {
       var counter = 0;
+      var errCounter = 0;
       var value = -1;
       var message = '';
       var callback = function (status, response) {
@@ -1223,12 +1224,18 @@ describe('AmplitudeClient', function () {
         value = status;
         message = response;
       };
+
+      var errCallback = function () {
+        errCounter++;
+      };
+
       var identify = new amplitude.Identify().set('key', 'value');
-      amplitude.identify(identify, callback);
+      amplitude.identify(identify, callback, errCallback);
 
       // before server responds, callback should not fire
       assert.lengthOf(server.requests, 1);
       assert.equal(counter, 0);
+      assert.equal(errCounter, 0);
       assert.equal(value, -1);
       assert.equal(message, '');
 
@@ -1238,41 +1245,76 @@ describe('AmplitudeClient', function () {
       assert.equal(counter, 1);
       assert.equal(value, 200);
       assert.equal(message, 'success');
+
+      // error callback should not fire
+      assert.equal(errCounter, 0);
     });
 
     it('should run the callback even if client not initialized with apiKey', function () {
       var counter = 0;
       var value = -1;
       var message = '';
+
+      var errCounter = 0;
+      var errValue = -1;
+      var errMessage = '';
       var callback = function (status, response) {
         counter++;
         value = status;
         message = response;
       };
+
+      var errCallback = function (status, response) {
+        errCounter++;
+        errValue = status;
+        errMessage = response;
+      };
+
       var identify = new amplitude.Identify().set('key', 'value');
-      new AmplitudeClient().identify(identify, callback);
+      new AmplitudeClient().identify(identify, callback, errCallback);
 
       // verify callback fired
       assert.equal(counter, 1);
       assert.equal(value, 0);
       assert.equal(message, 'No request sent');
+
+      // verify error callback fired
+      assert.equal(errCounter, 1);
+      assert.equal(errValue, 0);
+      assert.equal(errMessage, 'No request sent');
     });
 
     it('should run the callback even with an invalid identify object', function () {
       var counter = 0;
       var value = -1;
       var message = '';
+
+      var errCounter = 0;
+      var errValue = -1;
+      var errMessage = '';
       var callback = function (status, response) {
         counter++;
         value = status;
         message = response;
       };
-      amplitude.identify(null, callback);
+
+      var errCallback = function (status, response) {
+        errCounter++;
+        errValue = status;
+        errMessage = response;
+      };
+
+      amplitude.identify(null, callback, errCallback);
 
       // verify callback fired
       assert.equal(counter, 1);
       assert.equal(value, 0);
       assert.equal(message, 'No request sent');
+
+      // verify error callback fired
+      assert.equal(errCounter, 1);
+      assert.equal(errValue, 0);
+      assert.equal(errMessage, 'No request sent');
     });
   });
 
@@ -1390,6 +1432,7 @@ describe('AmplitudeClient', function () {
 
     it('should run the callback after making the identify call', function () {
       var counter = 0;
+      var errCounter = 0;
       var value = -1;
       var message = '';
       var callback = function (status, response) {
@@ -1397,12 +1440,16 @@ describe('AmplitudeClient', function () {
         value = status;
         message = response;
       };
+      var errCallback = function () {
+        errCounter++;
+      };
       var identify = new amplitude.Identify().set('key', 'value');
-      amplitude.groupIdentify(group_type, group_name, identify, callback);
+      amplitude.groupIdentify(group_type, group_name, identify, callback, errCallback);
 
       // before server responds, callback should not fire
       assert.lengthOf(server.requests, 1);
       assert.equal(counter, 0);
+      assert.equal(errCounter, 0);
       assert.equal(value, -1);
       assert.equal(message, '');
 
@@ -1412,24 +1459,42 @@ describe('AmplitudeClient', function () {
       assert.equal(counter, 1);
       assert.equal(value, 200);
       assert.equal(message, 'success');
+
+      // error callback should not be fired
+      assert.equal(errCounter, 0);
     });
 
     it('should run the callback even if client not initialized with apiKey', function () {
       var counter = 0;
       var value = -1;
       var message = '';
+
+      var errCounter = 0;
+      var errValue = -1;
+      var errMessage = '';
       var callback = function (status, response) {
         counter++;
         value = status;
         message = response;
       };
+
+      var errCallback = function (status, response) {
+        errCounter++;
+        errValue = status;
+        errMessage = response;
+      };
       var identify = new amplitude.Identify().set('key', 'value');
-      new AmplitudeClient().groupIdentify(group_type, group_name, identify, callback);
+      new AmplitudeClient().groupIdentify(group_type, group_name, identify, callback, errCallback);
 
       // verify callback fired
       assert.equal(counter, 1);
       assert.equal(value, 0);
       assert.equal(message, 'No request sent');
+
+      // verify error callback fired
+      assert.equal(errCounter, 1);
+      assert.equal(errValue, 0);
+      assert.equal(errMessage, 'No request sent');
     });
 
     it('should run the callback even with an invalid identify object', function () {
@@ -1894,15 +1959,28 @@ describe('AmplitudeClient', function () {
       var counter = 0;
       var value = -1;
       var message = '';
+
+      var errCounter = 0;
+      var errValue = -1;
+      var errMessage = '';
       var callback = function (status, response) {
         counter++;
         value = status;
         message = response;
       };
-      amplitude.logEvent(null, null, callback);
+
+      var errCallback = function (status, response) {
+        errCounter++;
+        errValue = status;
+        errMessage = response;
+      };
+      amplitude.logEvent(null, null, callback, errCallback);
       assert.equal(counter, 1);
       assert.equal(value, 0);
       assert.equal(message, 'No request sent');
+      assert.equal(errCounter, 1);
+      assert.equal(errValue, 0);
+      assert.equal(errMessage, 'No request sent');
     });
 
     it('should run callback if optout', function () {
@@ -1910,15 +1988,28 @@ describe('AmplitudeClient', function () {
       var counter = 0;
       var value = -1;
       var message = '';
+
+      var errCounter = 0;
+      var errValue = -1;
+      var errMessage = '';
       var callback = function (status, response) {
         counter++;
         value = status;
         message = response;
       };
-      amplitude.logEvent('test', null, callback);
+
+      var errCallback = function (status, response) {
+        errCounter++;
+        errValue = status;
+        errMessage = response;
+      };
+      amplitude.logEvent('test', null, callback, errCallback);
       assert.equal(counter, 1);
       assert.equal(value, 0);
       assert.equal(message, 'No request sent');
+      assert.equal(errCounter, 1);
+      assert.equal(errValue, 0);
+      assert.equal(errMessage, 'No request sent');
     });
 
     it('should not run callback if invalid callback and no eventType', function () {
@@ -1927,18 +2018,25 @@ describe('AmplitudeClient', function () {
 
     it('should run callback after logging event', function () {
       var counter = 0;
+      var errCounter = 0;
       var value = -1;
       var message = '';
+
+      var errCallback = function () {
+        errCounter++;
+      };
+
       var callback = function (status, response) {
         counter++;
         value = status;
         message = response;
       };
-      amplitude.logEvent('test', null, callback);
+      amplitude.logEvent('test', null, callback, errCallback);
 
       // before server responds, callback should not fire
       assert.lengthOf(server.requests, 1);
       assert.equal(counter, 0);
+      assert.equal(errCounter, 0);
       assert.equal(value, -1);
       assert.equal(message, '');
 
@@ -1948,6 +2046,9 @@ describe('AmplitudeClient', function () {
       assert.equal(counter, 1);
       assert.equal(value, 200);
       assert.equal(message, 'success');
+
+      // erro callback should not fire
+      assert.equal(errCounter, 0);
     });
 
     it('should run callback if batchEvents but under threshold', function () {
@@ -1958,6 +2059,7 @@ describe('AmplitudeClient', function () {
         eventUploadPeriodMillis: eventUploadPeriodMillis,
       });
       var counter = 0;
+      var errCounter = 0;
       var value = -1;
       var message = '';
       var callback = function (status, response) {
@@ -1965,7 +2067,12 @@ describe('AmplitudeClient', function () {
         value = status;
         message = response;
       };
-      amplitude.logEvent('test', null, callback);
+
+      var errCallback = function () {
+        errCounter++;
+      };
+
+      amplitude.logEvent('test', null, callback, errCallback);
       assert.lengthOf(server.requests, 0);
 
       // check that request is made after delay, but callback is not run a second time
@@ -1974,6 +2081,7 @@ describe('AmplitudeClient', function () {
       server.respondWith('success');
       server.respond();
       assert.equal(counter, 1);
+      assert.equal(errCounter, 0);
       assert.equal(value, 200);
       assert.equal(message, 'success');
     });
@@ -1981,6 +2089,7 @@ describe('AmplitudeClient', function () {
     it('should run callback once and only after all events are uploaded', function () {
       amplitude.init(apiKey, null, { uploadBatchSize: 10 });
       var counter = 0;
+      var errCounter = 0;
       var value = -1;
       var message = '';
       var callback = function (status, response) {
@@ -1989,6 +2098,10 @@ describe('AmplitudeClient', function () {
         message = response;
       };
 
+      var errCallback = function () {
+        errCounter++;
+      };
+
       // queue up 15 events, since batchsize 10, need to send in 2 batches
       amplitude._sending = true;
       for (var i = 0; i < 15; i++) {
@@ -1996,7 +2109,7 @@ describe('AmplitudeClient', function () {
       }
       amplitude._sending = false;
 
-      amplitude.logEvent('Event', { index: 100 }, callback);
+      amplitude.logEvent('Event', { index: 100 }, callback, errCallback);
 
       assert.lengthOf(server.requests, 1);
       server.respondWith('success');
@@ -2004,6 +2117,7 @@ describe('AmplitudeClient', function () {
 
       // after first response received, callback should not have fired
       assert.equal(counter, 0);
+      assert.equal(errCounter, 0);
       assert.equal(value, -1);
       assert.equal(message, '');
 
@@ -2015,22 +2129,37 @@ describe('AmplitudeClient', function () {
       assert.equal(counter, 1);
       assert.equal(value, 200);
       assert.equal(message, 'success');
+
+      // error callback should not have fired
+      assert.equal(errCounter, 0);
     });
 
     it('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 value = -1;
+      var message = '';
+      var reason = '';
+
+      var errCounter = 0;
+      var errValue = -1;
+      var errMessage = '';
+      var errReason = '';
       var callback = function (status, response, details) {
         counter++;
         value = status;
         message = response;
         reason = details.reason;
       };
-      amplitude.logEvent('DroppedEvent', {}, callback);
-      amplitude.logEvent('SavedEvent', {}, callback);
+
+      var errCallback = function (status, response, details) {
+        errCounter++;
+        errValue = status;
+        errMessage = response;
+        errReason = details.reason;
+      };
+      amplitude.logEvent('DroppedEvent', {}, callback, errCallback);
+      amplitude.logEvent('SavedEvent', {}, callback, errCallback);
       server.respondWith([0, {}, '']);
       server.respond();
 
@@ -2042,10 +2171,20 @@ describe('AmplitudeClient', function () {
         reason,
         'Event dropped because options.savedMaxCount exceeded. User may be offline or have a content blocker',
       );
+
+      // verify error callback fired
+      assert.equal(errCounter, 1);
+      assert.equal(errValue, 0);
+      assert.equal(errMessage, 'No request sent');
+      assert.equal(
+        errReason,
+        '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 errCounter = 0;
       var value = -1;
       var message = '';
       var callback = function (status, response) {
@@ -2054,6 +2193,10 @@ describe('AmplitudeClient', function () {
         message = response;
       };
 
+      var errCallback = function () {
+        errCounter++;
+      };
+
       // queue up 15 events
       amplitude._sending = true;
       for (var i = 0; i < 15; i++) {
@@ -2062,7 +2205,7 @@ describe('AmplitudeClient', function () {
       amplitude._sending = false;
 
       // 16th event with 413 will backoff to batches of 8
-      amplitude.logEvent('Event', { index: 100 }, callback);
+      amplitude.logEvent('Event', { index: 100 }, callback, errCallback);
 
       assert.lengthOf(server.requests, 1);
       var events = JSON.parse(queryString.parse(server.requests[0].requestBody).e);
@@ -2072,6 +2215,7 @@ describe('AmplitudeClient', function () {
       server.respondWith([413, {}, '']);
       server.respond();
       assert.equal(counter, 0);
+      assert.equal(errCounter, 1);
       assert.equal(value, -1);
       assert.equal(message, '');
 
@@ -2082,6 +2226,7 @@ describe('AmplitudeClient', function () {
       server.respondWith('success');
       server.respond();
       assert.equal(counter, 0);
+      assert.equal(errCounter, 1);
       assert.equal(value, -1);
       assert.equal(message, '');
 
@@ -2092,25 +2237,30 @@ describe('AmplitudeClient', function () {
       server.respondWith('success');
       server.respond();
       assert.equal(counter, 1);
+      assert.equal(errCounter, 1);
       assert.equal(value, 200);
       assert.equal(message, 'success');
     });
 
     it('should _not_ run callback when the server returns a 500', function () {
       const callback = sinon.spy();
+      const errCallback = sinon.spy();
 
-      amplitude.logEvent('test', null, callback);
+      amplitude.logEvent('test', null, callback, errCallback);
       server.respondWith([500, {}, 'Not found']);
       server.respond();
       assert.isFalse(callback.calledOnce);
+      assert.isTrue(errCallback.calledOnce);
     });
 
     it('should run the callback when the server finally returns a 200 after a 500', function () {
       const callback = sinon.spy();
+      const errCallback = sinon.spy();
 
-      amplitude.logEvent('test', null, callback);
+      amplitude.logEvent('test', null, callback, errCallback);
       server.respondWith([500, {}, 'Not found']);
       server.respond();
+      assert.isTrue(errCallback.calledOnce);
       // The SDK retries failed events when a new event is sent
       amplitude.logEvent('test2');
       server.respondWith([200, {}, 'success']);
@@ -2121,10 +2271,12 @@ describe('AmplitudeClient', function () {
 
     it('should run the callback when the server finally returns a 413 after a 500', function () {
       const callback = sinon.spy();
+      const errCallback = sinon.spy();
 
-      amplitude.logEvent('test', null, callback);
+      amplitude.logEvent('test', null, callback, errCallback);
       server.respondWith([500, {}, 'Not found']);
       server.respond();
+      assert.isTrue(errCallback.calledOnce);
       // The SDK retries failed events when a new event is sent
       amplitude.logEvent('test2');
       server.respondWith([413, {}, '']);
@@ -2562,6 +2714,7 @@ describe('AmplitudeClient', function () {
 
     it('should handle groups input', function () {
       var counter = 0;
+      var errCounter = 0;
       var value = -1;
       var message = '';
       var callback = function (status, response) {
@@ -2571,6 +2724,10 @@ describe('AmplitudeClient', function () {
         message = response;
       };
 
+      var errCallback = function () {
+        errCounter++;
+      };
+
       var eventProperties = {
         key: 'value',
       };
@@ -2582,7 +2739,7 @@ describe('AmplitudeClient', function () {
         null: null, // ignore null values
       };
 
-      amplitude.logEventWithGroups('Test', eventProperties, groups, callback);
+      amplitude.logEventWithGroups('Test', eventProperties, groups, callback, errCallback);
       assert.lengthOf(server.requests, 1);
       var events = JSON.parse(queryString.parse(server.requests[0].requestBody).e);
       assert.lengthOf(events, 1);
@@ -2600,11 +2757,13 @@ describe('AmplitudeClient', function () {
 
       // verify callback behavior
       assert.equal(counter, 0);
+      assert.equal(errCounter, 0);
       assert.equal(value, -1);
       assert.equal(message, '');
       server.respondWith('success');
       server.respond();
       assert.equal(counter, 1);
+      assert.equal(errCounter, 0);
       assert.equal(value, 200);
       assert.equal(message, 'success');
     });

From 88d5dd499d1721a66d18fb1751a3551f7520ce8f Mon Sep 17 00:00:00 2001
From: AJ Horst <AJ_Horst@intuit.com>
Date: Thu, 29 Jul 2021 23:21:22 -0700
Subject: [PATCH 5/6] refactor dual callback calls

---
 src/amplitude-client.js | 168 ++++++++++++++++------------------------
 1 file changed, 68 insertions(+), 100 deletions(-)

diff --git a/src/amplitude-client.js b/src/amplitude-client.js
index 1f86d4d5..f78446ee 100644
--- a/src/amplitude-client.js
+++ b/src/amplitude-client.js
@@ -1044,12 +1044,8 @@ AmplitudeClient.prototype.identify = function (identify_obj, opt_callback, opt_e
     return this._q.push(['identify'].concat(Array.prototype.slice.call(arguments, 0)));
   }
   if (!this._apiKeySet('identify()')) {
-    if (type(opt_callback) === 'function') {
-      opt_callback(0, 'No request sent', { reason: 'API key is not set' });
-    }
-    if (type(opt_error_callback) === 'function') {
-      opt_error_callback(0, 'No request sent', { reason: 'API key is not set' });
-    }
+    _logErrorsWithCallbacks(opt_callback, opt_error_callback, 0, 'No request sent', { reason: 'API key is not set' });
+
     return;
   }
 
@@ -1073,21 +1069,15 @@ AmplitudeClient.prototype.identify = function (identify_obj, opt_callback, opt_e
         opt_error_callback,
       );
     } else {
-      if (type(opt_callback) === 'function') {
-        opt_callback(0, 'No request sent', { reason: 'No user property operations' });
-      }
-      if (type(opt_error_callback) === 'function') {
-        opt_error_callback(0, 'No request sent', { reason: 'No user property operations' });
-      }
+      _logErrorsWithCallbacks(opt_callback, opt_error_callback, 0, 'No request sent', {
+        reason: 'No user property operations',
+      });
     }
   } else {
     utils.log.error('Invalid identify input type. Expected Identify object but saw ' + type(identify_obj));
-    if (type(opt_callback) === 'function') {
-      opt_callback(0, 'No request sent', { reason: 'Invalid identify input type' });
-    }
-    if (type(opt_error_callback) === 'function') {
-      opt_error_callback(0, 'No request sent', { reason: 'No user property operations' });
-    }
+    _logErrorsWithCallbacks(opt_callback, opt_error_callback, 0, 'No request sent', {
+      reason: 'Invalid identify input type',
+    });
   }
 };
 
@@ -1102,32 +1092,23 @@ AmplitudeClient.prototype.groupIdentify = function (
     return this._q.push(['groupIdentify'].concat(Array.prototype.slice.call(arguments, 0)));
   }
   if (!this._apiKeySet('groupIdentify()')) {
-    if (type(opt_callback) === 'function') {
-      opt_callback(0, 'No request sent', { reason: 'API key is not set' });
-    }
-    if (type(opt_error_callback) === 'function') {
-      opt_error_callback(0, 'No request sent', { reason: 'API key is not set' });
-    }
+    _logErrorsWithCallbacks(opt_callback, opt_error_callback, 0, 'No request sent', {
+      reason: 'API key is not set',
+    });
     return;
   }
 
   if (!utils.validateInput(group_type, 'group_type', 'string') || utils.isEmptyString(group_type)) {
-    if (type(opt_callback) === 'function') {
-      opt_callback(0, 'No request sent', { reason: 'Invalid group type' });
-    }
-    if (type(opt_error_callback) === 'function') {
-      opt_error_callback(0, 'No request sent', { reason: 'Invalid group type' });
-    }
+    _logErrorsWithCallbacks(opt_callback, opt_error_callback, 0, 'No request sent', {
+      reason: 'Invalid group type',
+    });
     return;
   }
 
   if (group_name === null || group_name === undefined) {
-    if (type(opt_callback) === 'function') {
-      opt_callback(0, 'No request sent', { reason: 'Invalid group name' });
-    }
-    if (type(opt_error_callback) === 'function') {
-      opt_error_callback(0, 'No request sent', { reason: 'Invalid group name' });
-    }
+    _logErrorsWithCallbacks(opt_callback, opt_error_callback, 0, 'No request sent', {
+      reason: 'Invalid group name',
+    });
     return;
   }
 
@@ -1151,21 +1132,15 @@ AmplitudeClient.prototype.groupIdentify = function (
         opt_error_callback,
       );
     } else {
-      if (type(opt_callback) === 'function') {
-        opt_callback(0, 'No request sent', { reason: 'No group property operations' });
-      }
-      if (type(opt_error_callback) === 'function') {
-        opt_error_callback(0, 'No request sent', { reason: 'No group property operations' });
-      }
+      _logErrorsWithCallbacks(opt_callback, opt_error_callback, 0, 'No request sent', {
+        reason: 'No group property operations',
+      });
     }
   } else {
     utils.log.error('Invalid identify input type. Expected Identify object but saw ' + type(identify_obj));
-    if (type(opt_callback) === 'function') {
-      opt_callback(0, 'No request sent', { reason: 'Invalid identify input type' });
-    }
-    if (type(opt_error_callback) === 'function') {
-      opt_error_callback(0, 'No request sent', { reason: 'No group property operations' });
-    }
+    _logErrorsWithCallbacks(opt_callback, opt_error_callback, 0, 'No request sent', {
+      reason: 'Invalid identify input type',
+    });
   }
 };
 
@@ -1204,21 +1179,15 @@ AmplitudeClient.prototype._logEvent = function _logEvent(
   _loadCookieData(this); // reload cookie before each log event to sync event meta-data between windows and tabs
 
   if (!eventType) {
-    if (type(callback) === 'function') {
-      callback(0, 'No request sent', { reason: 'Missing eventType' });
-    }
-    if (type(errorCallback) === 'function') {
-      errorCallback(0, 'No request sent', { reason: 'Missing eventType' });
-    }
+    _logErrorsWithCallbacks(callback, errorCallback, 0, 'No request sent', {
+      reason: 'Missing eventType',
+    });
     return;
   }
   if (this.options.optOut) {
-    if (type(callback) === 'function') {
-      callback(0, 'No request sent', { reason: 'optOut is set to true' });
-    }
-    if (type(errorCallback) === 'function') {
-      errorCallback(0, 'No request sent', { reason: 'optOut is set to true' });
-    }
+    _logErrorsWithCallbacks(callback, errorCallback, 0, 'No request sent', {
+      reason: 'optOut is set to true',
+    });
     return;
   }
 
@@ -1319,16 +1288,9 @@ AmplitudeClient.prototype._limitEventsQueued = function _limitEventsQueued(queue
   if (queue.length > this.options.savedMaxCount) {
     const deletedEvents = queue.splice(0, queue.length - this.options.savedMaxCount);
     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',
-        });
-      }
-      if (type(event.errorCallback) === 'function') {
-        event.errorCallback(0, 'No request sent', {
-          reason: 'Event dropped because options.savedMaxCount exceeded. User may be offline or have a content blocker',
-        });
-      }
+      _logErrorsWithCallbacks(event.callback, event.errorCallback, 0, 'No request sent', {
+        reason: 'Event dropped because options.savedMaxCount exceeded. User may be offline or have a content blocker',
+      });
     });
   }
 };
@@ -1385,31 +1347,23 @@ AmplitudeClient.prototype.logEventWithTimestamp = function logEvent(
     return this._q.push(['logEventWithTimestamp'].concat(Array.prototype.slice.call(arguments, 0)));
   }
   if (!this._apiKeySet('logEvent()')) {
-    if (type(opt_callback) === 'function') {
-      opt_callback(0, 'No request sent', { reason: 'API key not set' });
-    }
-    if (type(opt_error_callback) === 'function') {
-      opt_error_callback(0, 'No request sent', { reason: 'API key not set' });
-    }
+    _logErrorsWithCallbacks(opt_callback, opt_error_callback, 0, 'No request sent', {
+      reason: 'API key not set',
+    });
 
     return -1;
   }
   if (!utils.validateInput(eventType, 'eventType', 'string')) {
-    if (type(opt_callback) === 'function') {
-      opt_callback(0, 'No request sent', { reason: 'Invalid type for eventType' });
-    }
-    if (type(opt_error_callback) === 'function') {
-      opt_error_callback(0, 'No request sent', { reason: 'Invalid type for eventType' });
-    }
+    _logErrorsWithCallbacks(opt_callback, opt_error_callback, 0, 'No request sent', {
+      reason: 'Invalid type for eventType',
+    });
+
     return -1;
   }
   if (utils.isEmptyString(eventType)) {
-    if (type(opt_callback) === 'function') {
-      opt_callback(0, 'No request sent', { reason: 'Missing eventType' });
-    }
-    if (type(opt_error_callback) === 'function') {
-      opt_error_callback(0, 'No request sent', { reason: 'Missing eventType' });
-    }
+    _logErrorsWithCallbacks(opt_callback, opt_error_callback, 0, 'No request sent', {
+      reason: 'Missing eventType',
+    });
     return -1;
   }
   return this._logEvent(
@@ -1455,21 +1409,16 @@ AmplitudeClient.prototype.logEventWithGroups = function (
     return this._q.push(['logEventWithGroups'].concat(Array.prototype.slice.call(arguments, 0)));
   }
   if (!this._apiKeySet('logEventWithGroups()')) {
-    if (type(opt_callback) === 'function') {
-      opt_callback(0, 'No request sent', { reason: 'API key not set' });
-    }
-    if (type(opt_error_callback) === 'function') {
-      opt_error_callback(0, 'No request sent', { reason: 'API key not set' });
-    }
+    _logErrorsWithCallbacks(event.callback, event.errorCallback, 0, 'No request sent', {
+      reason: 'API key not set',
+    });
+
     return -1;
   }
   if (!utils.validateInput(eventType, 'eventType', 'string')) {
-    if (type(opt_callback) === 'function') {
-      opt_callback(0, 'No request sent', { reason: 'Invalid type for eventType' });
-    }
-    if (type(opt_error_callback) === 'function') {
-      opt_error_callback(0, 'No request sent', { reason: 'Invalid type for eventType' });
-    }
+    _logErrorsWithCallbacks(event.callback, event.errorCallback, 0, 'No request sent', {
+      reason: 'Invalid type for eventType',
+    });
     return -1;
   }
   return this._logEvent(eventType, eventProperties, null, null, groups, null, null, opt_callback, opt_error_callback);
@@ -1483,6 +1432,25 @@ var _isNumber = function _isNumber(n) {
   return !isNaN(parseFloat(n)) && isFinite(n);
 };
 
+/**
+ * Handles errors that are sent to both callbacks
+ * @private
+ */
+var _logErrorsWithCallbacks = function _logErrorsWithCallbacks(
+  opt_callback,
+  opt_error_callback,
+  status,
+  response,
+  details,
+) {
+  if (type(opt_callback) === 'function') {
+    opt_callback(status, response, details);
+  }
+  if (type(opt_error_callback) === 'function') {
+    opt_error_callback(status, response, details);
+  }
+};
+
 /**
  * Log revenue with Revenue interface. The new revenue interface allows for more revenue fields like
  * revenueType and event properties.

From 483a2c6a8704bdff814b07680906d68bff7b7fd8 Mon Sep 17 00:00:00 2001
From: AJ Horst <AJ_Horst@intuit.com>
Date: Fri, 30 Jul 2021 12:06:29 -0700
Subject: [PATCH 6/6] prefix private method with underscore

---
 src/amplitude-client.js | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/amplitude-client.js b/src/amplitude-client.js
index f78446ee..d6e34f16 100644
--- a/src/amplitude-client.js
+++ b/src/amplitude-client.js
@@ -1529,7 +1529,12 @@ if (BUILD_COMPAT_2_0) {
  * Calls error callback on unsent events
  * @private
  */
-AmplitudeClient.prototype.logErrorsOnEvents = function logErrorsOnEvents(maxEventId, maxIdentifyId, status, response) {
+AmplitudeClient.prototype._logErrorsOnEvents = function _logErrorsOnEvents(
+  maxEventId,
+  maxIdentifyId,
+  status,
+  response,
+) {
   const queues = ['_unsentEvents', '_unsentIdentifys'];
 
   for (var j = 0; j < queues.length; j++) {
@@ -1645,7 +1650,7 @@ AmplitudeClient.prototype.sendEvents = function sendEvents() {
 
         // handle payload too large
       } else {
-        scope.logErrorsOnEvents(maxEventId, maxIdentifyId, status, response);
+        scope._logErrorsOnEvents(maxEventId, maxIdentifyId, status, response);
         if (status === 413) {
           // utils.log('request too large');
           // Can't even get this one massive event through. Drop it, even if it is an identify.