http: move free socket error handling to agent

The http client should not know anything about free sockets. Let
the agent handle its pool of sockets.

PR-URL: https://github.com/nodejs/node/pull/32003
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This commit is contained in:
Robert Nagy
2020-02-28 12:29:45 +01:00
committed by Anna Henningsen
parent bffc9324a1
commit 2bf02285a3
2 changed files with 21 additions and 10 deletions

View File

@@ -57,6 +57,13 @@ class ReusedHandle {
}
}
function freeSocketErrorListener(err) {
const socket = this;
debug('SOCKET ERROR on FREE socket:', err.message, err.stack);
socket.destroy();
socket.emit('agentRemove');
}
function Agent(options) {
if (!(this instanceof Agent))
return new Agent(options);
@@ -82,6 +89,11 @@ function Agent(options) {
const name = this.getName(options);
debug('agent.on(free)', name);
// TODO(ronag): socket.destroy(err) might have been called
// before coming here and have an 'error' scheduled. In the
// case of socket.destroy() below this 'error' has no handler
// and could cause unhandled exception.
if (socket.writable &&
this.requests[name] && this.requests[name].length) {
const req = this.requests[name].shift();
@@ -118,6 +130,7 @@ function Agent(options) {
socket.setTimeout(agentTimeout);
}
socket.once('error', freeSocketErrorListener);
freeSockets.push(socket);
} else {
// Implementation doesn't want to keep socket alive
@@ -393,6 +406,7 @@ Agent.prototype.keepSocketAlive = function keepSocketAlive(socket) {
Agent.prototype.reuseSocket = function reuseSocket(socket, req) {
debug('have free socket');
socket.removeListener('error', freeSocketErrorListener);
req.reusedSocket = true;
socket.ref();
};

View File

@@ -473,13 +473,6 @@ function socketErrorListener(err) {
socket.destroy();
}
function freeSocketErrorListener(err) {
const socket = this;
debug('SOCKET ERROR on FREE socket:', err.message, err.stack);
socket.destroy();
socket.emit('agentRemove');
}
function socketOnEnd() {
const socket = this;
const req = this._httpMessage;
@@ -658,8 +651,11 @@ function responseKeepAlive(req) {
socket.removeListener('error', socketErrorListener);
socket.removeListener('data', socketOnData);
socket.removeListener('end', socketOnEnd);
socket.once('error', freeSocketErrorListener);
// There are cases where _handle === null. Avoid those. Passing null to
// TODO(ronag): Between here and emitFreeNT the socket
// has no 'error' handler.
// There are cases where _handle === null. Avoid those. Passing undefined to
// nextTick() will call getDefaultTriggerAsyncId() to retrieve the id.
const asyncId = socket._handle ? socket._handle.getAsyncId() : undefined;
// Mark this socket as available, AFTER user-added end
@@ -741,7 +737,6 @@ function tickOnSocket(req, socket) {
}
parser.onIncoming = parserOnIncomingClient;
socket.removeListener('error', freeSocketErrorListener);
socket.on('error', socketErrorListener);
socket.on('data', socketOnData);
socket.on('end', socketOnEnd);
@@ -781,6 +776,8 @@ function listenSocketTimeout(req) {
}
ClientRequest.prototype.onSocket = function onSocket(socket) {
// TODO(ronag): Between here and onSocketNT the socket
// has no 'error' handler.
process.nextTick(onSocketNT, this, socket);
};