From ed0c7b6e77c1ff59f04b9390c55848071ca3671b Mon Sep 17 00:00:00 2001 From: Daniel Cadenas Date: Tue, 8 Nov 2016 18:24:11 -0300 Subject: [PATCH 1/3] Add timeout https://trello.com/c/Re6iuMRj/1899-add-timeout-to-the-node-library --- README.md | 2 ++ package.json | 2 +- src/client.js | 8 +++++++- test/client.js | 47 ++++++++++++++++++++++++++++++++++++++++------- 4 files changed, 50 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 593a935..75891b2 100644 --- a/README.md +++ b/README.md @@ -27,6 +27,7 @@ var clearbit = new Client({key: 'api_key'}); * `webhook_id` *String*: Custom identifier for the webhook request * `subscribe` *Boolean*: Set to `true` to subscribe to the changes * `stream` *Boolean*: Set to `true` to use the [streaming API](https://clearbit.com/docs?shell#streaming) instead of webhooks + * `timeout` *Integer*: The timeout in milliseconds after which a socket closed error will be thrown. ```js var Person = clearbit.Person; @@ -51,6 +52,7 @@ Person.find({email: 'email@domain.com'}) * `domain` *String*: The company domain to look up **(required)** * `webhook_id` *String*: Custom identifier for the webhook request * `stream` *Boolean*: Set to `true` to use the [streaming API](https://clearbit.com/docs?shell#streaming) instead of webhooks + * `timeout` *Integer*: The timeout in milliseconds after which a socket closed error will be thrown. ```js var Company = clearbit.Company; diff --git a/package.json b/package.json index 50674f3..dd926bc 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "clearbit", - "version": "1.2.3", + "version": "1.3.0", "description": "Client for Clearbit.co business intelligence APIs", "main": "./src", "scripts": { diff --git a/src/client.js b/src/client.js index fd8a84f..c889878 100644 --- a/src/client.js +++ b/src/client.js @@ -59,6 +59,12 @@ ClearbitClient.prototype.request = function (options) { method: 'get' }); + if (options.timeout) { + var timeout = options.timeout; + } else { + var timeout = options.stream ? 60000 : 10000; + } + return needle.requestAsync( options.method, this.url(options), @@ -66,7 +72,7 @@ ClearbitClient.prototype.request = function (options) { { json: options.json, headers: options.headers, - timeout: options.stream ? 60000 : 10000, + timeout: timeout, username: this.key, password: '', user_agent: 'ClearbitNode/v' + pkg.version diff --git a/test/client.js b/test/client.js index 333e129..5d52693 100644 --- a/test/client.js +++ b/test/client.js @@ -100,18 +100,51 @@ describe('Client', function () { }); }); - it('uses a timeout of 60 seconds for streaming requests', function () { + function requestWithOptions(clientRequestOptions) { sinon.stub(needle, 'request').yieldsAsync(null, {}, undefined); - return client.request({ + + return client + .request(clientRequestOptions) + .then(() => needle.request.firstCall.args[3]) + .finally(() => needle.request.restore()); + } + + it('can specify a custom timeout for a request', function () { + return requestWithOptions({ + api: 'person', + timeout: 30000 + }) + .then((needleOptions) => { + expect(needleOptions).to.have.property('timeout', 30000); + }); + }); + + it('sets the default timeout to 10 seconds', function () { + return requestWithOptions({ api: 'person' }) + .then((needleOptions) => { + expect(needleOptions).to.have.property('timeout', 10000); + }); + }); + + + it('uses a default timeout of 60 seconds for streaming requests', function () { + return requestWithOptions({ api: 'person', stream: true }) - .then(function () { - expect(needle.request.firstCall.args[3]) - .to.have.property('timeout', 60000); + .then((needleOptions) => { + expect(needleOptions).to.have.property('timeout', 60000); + }); + }); + + it('can specify a custom timeout for streaming requests', function () { + return requestWithOptions({ + api: 'person', + stream: true, + timeout: 30000 }) - .finally(function () { - needle.request.restore(); + .then((needleOptions) => { + expect(needleOptions).to.have.property('timeout', 30000); }); }); From 8e759b9ebf3104f29ba35d74e6a4326733c480df Mon Sep 17 00:00:00 2001 From: Daniel Cadenas Date: Tue, 8 Nov 2016 18:55:01 -0300 Subject: [PATCH 2/3] Add timeout to the blacklist --- src/resource.js | 2 +- test/person.js | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/resource.js b/src/resource.js index 338bf91..70e88bf 100644 --- a/src/resource.js +++ b/src/resource.js @@ -111,7 +111,7 @@ function extractParams (options) { var params = _.omit(options || {}, 'path', 'method', 'params', 'client', 'api', 'stream', - 'headers' + 'headers', 'timeout' ); return _.isEmpty(params) ? null : params; diff --git a/test/person.js b/test/person.js index 7879206..683a82f 100644 --- a/test/person.js +++ b/test/person.js @@ -38,6 +38,13 @@ describe('Person', function () { return Person.find({email: 'alex@alexmaccaw.com', subscribe: true}); }); + it('removes non query options from the url', function () { + mock + .get('/v2/people/find?email=alex%40alexmaccaw.com') + .reply(200, alex); + return Person.find({email: 'alex@alexmaccaw.com', timeout: 10000}); + }); + it('can handle queued requests', function () { mock .get('/v2/people/find?email=alex%40alexmaccaw.com') From b62c9fe0855b687811cb09ee81d9e04c533be9a4 Mon Sep 17 00:00:00 2001 From: Daniel Cadenas Date: Wed, 9 Nov 2016 14:21:59 -0300 Subject: [PATCH 3/3] Make linter happy --- src/client.js | 6 +----- test/client.js | 12 ++++++------ 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/client.js b/src/client.js index c889878..cb8700b 100644 --- a/src/client.js +++ b/src/client.js @@ -59,11 +59,7 @@ ClearbitClient.prototype.request = function (options) { method: 'get' }); - if (options.timeout) { - var timeout = options.timeout; - } else { - var timeout = options.stream ? 60000 : 10000; - } + var timeout = options.timeout || options.stream && 60000 || 10000; return needle.requestAsync( options.method, diff --git a/test/client.js b/test/client.js index 5d52693..4da9fc9 100644 --- a/test/client.js +++ b/test/client.js @@ -105,8 +105,8 @@ describe('Client', function () { return client .request(clientRequestOptions) - .then(() => needle.request.firstCall.args[3]) - .finally(() => needle.request.restore()); + .then(function() { return needle.request.firstCall.args[3]; }) + .finally(function() { return needle.request.restore(); }); } it('can specify a custom timeout for a request', function () { @@ -114,14 +114,14 @@ describe('Client', function () { api: 'person', timeout: 30000 }) - .then((needleOptions) => { + .then(function(needleOptions) { expect(needleOptions).to.have.property('timeout', 30000); }); }); it('sets the default timeout to 10 seconds', function () { return requestWithOptions({ api: 'person' }) - .then((needleOptions) => { + .then(function(needleOptions) { expect(needleOptions).to.have.property('timeout', 10000); }); }); @@ -132,7 +132,7 @@ describe('Client', function () { api: 'person', stream: true }) - .then((needleOptions) => { + .then(function(needleOptions) { expect(needleOptions).to.have.property('timeout', 60000); }); }); @@ -143,7 +143,7 @@ describe('Client', function () { stream: true, timeout: 30000 }) - .then((needleOptions) => { + .then(function(needleOptions) { expect(needleOptions).to.have.property('timeout', 30000); }); });