diff --git a/lib/fetch.js b/lib/fetch.js index e931d37..ff3ee21 100644 --- a/lib/fetch.js +++ b/lib/fetch.js @@ -18,16 +18,19 @@ function fetch (uri, params, cb) { makeRequest.call(client, uri, params, function (er, req) { if (er) return cb(er) - req.on('error', function (er) { + req.once('error', retryOnError) + + function retryOnError (er) { if (operation.retry(er)) { client.log.info('retry', 'will retry, error on last attempt: ' + er) } else { cb(er) } - }) + } req.on('response', function (res) { client.log.http('fetch', '' + res.statusCode, uri) + req.removeListener('error', retryOnError) var er var statusCode = res && res.statusCode @@ -37,6 +40,10 @@ function fetch (uri, params, cb) { res.resume() if (process.version === 'v0.10.0') unstick(res) + req.once('error', function (er) { + res.emit('error', er) + }) + return cb(null, res) // Only retry on 408, 5xx or no `response`. } else if (statusCode === 408) { diff --git a/package.json b/package.json index 92bbf10..6ac51cf 100644 --- a/package.json +++ b/package.json @@ -25,6 +25,7 @@ "negotiator": "^0.6.1", "nock": "^8.0.0", "readable-stream": "^2.1.5", + "require-inject": "^1.4.0", "rimraf": "^2.5.4", "standard": "~8.5.0", "tap": "^7.0.0" diff --git a/test/econnreset.js b/test/econnreset.js new file mode 100644 index 0000000..ee06571 --- /dev/null +++ b/test/econnreset.js @@ -0,0 +1,95 @@ +'use strict' +var requireInject = require('require-inject') +var test = require('tap').test +var EventEmitter = require('events').EventEmitter +var PassThrough = require('readable-stream').PassThrough + +var content = [ + 'first chunk', + 'second chunk' +] +var requests = 0 +var fetch = requireInject('../lib/fetch.js', { + request: function (opts) { + var req = new EventEmitter() + ++requests + setTimeout(function () { + var res = new PassThrough() + res.statusCode = 200 + + setTimeout(function () { + res.write(content[0]) + }, 50) + + if (requests === 1) { + setTimeout(function () { + var err = new Error('read ECONNRESET') + err.code = 'ECONNRESET' + req.emit('error', err) + }, 100) + } else { + // we allow success on retries, though in practice we won't be + // retrying. + setTimeout(function () { + res.end(content[1]) + }, 100) + } + req.emit('response', res) + }, 50) + return req + } +}) + +function clientMock (t) { + return { + log: { + info: function (section, message) { + t.comment('[info] ' + section + ': ' + [].slice.call(arguments, 1).join(' ')) + }, + http: function (section, message) { + t.comment('[http] ' + section + ': ' + [].slice.call(arguments, 1).join(' ')) + } + }, + authify: function (alwaysAuth, parsed, headers, auth) { + return + }, + initialize: function (parsed, method, accept, headers) { + return {} + }, + attempt: require('../lib/attempt.js'), + config: { + retry: { + retries: 2, + factor: 10, + minTimeout: 10000, + maxTimeout: 60000 + } + } + } +} + +/* +This tests that errors that occur in the REQUEST object AFTER a `response` +event has been emitted will be passed through to the `response` stream. +This means that we won't try to retry these sorts of errors ourselves. +*/ + +test('econnreset', function (t) { + var client = clientMock(t) + fetch.call(client, 'http://example.com/', {}, function (err, res) { + t.ifError(err, 'initial fetch ok') + var data = '' + res.on('data', function (chunk) { + data += chunk + }) + res.on('error', function (err) { + t.comment('ERROR:', err.stack) + t.pass("errored and that's ok") + t.done() + }) + res.on('end', function () { + t.is(data, content.join(''), 'succeeded and got the right data') + t.done() + }) + }) +})