diff --git a/lib/_http_client.js b/lib/_http_client.js index 63a7befc8ebbb3..a80ff906f978ba 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -115,6 +115,8 @@ let debug = require('internal/util/debuglog').debuglog('http', (fn) => { const INVALID_PATH_REGEX = /[^\u0021-\u00ff]/; const kError = Symbol('kError'); +const kImmediateError = Symbol('kImmediateError'); +const kOnImmediateError = Symbol('kOnImmediateError'); const kLenientAll = HTTPParser.kLenientAll | 0; const kLenientNone = HTTPParser.kLenientNone | 0; @@ -898,6 +900,7 @@ function tickOnSocket(req, socket) { parser.joinDuplicateHeaders = req.joinDuplicateHeaders; parser.onIncoming = parserOnIncomingClient; + socket.removeListener('error', socket[kOnImmediateError]); socket.on('error', socketErrorListener); socket.on('data', socketOnData); socket.on('end', socketOnEnd); @@ -937,27 +940,36 @@ function listenSocketTimeout(req) { } ClientRequest.prototype.onSocket = function onSocket(socket, err) { - // TODO(ronag): Between here and onSocketNT the socket - // has no 'error' handler. + const req = this; + + if (!err && socket) { + socket[kOnImmediateError] = function onImmediateError(err) { + req[kImmediateError] = err; + } + socket.on('error', socket[kOnImmediateError]); + } + process.nextTick(onSocketNT, this, socket, err); }; function onSocketNT(req, socket, err) { - if (req.destroyed || err) { + if (req.destroyed || err || req[kImmediateError]) { req.destroyed = true; function _destroy(req, err) { if (!req.aborted && !err) { err = new ConnResetException('socket hang up'); } - if (err) { - emitErrorEvent(req, err); + if (err || req[kImmediateError] instanceof Error) { + const finalError = req[kImmediateError] ? req[kImmediateError] : err; + emitErrorEvent(req, finalError); } req._closed = true; req.emit('close'); } if (socket) { + socket.removeListener('error', socket[kOnImmediateError]); if (!err && req.agent && !socket.destroyed) { socket.emit('free'); } else { diff --git a/test/parallel/test-http-client-immediate-error.js b/test/parallel/test-http-client-immediate-error.js index ff29ad72bd8585..e00db37a2182ee 100644 --- a/test/parallel/test-http-client-immediate-error.js +++ b/test/parallel/test-http-client-immediate-error.js @@ -5,40 +5,116 @@ // net.createConnection(). const common = require('../common'); +console.log('require completed: common', Date.now()); const assert = require('assert'); +console.log('require completed: assert', Date.now()); const net = require('net'); +console.log('require completed: net', Date.now()); const http = require('http'); +console.log('require completed: http', Date.now()); const { internalBinding } = require('internal/test/binding'); -const { UV_ENETUNREACH } = internalBinding('uv'); +console.log('require completed: internal/test/binding', Date.now()); +const { UV_ENETUNREACH, UV_EADDRINUSE } = internalBinding('uv'); const { newAsyncId, symbols: { async_id_symbol } } = require('internal/async_hooks'); +console.log('require completed: internal/async_hooks', Date.now()); -const agent = new http.Agent(); -agent.createConnection = common.mustCall((cfg) => { - const sock = new net.Socket(); +const config = { + host: 'http://example.com', + // We need to specify 'family' in both items of the array so 'internalConnectMultiple' is invoked + connectMultiple: [{ address: '192.4.4.4', family: 4 }, { address: '200::1', family: 6 }], + connectSolo: { address: '192.4.4.4', family: 4 }, +}; - // Fake the handle so we can enforce returning an immediate error - sock._handle = { - connect: common.mustCall((req, addr, port) => { - return UV_ENETUNREACH; - }), - readStart() {}, - close() {} - }; +function agentFactory(handle, count) { + const agent = new http.Agent(); + agent.createConnection = common.mustCall((...cfg) => { + const normalized = net._normalizeArgs(cfg); + const sock = new net.Socket(); - // Simulate just enough socket handle initialization - sock[async_id_symbol] = newAsyncId(); + // Fake the handle so we can enforce returning an immediate error + sock._handle = handle; - sock.connect(cfg); - return sock; + // Simulate just enough socket handle initialization + sock[async_id_symbol] = newAsyncId(); + + sock.connect(normalized); + return sock; + }, count); + + return agent; +}; + +const handleWithoutBind = (callCount) => ({ + connect: common.mustCall((req, addr, port) => { + return UV_ENETUNREACH; + }, callCount), + readStart() { }, + close() { }, +}); + +const handleWithBind = (callCount) => ({ + readStart() { }, + close() { }, + bind: common.mustCall(() => UV_EADDRINUSE, callCount), }); +const agentWithoutBind = agentFactory(handleWithoutBind(3), 3); // Because three tests will use this +const agentWithBind = agentFactory(handleWithBind(2), 2); // Because two tests will use this handle + +console.log('test initiated: test-1', Date.now()); http.get({ host: '127.0.0.1', port: 1, - agent + agent: agentWithoutBind, +}).on('error', common.mustCall((err) => { + assert.strictEqual(err.code, 'ENETUNREACH'); + console.log('test completed: test-1', Date.now()); +})); + +console.log('test initiated: test-2', Date.now()); +http.request(config.host, { + agent: agentWithoutBind, + lookup(_0, _1, cb) { + cb(null, config.connectMultiple); + }, }).on('error', common.mustCall((err) => { assert.strictEqual(err.code, 'ENETUNREACH'); + console.log('test completed: test-2', Date.now()); +})); + +console.log('test initiated: test-3', Date.now()); +http.request(config.host, { + agent: agentWithoutBind, + lookup(_0, _1, cb) { + cb(null, config.connectSolo.address, config.connectSolo.family); + }, + family: 4, +}).on('error', common.mustCall((err) => { + assert.strictEqual(err.code, 'ENETUNREACH'); + console.log('test completed: test-3', Date.now()); +})); + +console.log('test initiated: test-4', Date.now()); +http.request(config.host, { + agent: agentWithBind, + family: 4, // We specify 'family' so 'internalConnect' is invoked + localPort: 2222 // Required to trigger _handle.bind() +}).on('error', common.mustCall((err) => { + assert.strictEqual(err.code, 'EADDRINUSE'); + console.log('test completed: test-4', Date.now()); +})); + +console.log('test initiated: test-5', Date.now()); +http.request(config.host, { + agent: agentWithBind, + lookup(_0, _1, cb) { + cb(null, config.connectMultiple); + }, + localPort: 2222, // Required to trigger _handle.bind() +}).on('error', common.mustCall((err) => { + assert.strictEqual(err.code, 'EADDRINUSE'); + console.log('test completed: test-5', Date.now()); }));