From 63d95b631698b4f6faa5d6d458d573eadc8f6859 Mon Sep 17 00:00:00 2001 From: Vsevolod Strukchinsky Date: Mon, 6 Jul 2015 12:04:41 +0500 Subject: [PATCH 1/7] Drop infinity-agent Overall it is a bad practice to 'fix' internal modules within the other module. I think graceful-fs (https://github.com/isaacs/node-graceful-fs#global-patching) is good example, how to do this: ```js var http = require('http'); var https = requrie('https'); var infinityAgent = require('infinity-agent'); infinityAgent.patch(http); infinityAgent.patch(https); ``` This will enable global agent tuning in edge cases. Closes #73 and closes #60 --- index.js | 28 +--------------------------- readme.md | 20 ++++++++++++-------- 2 files changed, 13 insertions(+), 35 deletions(-) diff --git a/index.js b/index.js index 21d171414..d60badb77 100644 --- a/index.js +++ b/index.js @@ -6,7 +6,6 @@ var util = require('util'); var zlib = require('zlib'); var querystring = require('querystring'); var objectAssign = require('object-assign'); -var infinityAgent = require('infinity-agent'); var duplexify = require('duplexify'); var isStream = require('is-stream'); var readAllStream = require('read-all-stream'); @@ -35,9 +34,7 @@ function got(url, opts, cb) { } opts = objectAssign( - { - protocol: 'http:' - }, + {protocol: 'http:'}, typeof url === 'string' ? urlLib.parse(prependHttp(url)) : url, opts ); @@ -110,29 +107,6 @@ function got(url, opts, cb) { var fn = opts.protocol === 'https:' ? https : http; var url = urlLib.format(opts); - if (opts.agent === undefined) { - opts.agent = infinityAgent[fn === https ? 'https' : 'http'].globalAgent; - - if (process.version.indexOf('v0.10') === 0 && fn === https && ( - typeof opts.ca !== 'undefined' || - typeof opts.cert !== 'undefined' || - typeof opts.ciphers !== 'undefined' || - typeof opts.key !== 'undefined' || - typeof opts.passphrase !== 'undefined' || - typeof opts.pfx !== 'undefined' || - typeof opts.rejectUnauthorized !== 'undefined')) { - opts.agent = new infinityAgent.https.Agent({ - ca: opts.ca, - cert: opts.cert, - ciphers: opts.ciphers, - key: opts.key, - passphrase: opts.passphrase, - pfx: opts.pfx, - rejectUnauthorized: opts.rejectUnauthorized - }); - } - } - var req = fn.request(opts, function (response) { var statusCode = response.statusCode; var res = response; diff --git a/readme.md b/readme.md index ca44f379f..2c32cdd23 100644 --- a/readme.md +++ b/readme.md @@ -102,14 +102,6 @@ Type: `number` Milliseconds after which the request will be aborted and an error event with `ETIMEDOUT` code will be emitted. -###### agent - -[http.Agent](http://nodejs.org/api/http.html#http_class_http_agent) instance. - -If `undefined` - [`infinity-agent`](https://github.com/floatdrop/infinity-agent) will be used to backport Agent class from Node.js core. - -To use default [globalAgent](http://nodejs.org/api/http.html#http_http_globalagent) just pass `null`. - ##### callback(error, data, response) ###### error @@ -185,6 +177,18 @@ got('todomvc.com', { ``` +## Node 0.10 + +It is a known issue with Node [http.Agent](https://nodejs.org/docs/v0.10.39/api/http.html#http_class_http_agent) and `agent.maxSockets`, which is set to `5`. This can cause low performance of application and (in rare cases) deadlocks. To avoid this you can set it manually: + +```js +require('http').globalAgent.maxSockets = Infinity; +require('https').globalAgent.maxSockets = Infinity; +``` + +This should only ever be done at the top-level application layer. + + ## Related - [gh-got](https://github.com/sindresorhus/gh-got) - Convenience wrapper for interacting with the GitHub API From 04dbc4b7289f4642feefc20aad5f53812cdf0514 Mon Sep 17 00:00:00 2001 From: Vsevolod Strukchinsky Date: Thu, 9 Jul 2015 11:26:19 +0500 Subject: [PATCH 2/7] Add Promise API * Base logic moved into event-emitter and all APIs wrapped around it * read-all-stream combined into one place - less code duplication * Unzip stream now inherits all properties and function from response and returned instead of response Closes #66 --- index.js | 342 +++++++++++++++++++++++------------------ package.json | 2 +- readme.md | 21 ++- test/test-helpers.js | 22 ++- test/test-http.js | 36 +---- test/test-json.js | 7 - test/test-post.js | 26 ---- test/test-redirects.js | 20 +-- test/test-stream.js | 90 +++++++++++ 9 files changed, 323 insertions(+), 243 deletions(-) create mode 100644 test/test-stream.js diff --git a/index.js b/index.js index d60badb77..a1d1cd7ef 100644 --- a/index.js +++ b/index.js @@ -1,4 +1,5 @@ 'use strict'; +var EventEmitter = require('events').EventEmitter; var http = require('http'); var https = require('https'); var urlLib = require('url'); @@ -14,6 +15,7 @@ var prependHttp = require('prepend-http'); var lowercaseKeys = require('lowercase-keys'); var isRedirect = require('is-redirect'); var NestedErrorStacks = require('nested-error-stacks'); +var pinkiePromise = require('pinkie-promise'); function GotError(message, nested) { NestedErrorStacks.call(this, message, nested); @@ -23,196 +25,157 @@ function GotError(message, nested) { util.inherits(GotError, NestedErrorStacks); GotError.prototype.name = 'GotError'; -function got(url, opts, cb) { - if (typeof url !== 'string' && typeof url !== 'object') { - throw new GotError('Parameter `url` must be a string or object, not ' + typeof url); - } - - if (typeof opts === 'function') { - cb = opts; - opts = {}; - } - - opts = objectAssign( - {protocol: 'http:'}, - typeof url === 'string' ? urlLib.parse(prependHttp(url)) : url, - opts - ); - - opts.headers = objectAssign({ - 'user-agent': 'https://github.com/sindresorhus/got', - 'accept-encoding': 'gzip,deflate' - }, lowercaseKeys(opts.headers)); - - if (opts.pathname) { - opts.path = opts.pathname; - } - - if (opts.query) { - if (typeof opts.query !== 'string') { - opts.query = querystring.stringify(opts.query); - } +function requestAsEventEmitter(opts) { + opts = opts || {}; - opts.path = opts.pathname + '?' + opts.query; - delete opts.query; - } - - var encoding = opts.encoding; - var body = opts.body; - var json = opts.json; - var timeout = opts.timeout; - var proxy; + var ee = new EventEmitter(); var redirectCount = 0; - delete opts.encoding; - delete opts.body; - delete opts.json; - delete opts.timeout; - - if (json) { - opts.headers.accept = opts.headers.accept || 'application/json'; - } - - if (body) { - if (typeof body !== 'string' && !Buffer.isBuffer(body) && !isStream.readable(body)) { - throw new GotError('options.body must be a ReadableStream, string or Buffer'); - } - - opts.method = opts.method || 'POST'; - - if (!opts.headers['content-length'] && !opts.headers['transfer-encoding'] && !isStream.readable(body)) { - var length = typeof body === 'string' ? Buffer.byteLength(body) : body.length; - opts.headers['content-length'] = length; - } - } - - opts.method = opts.method || 'GET'; - - // returns a proxy stream to the response - // if no callback has been provided - if (!cb) { - proxy = duplexify(); - - // forward errors on the stream - cb = function (err, data, response) { - proxy.emit('error', err, data, response); - }; - } - - if (proxy && json) { - throw new GotError('got can not be used as stream when options.json is used'); - } - - function get(opts, cb) { + function get(opts) { var fn = opts.protocol === 'https:' ? https : http; var url = urlLib.format(opts); - var req = fn.request(opts, function (response) { - var statusCode = response.statusCode; - var res = response; - - // auto-redirect only for GET and HEAD methods + var req = fn.request(opts, function (res) { + var statusCode = res.statusCode; if (isRedirect(statusCode) && 'location' in res.headers && (opts.method === 'GET' || opts.method === 'HEAD')) { - // discard response res.resume(); if (++redirectCount > 10) { - cb(new GotError('Redirected 10 times. Aborting.'), undefined, res); + ee.emit('error', new GotError('Redirected 10 times. Aborting.'), undefined, res); return; } var redirectUrl = urlLib.resolve(url, res.headers.location); var redirectOpts = objectAssign({}, opts, urlLib.parse(redirectUrl)); - if (opts.agent === infinityAgent.http.globalAgent && redirectOpts.protocol === 'https:' && opts.protocol === 'http:') { - redirectOpts.agent = undefined; - } - - if (proxy) { - proxy.emit('redirect', res, redirectOpts); - } + ee.emit('redirect', res, redirectOpts); - get(redirectOpts, cb); + get(redirectOpts); return; } - if (proxy) { - proxy.emit('response', res); - } - if (['gzip', 'deflate'].indexOf(res.headers['content-encoding']) !== -1) { - res = res.pipe(zlib.createUnzip()); + var unzip = zlib.createUnzip(); + unzip.httpVersion = res.httpVersion; + unzip.headers = res.headers; + unzip.rawHeaders = res.rawHeaders; + unzip.trailers = res.trailers; + unzip.rawTrailers = res.rawTrailers; + unzip.setTimeout = res.setTimeout.bind(res); + unzip.statusCode = res.statusCode; + unzip.statusMessage = res.statusMessage; + unzip.socket = res.socket; + res = res.pipe(unzip); } - if (statusCode < 200 || statusCode > 299) { - readAllStream(res, encoding, function (err, data) { - err = new GotError(opts.method + ' ' + url + ' response code is ' + statusCode + ' (' + http.STATUS_CODES[statusCode] + ')', err); - err.code = statusCode; + ee.emit('response', res); + }).once('error', function (err) { + ee.emit('error', new GotError('Request to ' + url + ' failed', err)); + }); - if (data && json) { - try { - data = JSON.parse(data); - } catch (e) { - err = new GotError('Parsing ' + url + ' response failed', new GotError(e.message, err)); - } - } + if (opts.timeout) { + timedOut(req, opts.timeout); + } + + ee.emit('request', req); + } + + setImmediate(get, opts); // quirk to attach event listeners + return ee; +} - cb(err, data, response); - }); +function asCallback(opts, cb) { + var ee = requestAsEventEmitter(opts); + var url = urlLib.format(opts); + ee.on('request', function (req) { + if (isStream.readable(opts.body)) { + opts.body.pipe(req); + opts.body = undefined; + return; + } + + req.end(opts.body); + }); + + ee.on('response', function (res) { + readAllStream(res, opts.encoding, function (err, data) { + if (err) { + cb(new GotError('Reading ' + url + ' response failed', err), null, res); return; } - // pipe the response to the proxy if in proxy mode - if (proxy) { - proxy.setReadable(res); - return; + var statusCode = res.statusCode; + + if (statusCode < 200 || statusCode > 299) { + err = new GotError(opts.method + ' ' + url + ' response code is ' + statusCode + ' (' + http.STATUS_CODES[statusCode] + ')', err); + err.code = statusCode; } - readAllStream(res, encoding, function (err, data) { - if (err) { - err = new GotError('Reading ' + url + ' response failed', err); - } else if (json && statusCode !== 204) { - // only parse json if the option is enabled, and the response - // is not a 204 (empty reponse) - try { - data = JSON.parse(data); - } catch (e) { - err = new GotError('Parsing ' + url + ' response failed', e); - } + if (opts.json && statusCode !== 204) { + try { + data = JSON.parse(data); + } catch (e) { + err = new GotError('Parsing ' + url + ' response failed', new GotError(e.message, err)); } + } - cb(err, data, response); - }); - }).once('error', function (err) { - cb(new GotError('Request to ' + url + ' failed', err)); + cb(err, data, res); }); + }); - if (timeout) { - timedOut(req, timeout); - } + ee.on('error', cb); +} - if (!proxy) { - if (isStream.readable(body)) { - body.pipe(req); - } else { - req.end(body); - } +function asPromise(opts) { + var resolve, reject; + + var promise = new pinkiePromise(function (_resolve, _reject) { + resolve = _resolve; + reject = _reject; + }); + + var cb = function (err, data, response) { + response.body = data; + if (err) { + err.response = response; + reject(err); return; } - if (body) { - proxy.write = function () { - throw new Error('got\'s stream is not writable when options.body is used'); - }; + resolve(response); + }; - if (isStream.readable(body)) { - body.pipe(req); - } else { - req.end(body); - } + asCallback(opts, cb); + return promise; +} + +function asStream(opts) { + var proxy = duplexify(); + + if (opts.json) { + throw new GotError('got can not be used as stream when options.json is used'); + } + + if (opts.body) { + proxy.write = function () { + throw new Error('got\'s stream is not writable when options.body is used'); + }; + } + + var ee = requestAsEventEmitter(opts); + + ee.on('request', function (req) { + proxy.emit('request', req); + + if (isStream.readable(opts.body)) { + opts.body.pipe(req); + return; + } + if (opts.body) { + req.end(opts.body); return; } @@ -222,13 +185,88 @@ function got(url, opts, cb) { } req.end(); - } + }); + + ee.on('response', function (res) { + proxy.setReadable(res); + proxy.emit('response', res); + }); - get(opts, cb); + ee.on('redirect', proxy.emit.bind(proxy, 'redirect')); return proxy; } +function normalizeArguments(url, opts) { + if (typeof url !== 'string' && typeof url !== 'object') { + throw new GotError('Parameter `url` must be a string or object, not ' + typeof url); + } + + opts = objectAssign( + { + protocol: 'http:' + }, + typeof url === 'string' ? urlLib.parse(prependHttp(url)) : url, + opts + ); + + opts.headers = objectAssign({ + 'user-agent': 'https://github.com/sindresorhus/got', + 'accept-encoding': 'gzip,deflate' + }, lowercaseKeys(opts.headers)); + + if (opts.pathname) { + opts.path = opts.pathname; + } + + var query = opts.query; + if (query) { + if (typeof query !== 'string') { + opts.query = querystring.stringify(query); + } + + opts.path = opts.pathname + '?' + opts.query; + delete opts.query; + } + + if (opts.json) { + opts.headers.accept = opts.headers.accept || 'application/json'; + } + + var body = opts.body; + if (body) { + if (typeof body !== 'string' && !Buffer.isBuffer(body) && !isStream.readable(body)) { + throw new GotError('options.body must be a ReadableStream, string or Buffer'); + } + + opts.method = opts.method || 'POST'; + + if (!opts.headers['content-length'] && !opts.headers['transfer-encoding'] && !isStream.readable(body)) { + var length = typeof body === 'string' ? Buffer.byteLength(body) : body.length; + opts.headers['content-length'] = length; + } + } + + opts.method = opts.method || 'GET'; + + return opts; +} + +function got(url, opts, cb) { + if (typeof opts === 'function') { + cb = opts; + opts = {}; + } + + opts = normalizeArguments(url, opts); + + if (cb) { + return asCallback(opts, cb); + } + + return asPromise(opts); +} + [ 'get', 'post', @@ -247,4 +285,8 @@ function got(url, opts, cb) { }; }); +got.stream = function (url, opts) { + return asStream(normalizeArguments(url, opts)); +}; + module.exports = got; diff --git a/package.json b/package.json index 5f14c07d4..46e1fac61 100644 --- a/package.json +++ b/package.json @@ -43,12 +43,12 @@ ], "dependencies": { "duplexify": "^3.2.0", - "infinity-agent": "^2.0.0", "is-redirect": "^1.0.0", "is-stream": "^1.0.0", "lowercase-keys": "^1.0.0", "nested-error-stacks": "^1.0.0", "object-assign": "^3.0.0", + "pinkie-promise": "^1.0.0", "prepend-http": "^1.0.0", "read-all-stream": "^3.0.0", "timed-out": "^2.0.0" diff --git a/readme.md b/readme.md index 2c32cdd23..65b6cb533 100644 --- a/readme.md +++ b/readme.md @@ -35,12 +35,18 @@ got('todomvc.com', function (err, data, res) { //=> ... }); +// Promise mode +got('todomvc.com') + .then(function (res) { + console.log(res.body); + }) + .catch(console.error); // Stream mode -got('todomvc.com').pipe(fs.createWriteStream('index.html')); +got.stream('todomvc.com').pipe(fs.createWriteStream('index.html')); -// For POST, PUT and PATCH methods got returns a WritableStream -fs.createReadStream('index.html').pipe(got.post('todomvc.com')); +// For POST, PUT and PATCH methods got.stream returns a WritableStream +fs.createReadStream('index.html').pipe(got.stream('todomvc.com', {method: 'POST'})); ``` ### API @@ -104,6 +110,8 @@ Milliseconds after which the request will be aborted and an error event with `ET ##### callback(error, data, response) +Function to be called, when error or data recieved. If omitted - Promise will be returned. + ###### error `Error` object with HTTP status code as `code` property. @@ -118,6 +126,10 @@ The [response object](http://nodejs.org/api/http.html#http_http_incomingmessage) When in stream mode, you can listen for events: +##### .on('request', request) + +`request` event to get the request object of the request. + ##### .on('response', response) `response` event to get the response object of the final request. @@ -130,9 +142,6 @@ When in stream mode, you can listen for events: `error` event emitted in case of protocol error (like `ENOTFOUND` etc.) or status error (4xx or 5xx). Second argument is body of server response in case of status error. Third argument is response object. -###### response - -The [response object](http://nodejs.org/api/http.html#http_http_incomingmessage). #### got.get(url, [options], [callback]) #### got.post(url, [options], [callback]) diff --git a/test/test-helpers.js b/test/test-helpers.js index 0bdf2d1e5..beda26860 100644 --- a/test/test-helpers.js +++ b/test/test-helpers.js @@ -8,13 +8,18 @@ s.on('/', function (req, res) { res.end('ok'); }); +s.on('/404', function (req, res) { + res.statusCode = 404; + res.end('not found'); +}); + test('setup', function (t) { s.listen(s.port, function () { t.end(); }); }); -test('callback mode', {timeout: 1000}, function (t) { +test('callback mode', function (t) { got.get(s.url, function (err, data) { t.error(err); t.equal(data, 'ok'); @@ -22,11 +27,18 @@ test('callback mode', {timeout: 1000}, function (t) { }); }); -test('stream mode', {timeout: 1000}, function (t) { +test('promise mode', function (t) { + t.plan(2); + got.get(s.url) - .on('data', function (data) { - t.equal(data.toString(), 'ok'); - t.end(); + .then(function (res) { + t.equal(res.body, 'ok'); + }); + + got.get(s.url + '/404') + .catch(function (err) { + t.equal(err.message, 'GET http://localhost:6767/404 response code is 404 (Not Found)'); + t.equal(err.response.body, 'not found'); }); }); diff --git a/test/test-http.js b/test/test-http.js index df909a4e8..10d13404d 100644 --- a/test/test-http.js +++ b/test/test-http.js @@ -70,39 +70,11 @@ test('buffer on encoding === null', function (t) { }); }); -test('stream mode', function (t) { - got(s.url) - .on('data', function (data) { - t.equal(data.toString(), 'ok'); - t.end(); - }); -}); - -test('emit response object to stream', function (t) { - got(s.url) - .on('response', function (res) { - t.ok(res); - t.ok(res.headers); - t.end(); - }); -}); - -test('proxy errors to the stream', function (t) { - got(s.url + '/404') - .on('error', function (err, data, res) { - t.equal(err.code, 404); - t.equal(data, 'not'); - t.ok(res); - t.end(); - }); -}); - test('timeout option', function (t) { - got(s.url + '/404', {timeout: 1}) - .on('error', function (err) { - t.equal(err.code, 'ETIMEDOUT'); - t.end(); - }); + got(s.url + '/404', {timeout: 1}, function (err) { + t.equal(err.code, 'ETIMEDOUT'); + t.end(); + }); }); test('query option', function (t) { diff --git a/test/test-json.js b/test/test-json.js index 86fcbd30e..539d19922 100644 --- a/test/test-json.js +++ b/test/test-json.js @@ -33,13 +33,6 @@ test('setup', function (t) { }); }); -test('json option can not be used in stream mode', function (t) { - t.throws(function () { - got(s.url, {json: true}); - }, 'got can not be used as stream when options.json is used'); - t.end(); -}); - test('json option should parse response', function (t) { got(s.url, {json: true}, function (err, json) { t.error(err); diff --git a/test/test-post.js b/test/test-post.js index 9dabbbedc..19af0b0d9 100644 --- a/test/test-post.js +++ b/test/test-post.js @@ -67,32 +67,6 @@ test('works with empty post response', function (t) { }); }); -test('return readable stream', function (t) { - got.post(s.url, {body: from2Array(['wow'])}) - .on('data', function (data) { - t.equal(data.toString(), 'wow'); - t.end(); - }); -}); - -test('return writeable stream', function (t) { - got.post(s.url) - .on('data', function (data) { - t.equal(data.toString(), 'wow'); - t.end(); - }) - .end('wow'); -}); - -test('throws on write to stream with body specified', function (t) { - t.throws(function () { - got(s.url, {body: 'wow'}).write('wow'); - }); - - // wait for request to end - setTimeout(t.end.bind(t), 10); -}); - test('post have content-length header to string', function (t) { t.plan(5); diff --git a/test/test-redirects.js b/test/test-redirects.js index 2a7e04d57..56e39bb5f 100644 --- a/test/test-redirects.js +++ b/test/test-redirects.js @@ -4,6 +4,10 @@ var got = require('../'); var server = require('./server.js'); var s = server.createServer(); +s.on('/', function (req, res) { + res.end('reached'); +}); + s.on('/finite', function (req, res) { res.writeHead(302, { location: s.url + '/' @@ -32,10 +36,6 @@ s.on('/relativeQuery?bang', function (req, res) { res.end(); }); -s.on('/', function (req, res) { - res.end('reached'); -}); - test('setup', function (t) { s.listen(s.port, function () { t.end(); @@ -90,18 +90,6 @@ test('redirect only GET and HEAD requests', function (t) { }); }); -test('redirect event', function (t) { - got(s.url + '/endless') - .on('redirect', function (res, opts) { - t.equal(res.headers.location, s.url + '/endless'); - opts.path = '/'; - }) - .on('data', function (data) { - t.equal(data.toString(), 'reached'); - t.end(); - }); -}); - test('cleanup', function (t) { s.close(); t.end(); diff --git a/test/test-stream.js b/test/test-stream.js new file mode 100644 index 000000000..c11a17b35 --- /dev/null +++ b/test/test-stream.js @@ -0,0 +1,90 @@ +'use strict'; + +var test = require('tap').test; +var from2Array = require('from2-array'); +var got = require('../'); +var server = require('./server.js'); +var s = server.createServer(); + +s.on('/', function (req, res) { + res.end('ok'); +}); + +s.on('/post', function (req, res) { + req.pipe(res); +}); + +s.on('/redirect', function (req, res) { + res.writeHead(302, { + location: s.url + }); + res.end(); +}); + +test('setup', function (t) { + s.listen(s.port, function () { + t.end(); + }); +}); + +test('json option can not be used in stream mode', function (t) { + t.throws(function () { + got.stream(s.url, {json: true}); + }, 'got can not be used as stream when options.json is used'); + t.end(); +}); + +test('return readable stream', function (t) { + got.stream(s.url) + .on('data', function (data) { + t.equal(data.toString(), 'ok'); + t.end(); + }); +}); + +test('return writeable stream', function (t) { + t.plan(1); + got.stream(s.url + '/post', {method: 'POST'}) + .on('data', function (data) { + t.equal(data.toString(), 'wow'); + }) + .end('wow'); +}); + +test('throws on write to stream with body specified', function (t) { + t.throws(function () { + got.stream(s.url, {body: 'wow'}).write('wow'); + }, 'got\'s stream is not writable when options.body is used'); + + // wait for request to end + setTimeout(t.end.bind(t), 10); +}); + +test('request event', function (t) { + got.stream(s.url) + .on('request', function (req) { + t.ok(req); + t.end(); + }); +}); + +test('redirect event', function (t) { + got.stream(s.url + '/redirect') + .on('redirect', function (res, opts) { + t.equal(res.headers.location, s.url); + t.end(); + }); +}); + +test('response event', function (t) { + got.stream(s.url) + .on('response', function (res) { + t.equal(res.statusCode, 200); + t.end(); + }); +}); + +test('cleanup', function (t) { + s.close(); + t.end(); +}); From 16f93eaff162c80a732c63f1957b5d1fc6760705 Mon Sep 17 00:00:00 2001 From: Vsevolod Strukchinsky Date: Thu, 16 Jul 2015 12:39:00 +0500 Subject: [PATCH 3/7] Quick shims --- index.js | 35 ++++++++++++++--------------------- package.json | 3 +-- readme.md | 6 +++--- 3 files changed, 18 insertions(+), 26 deletions(-) diff --git a/index.js b/index.js index a1d1cd7ef..b182a10ea 100644 --- a/index.js +++ b/index.js @@ -128,26 +128,20 @@ function asCallback(opts, cb) { } function asPromise(opts) { - var resolve, reject; + var promise = new pinkiePromise(function (resolve, reject) { + asCallback(opts, function (err, data, response) { + response.body = data; - var promise = new pinkiePromise(function (_resolve, _reject) { - resolve = _resolve; - reject = _reject; - }); - - var cb = function (err, data, response) { - response.body = data; - - if (err) { - err.response = response; - reject(err); - return; - } + if (err) { + err.response = response; + reject(err); + return; + } - resolve(response); - }; + resolve(response); + }); + }); - asCallback(opts, cb); return promise; } @@ -203,9 +197,7 @@ function normalizeArguments(url, opts) { } opts = objectAssign( - { - protocol: 'http:' - }, + {protocol: 'http:'}, typeof url === 'string' ? urlLib.parse(prependHttp(url)) : url, opts ); @@ -261,7 +253,8 @@ function got(url, opts, cb) { opts = normalizeArguments(url, opts); if (cb) { - return asCallback(opts, cb); + asCallback(opts, cb); + return; } return asPromise(opts); diff --git a/package.json b/package.json index 46e1fac61..7570a971a 100644 --- a/package.json +++ b/package.json @@ -20,8 +20,7 @@ "node": ">=0.10.0" }, "scripts": { - "test": "tap test/test-*.js", - "coverage": "istanbul cover tape --report html -- test/test-*.js" + "test": "tap test/test-*.js" }, "files": [ "index.js" diff --git a/readme.md b/readme.md index 65b6cb533..41803a35f 100644 --- a/readme.md +++ b/readme.md @@ -186,16 +186,16 @@ got('todomvc.com', { ``` -## Node 0.10 +## Node 0.10.x -It is a known issue with Node [http.Agent](https://nodejs.org/docs/v0.10.39/api/http.html#http_class_http_agent) and `agent.maxSockets`, which is set to `5`. This can cause low performance of application and (in rare cases) deadlocks. To avoid this you can set it manually: +It is a known issue with old good Node 0.10.x [http.Agent](https://nodejs.org/docs/v0.10.39/api/http.html#http_class_http_agent) and `agent.maxSockets`, which is set to `5`. This can cause low performance of application and (in rare cases) deadlocks. To avoid this you can set it manually: ```js require('http').globalAgent.maxSockets = Infinity; require('https').globalAgent.maxSockets = Infinity; ``` -This should only ever be done at the top-level application layer. +This should only ever be done if you have Node version 0.10.x and at the top-level application layer. ## Related From a1eb3f7801a302667dd3995fd2ed261d2b180287 Mon Sep 17 00:00:00 2001 From: Vsevolod Strukchinsky Date: Thu, 16 Jul 2015 13:12:18 +0500 Subject: [PATCH 4/7] Move setImmediate closer to sync event > EventEmitter was made to be used as an abstraction around asynchronous events. Thus, when an event is emitted it will have happened on a different tick than when the event was set. See https://github.com/joyent/node/issues/8470#issuecomment-58315886 --- index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index b182a10ea..ff4e194f0 100644 --- a/index.js +++ b/index.js @@ -77,10 +77,10 @@ function requestAsEventEmitter(opts) { timedOut(req, opts.timeout); } - ee.emit('request', req); + setImmediate(ee.emit.bind(ee), 'request', req); } - setImmediate(get, opts); // quirk to attach event listeners + get(opts); return ee; } From 00a9129b856aff74ffe0286780754265947bc2cd Mon Sep 17 00:00:00 2001 From: Vsevolod Strukchinsky Date: Thu, 16 Jul 2015 13:32:35 +0500 Subject: [PATCH 5/7] Add helpers for stream API --- index.js | 12 ++++++++++-- readme.md | 2 +- test/test-stream.js | 2 +- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index ff4e194f0..44f453a16 100644 --- a/index.js +++ b/index.js @@ -260,14 +260,16 @@ function got(url, opts, cb) { return asPromise(opts); } -[ +var helpers = [ 'get', 'post', 'put', 'patch', 'head', 'delete' -].forEach(function (el) { +]; + +helpers.forEach(function (el) { got[el] = function (url, opts, cb) { if (typeof opts === 'function') { cb = opts; @@ -282,4 +284,10 @@ got.stream = function (url, opts) { return asStream(normalizeArguments(url, opts)); }; +helpers.forEach(function (el) { + got.stream[el] = function (url, opts) { + return got.stream(url, objectAssign({}, opts, {method: el.toUpperCase()})); + }; +}); + module.exports = got; diff --git a/readme.md b/readme.md index 41803a35f..8a723830a 100644 --- a/readme.md +++ b/readme.md @@ -46,7 +46,7 @@ got('todomvc.com') got.stream('todomvc.com').pipe(fs.createWriteStream('index.html')); // For POST, PUT and PATCH methods got.stream returns a WritableStream -fs.createReadStream('index.html').pipe(got.stream('todomvc.com', {method: 'POST'})); +fs.createReadStream('index.html').pipe(got.stream.post('todomvc.com')); ``` ### API diff --git a/test/test-stream.js b/test/test-stream.js index c11a17b35..5cf57ac6f 100644 --- a/test/test-stream.js +++ b/test/test-stream.js @@ -44,7 +44,7 @@ test('return readable stream', function (t) { test('return writeable stream', function (t) { t.plan(1); - got.stream(s.url + '/post', {method: 'POST'}) + got.stream.post(s.url + '/post') .on('data', function (data) { t.equal(data.toString(), 'wow'); }) From b38a7975cc3a2aaa66856a783b71e5671fdcb83f Mon Sep 17 00:00:00 2001 From: Vsevolod Strukchinsky Date: Sat, 18 Jul 2015 21:34:05 +0500 Subject: [PATCH 6/7] Use unzip-response to unzip stream --- index.js | 16 ++-------------- package.json | 3 ++- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/index.js b/index.js index 44f453a16..150cd9092 100644 --- a/index.js +++ b/index.js @@ -4,7 +4,6 @@ var http = require('http'); var https = require('https'); var urlLib = require('url'); var util = require('util'); -var zlib = require('zlib'); var querystring = require('querystring'); var objectAssign = require('object-assign'); var duplexify = require('duplexify'); @@ -16,6 +15,7 @@ var lowercaseKeys = require('lowercase-keys'); var isRedirect = require('is-redirect'); var NestedErrorStacks = require('nested-error-stacks'); var pinkiePromise = require('pinkie-promise'); +var unzipResponse = require('unzip-response'); function GotError(message, nested) { NestedErrorStacks.call(this, message, nested); @@ -54,19 +54,7 @@ function requestAsEventEmitter(opts) { return; } - if (['gzip', 'deflate'].indexOf(res.headers['content-encoding']) !== -1) { - var unzip = zlib.createUnzip(); - unzip.httpVersion = res.httpVersion; - unzip.headers = res.headers; - unzip.rawHeaders = res.rawHeaders; - unzip.trailers = res.trailers; - unzip.rawTrailers = res.rawTrailers; - unzip.setTimeout = res.setTimeout.bind(res); - unzip.statusCode = res.statusCode; - unzip.statusMessage = res.statusMessage; - unzip.socket = res.socket; - res = res.pipe(unzip); - } + res = unzipResponse(res); ee.emit('response', res); }).once('error', function (err) { diff --git a/package.json b/package.json index 7570a971a..78fbcef48 100644 --- a/package.json +++ b/package.json @@ -50,7 +50,8 @@ "pinkie-promise": "^1.0.0", "prepend-http": "^1.0.0", "read-all-stream": "^3.0.0", - "timed-out": "^2.0.0" + "timed-out": "^2.0.0", + "unzip-response": "^1.0.0" }, "devDependencies": { "from2-array": "0.0.3", From 8768176829801b93468a16e69be2eb119159f586 Mon Sep 17 00:00:00 2001 From: Vsevolod Strukchinsky Date: Thu, 23 Jul 2015 13:02:04 +0500 Subject: [PATCH 7/7] Small simplifications --- index.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/index.js b/index.js index 150cd9092..fb44c80b2 100644 --- a/index.js +++ b/index.js @@ -31,7 +31,7 @@ function requestAsEventEmitter(opts) { var ee = new EventEmitter(); var redirectCount = 0; - function get(opts) { + var get = function (opts) { var fn = opts.protocol === 'https:' ? https : http; var url = urlLib.format(opts); @@ -54,9 +54,7 @@ function requestAsEventEmitter(opts) { return; } - res = unzipResponse(res); - - ee.emit('response', res); + ee.emit('response', unzipResponse(res)); }).once('error', function (err) { ee.emit('error', new GotError('Request to ' + url + ' failed', err)); }); @@ -66,7 +64,7 @@ function requestAsEventEmitter(opts) { } setImmediate(ee.emit.bind(ee), 'request', req); - } + }; get(opts); return ee;