diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index 61456bb4..f973cea7 100644 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -35,7 +35,7 @@ jobs: node-version: ${{ matrix.node-version }} - name: Install Dependencies - run: npm i -g npminstall && npminstall + run: npm i -g npminstall@latest-5 && npminstall - name: Continuous Integration run: npm run ci diff --git a/lib/urllib.js b/lib/urllib.js index 4d429515..858496cc 100644 --- a/lib/urllib.js +++ b/lib/urllib.js @@ -728,6 +728,13 @@ function requestWithCallback(url, args, callback) { } function decodeContent(res, body, cb) { + if (responseAborted) { + // err = new Error('Remote socket was terminated before `response.end()` was called'); + // err.name = 'RemoteSocketClosedError'; + debug('Request#%d %s: Remote socket was terminated before `response.end()` was called', reqId, url); + var err = responseError || new Error('Remote socket was terminated before `response.end()` was called'); + return cb(err); + } var encoding = res.headers['content-encoding']; if (body.length === 0 || !encoding) { return cb(null, body, encoding); @@ -758,7 +765,10 @@ function requestWithCallback(url, args, callback) { args.requestUrls.push(parsedUrl.href); + var hasResponse = false; + var responseError; function onResponse(res) { + hasResponse = true; socketHandledResponses = res.socket[SOCKET_RESPONSE_COUNT] = (res.socket[SOCKET_RESPONSE_COUNT] || 0) + 1; if (timing) { timing.waiting = Date.now() - requestStartTime; @@ -781,7 +791,8 @@ function requestWithCallback(url, args, callback) { return done(null, null, res); } - res.on('error', function () { + res.on('error', function (err) { + responseError = err; debug('Request#%d %s: `res error` event emit, total size %d, socket handled %s requests and %s responses', reqId, url, responseSize, socketHandledRequests, socketHandledResponses); }); @@ -939,12 +950,6 @@ function requestWithCallback(url, args, callback) { } } - if (responseAborted) { - // err = new Error('Remote socket was terminated before `response.end()` was called'); - // err.name = 'RemoteSocketClosedError'; - debug('Request#%d %s: Remote socket was terminated before `response.end()` was called', reqId, url); - } - done(err, data, res); }); } @@ -1161,12 +1166,17 @@ function requestWithCallback(url, args, callback) { }); } - var isRequestError = false; + var isRequestDone = false; function handleRequestError(err) { - if (isRequestError || !err) { + if (!err) { return; } - isRequestError = true; + // only ignore request error if response has been received + // if response has not received, socket error will emit on req + if (isRequestDone && hasResponse) { + return; + } + isRequestDone = true; if (err.name === 'Error') { err.name = connected ? 'ResponseError' : 'RequestError'; @@ -1178,7 +1188,9 @@ function requestWithCallback(url, args, callback) { debug('Request#%d pump args.stream to req', reqId); pump(args.stream, req, handleRequestError); } else { - req.end(body); + req.end(body, function () { + isRequestDone = true; + }); } // when stream already consumed, req's `finish` event is emitted and pump will ignore error after pipe finished // but if server response timeout later, we will abort the request and emit an error in req diff --git a/test/proxy.test.js b/test/proxy.test.js index d8b9af14..5b061782 100644 --- a/test/proxy.test.js +++ b/test/proxy.test.js @@ -6,7 +6,7 @@ var proxy = require('./fixtures/reverse-proxy'); var isNode010 = /^v0\.10\.\d+$/.test(process.version); var isNode012 = /^v0\.12\.\d+$/.test(process.version); -var testUrl = process.env.CI ? 'https://registry.npmjs.com' : 'https://registry.npm.taobao.org'; +var testUrl = process.env.CI ? 'https://registry.npmjs.com' : 'https://registry.npmmirror.com'; if (!isNode010 && !isNode012) { describe('test/proxy.test.js', function() { @@ -25,15 +25,12 @@ if (!isNode010 && !isNode012) { }); it('should proxy http work', function(done) { - urllib.request('http://registry.npm.taobao.org/pedding/1.0.0', { - dataType: 'json', + urllib.request('http://registry.npmmirror.com/pedding/1.0.0', { enableProxy: true, proxy: proxyUrl, }, function(err, data, res) { assert(!err); - console.log(res.headers); - assert(data.name === 'pedding'); - assert(res.status === 200); + assert(res.status === 301); done(); }); }); diff --git a/test/urllib.test.js b/test/urllib.test.js index e545f51f..5e214c59 100644 --- a/test/urllib.test.js +++ b/test/urllib.test.js @@ -411,11 +411,8 @@ describe('test/urllib.test.js', function () { it('should handle server socket end("balabal") will error', function (done) { urllib.request(host + '/socket.end.error', function (err, data) { assert(err); - assert(err.name === 'ResponseError'); - err.code && assert(err.code === 'HPE_INVALID_CHUNK_SIZE'); - assert(/Parse Error.*GET http:\/\/127\.0\.0\.1:/.test(err.message) >= 0); - assert(err.bytesParsed === 2); - assert(!data); + err.code && assert(err.code === 'ECONNRESET'); + assert(data); done(); }); }); @@ -980,6 +977,23 @@ describe('test/urllib.test.js', function () { }); }); }); + + it('should not emit request error if request is done', function (done) { + var req = urllib.request(host + '/stream', { + method: 'post', + data: {foo: 'bar'}, + ctx: { + foo: 'stream request' + } + }, function (err) { + assert(!err); + done(); + }); + + req.on('response', () => { + req.emit('error', new Error('mock error')); + }); + }); }); describe('support stream', function () {