From e1fd479d3af84a25467c3abe9b24c17bdd5f9f9d Mon Sep 17 00:00:00 2001 From: "justin.fiedler" Date: Thu, 20 Jan 2022 23:27:00 +0000 Subject: [PATCH] fix: catch errors with Request.send --- src/amplitude-client.js | 80 ++++++++++++++++++++++------------------ test/amplitude-client.js | 16 ++++++++ 2 files changed, 61 insertions(+), 35 deletions(-) diff --git a/src/amplitude-client.js b/src/amplitude-client.js index 893888fb..fb72a5c4 100644 --- a/src/amplitude-client.js +++ b/src/amplitude-client.js @@ -1827,45 +1827,55 @@ AmplitudeClient.prototype.sendEvents = function sendEvents() { return; } var scope = this; - new Request(url, data, this.options.headers).send(function (status, response) { - scope._sending = false; - try { - if (status === 200 && response === 'success') { - scope.removeEvents(maxEventId, maxIdentifyId, status, response); - - // Update the event cache after the removal of sent events. - if (scope.options.saveEvents) { - scope.saveEvents(); - } - - // Send more events if any queued during previous send. - scope._sendEventsIfReady(); - - // handle payload too large - } 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); + try { + new Request(url, data, this.options.headers).send(function (status, response) { + scope._sending = false; + try { + if (status === 200 && response === 'success') { + scope.removeEvents(maxEventId, maxIdentifyId, status, response); + + // Update the event cache after the removal of sent events. + if (scope.options.saveEvents) { + scope.saveEvents(); } - // The server complained about the length of the request. Backoff and try again. - scope.options.uploadBatchSize = Math.ceil(numEvents / 2); - scope.sendEvents(); + // Send more events if any queued during previous send. + scope._sendEventsIfReady(); + + // handle payload too large + } 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(); + } } + // else { + // all the events are still queued, and will be retried when the next + // event is sent In the interest of debugging, it would be nice to have + // something like an event emitter for a better debugging experince + // here. + // } + } catch (e) { + // utils.log.error('failed upload'); } - // else { - // all the events are still queued, and will be retried when the next - // event is sent In the interest of debugging, it would be nice to have - // something like an event emitter for a better debugging experince - // here. - // } - } catch (e) { - // utils.log.error('failed upload'); - } - }); + }); + } catch (e) { + const [status, response] = [0, 'Request failed to send']; + + utils.log.error(response); + scope._logErrorsOnEvents(maxEventId, maxIdentifyId, status, response); + scope.removeEvents(maxEventId, maxIdentifyId, status, response, { + reason: e.message, + }); + } }; /** diff --git a/test/amplitude-client.js b/test/amplitude-client.js index 368d5e14..36b419cb 100644 --- a/test/amplitude-client.js +++ b/test/amplitude-client.js @@ -11,6 +11,7 @@ import Identify from '../src/identify.js'; import constants from '../src/constants.js'; import { mockCookie, restoreCookie, getCookie } from './mock-cookie'; import { AmplitudeServerZone } from '../src/server-zone.js'; +import Request from '../src/xhr'; // maintain for testing backwards compatability describe('AmplitudeClient', function () { @@ -4496,5 +4497,20 @@ describe('AmplitudeClient', function () { assert.equal(amplitude.getSessionId(), startTime); assert.equal(amplitude.options.userId, 'test user'); }); + + it('should handle failed requests to send events', function () { + const errorLogSpy = sinon.spy(utils.log, 'error'); + const requestStub = sinon.stub(Request.prototype, 'send').throws(); + + const amplitude = new AmplitudeClient(); + amplitude.init(apiKey); + amplitude.logEvent('testEvent1'); + + assert.isTrue(errorLogSpy.calledWith('Request failed to send')); + assert.equal(amplitude._unsentEvents.length, 0); + + errorLogSpy.restore(); + requestStub.restore(); + }); }); });