From be779f4863fbae054742e89b02017fe775e17766 Mon Sep 17 00:00:00 2001 From: "Chapman, Christopher" Date: Mon, 15 May 2017 12:57:17 -0700 Subject: [PATCH 1/4] Fixing out of order messages with a priority queue The client uses a priorityQueue from an old version of async. For some reason this implementation puts items with the same priority ahead of existing tasks with the same priority. See: https://github.com/caolan/async/blob/621f13805aa326865b85dbbf7128baf7146ab976/lib/async.js#L1030 Since this is not being used as a priority queue anyways(no priority is being passed to the queue) we can fix this by simply switching to a normal queue. Problem solved :) --- lib/clients/client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/clients/client.js b/lib/clients/client.js index fe82ec3ff..dbe3f070d 100644 --- a/lib/clients/client.js +++ b/lib/clients/client.js @@ -59,7 +59,7 @@ function BaseAPIClient(token, opts) { * @type {Object} * @private */ - this.requestQueue = async.priorityQueue( + this.requestQueue = async.queue( bind(this._callTransport, this), clientOpts.maxRequestConcurrency || 3 ); From 33fe32556bc34318f5157597862172c8513d300e Mon Sep 17 00:00:00 2001 From: "Chapman, Christopher" Date: Mon, 15 May 2017 12:57:17 -0700 Subject: [PATCH 2/4] Fixing out of order messages with a priority queue The client uses a priorityQueue from an old version of async. For some reason this implementation puts items with the same priority ahead of existing tasks with the same priority. See: https://github.com/caolan/async/blob/621f13805aa326865b85dbbf7128baf7146ab976/lib/async.js#L1030 Since this is not being used as a priority queue anyways(no priority is being passed to the queue) we can fix this by simply switching to a normal queue. Problem solved :) Adding a test for the problem as well. Ensures that code is executed in order. --- test/clients/web/client.js | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/test/clients/web/client.js b/test/clients/web/client.js index 6b30ec256..b0d7ad0dc 100644 --- a/test/clients/web/client.js +++ b/test/clients/web/client.js @@ -72,6 +72,39 @@ describe('Web API Client', function () { }); }); + it('should make API calls in the order they are executed', function (done) { + var args1 = { + headers: {}, + statusCode: 200, + body: '{"test": 10}' + }; + + var args2 = { + headers: {}, + statusCode: 200, + body: '{"test": 20}' + }; + + var client = new WebAPIClient('test-token', { transport: mockTransport }); + sinon.spy(client, 'transport'); + + client._makeAPICall('test', args1, null, function(){ + expect(client.transport.callCount).to.equal(1); + expect(client.transport.args[0][0].data.body).to.equal('{"test": 10}'); + expect(client.transport.args.length).to.equal(1); + + }) + client._makeAPICall('test', args2, null, function(){ + expect(client.transport.callCount).to.equal(2); + expect(client.transport.args[0][0].data.body).to.equal('{"test": 10}'); + expect(client.transport.args[1][0].data.body).to.equal('{"test": 20}'); + expect(client.transport.args.length).to.equal(2); + + }) + done() + + }); + it('should not crash when no callback is supplied to an API request', function () { var client = new WebAPIClient('test-token', { transport: mockTransport }); @@ -92,6 +125,7 @@ describe('Web API Client', function () { }); sinon.spy(client, 'transport'); + client._makeAPICall('test', {}, null, function () { expect(client.transport.callCount).to.equal(2); done(); From a9f224664cc1e2885e860ff3d2bdf2ead722beeb Mon Sep 17 00:00:00 2001 From: "Chapman, Christopher" Date: Mon, 15 May 2017 14:01:22 -0700 Subject: [PATCH 3/4] Fixing styles in test --- test/clients/web/client.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/test/clients/web/client.js b/test/clients/web/client.js index b0d7ad0dc..3507c4229 100644 --- a/test/clients/web/client.js +++ b/test/clients/web/client.js @@ -88,21 +88,19 @@ describe('Web API Client', function () { var client = new WebAPIClient('test-token', { transport: mockTransport }); sinon.spy(client, 'transport'); - client._makeAPICall('test', args1, null, function(){ + client._makeAPICall('test', args1, null, function () { expect(client.transport.callCount).to.equal(1); expect(client.transport.args[0][0].data.body).to.equal('{"test": 10}'); expect(client.transport.args.length).to.equal(1); - - }) - client._makeAPICall('test', args2, null, function(){ + }); + client._makeAPICall('test', args2, null, function () { expect(client.transport.callCount).to.equal(2); expect(client.transport.args[0][0].data.body).to.equal('{"test": 10}'); expect(client.transport.args[1][0].data.body).to.equal('{"test": 20}'); expect(client.transport.args.length).to.equal(2); - }) - done() - + }); + done(); }); it('should not crash when no callback is supplied to an API request', function () { From 5728892ebc574b92a6b8175ce83545393c301690 Mon Sep 17 00:00:00 2001 From: chapmanc Date: Mon, 15 May 2017 14:10:56 -0700 Subject: [PATCH 4/4] Trying to fix the username associated with github --- test/clients/web/client.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/clients/web/client.js b/test/clients/web/client.js index 3507c4229..fe9bf7c34 100644 --- a/test/clients/web/client.js +++ b/test/clients/web/client.js @@ -123,7 +123,6 @@ describe('Web API Client', function () { }); sinon.spy(client, 'transport'); - client._makeAPICall('test', {}, null, function () { expect(client.transport.callCount).to.equal(2); done();