Skip to content

Commit 333f8a4

Browse files
fix: catch errors with Request.send (#490)
1 parent 0119ac7 commit 333f8a4

File tree

2 files changed

+61
-35
lines changed

2 files changed

+61
-35
lines changed

src/amplitude-client.js

+45-35
Original file line numberDiff line numberDiff line change
@@ -1827,45 +1827,55 @@ AmplitudeClient.prototype.sendEvents = function sendEvents() {
18271827
return;
18281828
}
18291829
var scope = this;
1830-
new Request(url, data, this.options.headers).send(function (status, response) {
1831-
scope._sending = false;
1832-
try {
1833-
if (status === 200 && response === 'success') {
1834-
scope.removeEvents(maxEventId, maxIdentifyId, status, response);
1835-
1836-
// Update the event cache after the removal of sent events.
1837-
if (scope.options.saveEvents) {
1838-
scope.saveEvents();
1839-
}
1840-
1841-
// Send more events if any queued during previous send.
1842-
scope._sendEventsIfReady();
1843-
1844-
// handle payload too large
1845-
} else {
1846-
scope._logErrorsOnEvents(maxEventId, maxIdentifyId, status, response);
1847-
if (status === 413) {
1848-
// utils.log('request too large');
1849-
// Can't even get this one massive event through. Drop it, even if it is an identify.
1850-
if (scope.options.uploadBatchSize === 1) {
1851-
scope.removeEvents(maxEventId, maxIdentifyId, status, response);
1830+
try {
1831+
new Request(url, data, this.options.headers).send(function (status, response) {
1832+
scope._sending = false;
1833+
try {
1834+
if (status === 200 && response === 'success') {
1835+
scope.removeEvents(maxEventId, maxIdentifyId, status, response);
1836+
1837+
// Update the event cache after the removal of sent events.
1838+
if (scope.options.saveEvents) {
1839+
scope.saveEvents();
18521840
}
18531841

1854-
// The server complained about the length of the request. Backoff and try again.
1855-
scope.options.uploadBatchSize = Math.ceil(numEvents / 2);
1856-
scope.sendEvents();
1842+
// Send more events if any queued during previous send.
1843+
scope._sendEventsIfReady();
1844+
1845+
// handle payload too large
1846+
} else {
1847+
scope._logErrorsOnEvents(maxEventId, maxIdentifyId, status, response);
1848+
if (status === 413) {
1849+
// utils.log('request too large');
1850+
// Can't even get this one massive event through. Drop it, even if it is an identify.
1851+
if (scope.options.uploadBatchSize === 1) {
1852+
scope.removeEvents(maxEventId, maxIdentifyId, status, response);
1853+
}
1854+
1855+
// The server complained about the length of the request. Backoff and try again.
1856+
scope.options.uploadBatchSize = Math.ceil(numEvents / 2);
1857+
scope.sendEvents();
1858+
}
18571859
}
1860+
// else {
1861+
// all the events are still queued, and will be retried when the next
1862+
// event is sent In the interest of debugging, it would be nice to have
1863+
// something like an event emitter for a better debugging experince
1864+
// here.
1865+
// }
1866+
} catch (e) {
1867+
// utils.log.error('failed upload');
18581868
}
1859-
// else {
1860-
// all the events are still queued, and will be retried when the next
1861-
// event is sent In the interest of debugging, it would be nice to have
1862-
// something like an event emitter for a better debugging experince
1863-
// here.
1864-
// }
1865-
} catch (e) {
1866-
// utils.log.error('failed upload');
1867-
}
1868-
});
1869+
});
1870+
} catch (e) {
1871+
const [status, response] = [0, 'Request failed to send'];
1872+
1873+
utils.log.error(response);
1874+
scope._logErrorsOnEvents(maxEventId, maxIdentifyId, status, response);
1875+
scope.removeEvents(maxEventId, maxIdentifyId, status, response, {
1876+
reason: e.message,
1877+
});
1878+
}
18691879
};
18701880

18711881
/**

test/amplitude-client.js

+16
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import Identify from '../src/identify.js';
1111
import constants from '../src/constants.js';
1212
import { mockCookie, restoreCookie, getCookie } from './mock-cookie';
1313
import { AmplitudeServerZone } from '../src/server-zone.js';
14+
import Request from '../src/xhr';
1415

1516
// maintain for testing backwards compatability
1617
describe('AmplitudeClient', function () {
@@ -4533,5 +4534,20 @@ describe('AmplitudeClient', function () {
45334534
assert.equal(amplitude.getSessionId(), startTime);
45344535
assert.equal(amplitude.options.userId, 'test user');
45354536
});
4537+
4538+
it('should handle failed requests to send events', function () {
4539+
const errorLogSpy = sinon.spy(utils.log, 'error');
4540+
const requestStub = sinon.stub(Request.prototype, 'send').throws();
4541+
4542+
const amplitude = new AmplitudeClient();
4543+
amplitude.init(apiKey);
4544+
amplitude.logEvent('testEvent1');
4545+
4546+
assert.isTrue(errorLogSpy.calledWith('Request failed to send'));
4547+
assert.equal(amplitude._unsentEvents.length, 0);
4548+
4549+
errorLogSpy.restore();
4550+
requestStub.restore();
4551+
});
45364552
});
45374553
});

0 commit comments

Comments
 (0)