Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http.Agent: idle sockets throw unhandled ECONNRESET #3595

Closed
thorn0 opened this issue Oct 29, 2015 · 29 comments
Closed

http.Agent: idle sockets throw unhandled ECONNRESET #3595

thorn0 opened this issue Oct 29, 2015 · 29 comments
Labels
http Issues or PRs related to the http subsystem.

Comments

@thorn0
Copy link

thorn0 commented Oct 29, 2015

Please see this code: https://gist.github.com/thorn0/0812cdb7eb12b2348337
That's what it outputs for me (Node 4.2.1 and 5.0.0, Windows 7 x64):

1.108s: Complete
121.193s: uncaughtException
Error: read ECONNRESET
    at exports._errnoException (util.js:874:11)
    at TCP.onread (net.js:544:26)

It's reproduced with most servers that run Microsoft IIS.

@mscdex mscdex added the http Issues or PRs related to the http subsystem. label Oct 30, 2015
@bnoordhuis
Copy link
Member

Reproducing your code here for the sake of posterity:

var http = require("http");
var domain = require("domain");

function log() {
  var args = [].slice.call(arguments);
  args.unshift(process.uptime()+"s:");
  console.log.apply(console, args);
}

var agent = new http.Agent({
  keepAlive: true,
  keepAliveMsecs: 10000
});

var req = http.request({
  hostname: "www.asp.net",
  port: 80,
  path: "/",
  method: "GET",
  agent: agent
}, function(res) {
  res.on("data", function() {
  });
  res.on("end", function() {
    log("Complete");
  });
});

req.on("socket", function(sock) {
  sock.on("close", function() {
    log("socket closed");
  });
});

req.on("error", function(err) {
  log("an error ocurred", err);
});

process.on('uncaughtException', function(err) {
  log("uncaughtException");
  console.error(err.stack);
  process.exit();
});
req.end();

setTimeout(function() {
  log("done");
}, 300000);

I'm not sure what you think keepAlive and keepAliveMsecs do but they enable TCP (not HTTP) keep-alive on your side. It's completely transparent to the remote end (the IIS server). Most HTTP servers will disconnect after a period of inactivity, hence the ECONNRESET.

I don't see a bug here. I'll close the issue.

@thorn0
Copy link
Author

thorn0 commented Oct 30, 2015

I'm really confused. https://nodejs.org/api/http.html states the opposite:

If you opt into using HTTP KeepAlive, you can create an Agent object with that flag set to true.

@bnoordhuis
Copy link
Member

Maybe I should have said: keepAlive in conjunction with keepAliveMsecs. The latter sets the TCP keep-alive interval, the former tells node to reuse the socket when it can. The point is though that regardless of using TCP or HTTP keep-alive on your end, the remote end can still elect to close the connection.

If you think the documentation is unclear, can you file an issue for that and suggest improvements or send a pull request?

@thorn0
Copy link
Author

thorn0 commented Oct 30, 2015

I still can't get it. http.Agent encapsulates all the other parts of the socket pool management logic. Why doesn't it catch this ECONNRESET at that?

@bnoordhuis
Copy link
Member

Because ECONNRESET is an exceptional error, it means the remote end forcibly closed the connection. If the agent swallowed the error you couldn't distinguish an empty reply from unclean termination, for example.

@thorn0
Copy link
Author

thorn0 commented Oct 30, 2015

An empty reply to what? The discussed exception happens when the socket is waiting in the pool and isn't used for a request.

@bnoordhuis
Copy link
Member

With all due respect, that's not very clear from your original message. I'll reopen the issue, pull requests welcome.

@bnoordhuis bnoordhuis reopened this Oct 30, 2015
@bnoordhuis
Copy link
Member

For whoever is picking this up, it's probably worth mentioning that the naive solution of attaching an error listener in the agent's 'free' event listener introduces a race window in lib/_http_client.js between the removal of the socket's 'error' event listener in responseOnEnd() and returning it to the pool in emitFreeNT().

@thorn0 thorn0 changed the title http.request: keep-alive leads to unhandled ECONNRESET http.Agent: idle sockets throw unhandled ECONNRESET Oct 30, 2015
@livesankp
Copy link

Hello,

I am facing the same issue in signalr which uses long polling and my program stops in 2 mins. This was working fine before node.js upgrade from 0.12.6 to 4.1.2

@jfromaniello
Copy link
Contributor

I think it will be nice if the Http Agent had a socket event for every socket created. If I understand correctly the issue, the server can close the connection at any time, being the socket free or not.

@thorn0
Copy link
Author

thorn0 commented Dec 22, 2015

No, this issue is only about free (idle) sockets.

jfromaniello added a commit to jfromaniello/node that referenced this issue Jan 4, 2016
This change adds a new event handler to the `error` event of the socket
after it has been used by the http_client.

The purpose of this change is to catch errors on *keep alived*
connections from idle sockets that otherwise will cause an uncaugh error
event on the application.

Closes nodejs#3595
@indutny indutny closed this as completed in 5a2541d Jan 4, 2016
@drewfish
Copy link
Contributor

@mikemorris looks like this fix shipped with 4.4.0 (and 5.4.0).

@aymeba
Copy link

aymeba commented Mar 14, 2016

Hi,

we are also getting an ECONNRESET error. I'm not sure if this is the same bug?

  • Send a request to our backend
  • Backend processes the request
  • Request is disconnecting without waiting the response and we are getting ECONNRESET error
  • We are sure, that our backend server doesn't break the connection

Version: 4.2.2

@thorn0
Copy link
Author

thorn0 commented Mar 14, 2016

@aymeba No. This issue is only about idle sockets in the pool. It's not about sockets assigned to requests (your case)..

@aymeba
Copy link

aymeba commented Mar 14, 2016

@thorn0 thanks. Is there a known issue about requests?

@thorn0
Copy link
Author

thorn0 commented Mar 14, 2016

@aymeba Why do you think it's a bug in Node?

@aymeba
Copy link

aymeba commented Mar 14, 2016

@thorn0 I'm not sure if it is a bug, therefore I am asking whether there is a known issue about such cases? Because we setup everything in our info such ulimit etc. But it happens every 15 seconds periodic on different API calls

@thorn0
Copy link
Author

thorn0 commented Mar 14, 2016

@aymeba I don't know about such known issues. ECONNRESET is a normal error if you have network troubles.

@lukasz-zak
Copy link

Thank you @thealphanerd and whole Node team for release this fix in 4.x branch.

scovetta pushed a commit to scovetta/node that referenced this issue Apr 2, 2016
This change adds a new event handler to the `error` event of the socket
after it has been used by the http_client.

The purpose of this change is to catch errors on *keep alived*
connections from idle sockets that otherwise will cause an uncaugh error
event on the application.

Fix: nodejs#3595
PR-URL: nodejs#4482
Reviewed-By: Fedor Indutny <fedor@indutny.com>
@nodesocket
Copy link

Is this going to be ported into the 0.12.X build by chance? I think we are being bitten by this with npm. See the following issue npm/registry#10.

@bnoordhuis
Copy link
Member

@nodesocket v0.12 is in maintenance mode now, meaning that it gets security fixes but not much more. Though if you or someone else is willing to work on the backport, we'll certainly review it.

@definoob
Copy link

I've got also ECONNRESET error. But it is really weird.
It was working well. After a week, one of my client asked some update, and it doesn't work suddenly.
I didn't change the project~

It was calling google place api using axios, also I tried node-fetch as well.
Both of them give above error.
What's the issue?
Is this because of Node?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http Issues or PRs related to the http subsystem.
Projects
None yet
Development

No branches or pull requests