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

Set default keepAlive options when creating HTTP/S agents #55770

Open
MarioRomanDono opened this issue Nov 7, 2024 · 1 comment
Open

Set default keepAlive options when creating HTTP/S agents #55770

MarioRomanDono opened this issue Nov 7, 2024 · 1 comment
Labels
feature request Issues that request new features to be added to Node.js. https Issues or PRs related to the https subsystem.

Comments

@MarioRomanDono
Copy link

What is the problem this feature will solve?

Since #37184 proposal was accepted, Node's HTTP/S globalAgent sets the keepAlive option to true by default, with a socket timeout of 5 seconds. However, I find this is inconsistent with the observed behavior when using the Agent constructor, which by default returns an agent with keepAlive set to false.

What is the feature you are proposing to solve the problem?

Set keepAlive to true when creating a new HTTP/S agent, as well as setting the socket timeout to 5 seconds, to equal the behavior of the globalAgent. However, I recognize that this change could have unintended effects for users. Additionally, the impact of setting keepAlive to true in the globalAgent is still under discussion (see #47130). So, please, check my alternatives too.

What alternatives have you considered?

  1. We can keep setting keepAlive to false by default, but in case it is provided, set a default socket timeout if timeout is not provided. I ran into this problem this week: I was using keepAlive without setting a timeout value, so sockets were not being closed after inactivity. If the server sends a FIN packet and that event is not processed by the client before sending a new request, an ECONNRESET error will be thrown (as observed in ECONNRESET while doing http 1.1 keep alive requests and server closes the connections #47130). I found that setting timeout to 5 seconds, as with the globalAgent, greatly reduces the number of errors.
  2. If, for some reason, it is discouraged to set a default timeout value, at least specify in Node HTTP docs that, if it is not provided by the user, no timeout will be set (and that it could lead to possible errors as described above).

Whichever alternative is chosen, I would be more than willing to submit a PR with a solution. Thank you!

@nodejs/http (I don't know if that still works)

@MarioRomanDono MarioRomanDono added the feature request Issues that request new features to be added to Node.js. label Nov 7, 2024
@RedYetiDev RedYetiDev added the https Issues or PRs related to the https subsystem. label Nov 7, 2024
@RedYetiDev
Copy link
Member

@nodejs/http (I don't know if that still works)

For future reference, it does :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. https Issues or PRs related to the https subsystem.
Projects
Status: Awaiting Triage
Development

No branches or pull requests

2 participants