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

httpclient.close() isn't really safe #164

Open
hekike opened this issue Apr 3, 2018 · 0 comments
Open

httpclient.close() isn't really safe #164

hekike opened this issue Apr 3, 2018 · 0 comments

Comments

@hekike
Copy link
Member

hekike commented Apr 3, 2018

MOVED FROM: restify/node-restify#642

HttpClient.close() reaches into the Node agent and calls end() on all of the sockets in use by the agent. It doesn't check that the socket's remote address matches the server that the client was created with. More importantly, though, the Node documentation says not to touch these sockets, and the problem with doing this is that if you call client.close() and then try to make another request (with a different client or even outside restify, AFAICT), the Node agent may decide to use one of the sockets that has started closing, and it will immediately get a "socket hang up" error.

I don't think there's a great way to implement close() given the current agent API, but what restify's doing right now makes it possible to run into bugs like node-manta#194. If the point of close() is basically for command-line programs to wrap things up, then it should probably be a global function (i.e., not per-agent), since it's closing everything, and it probably needs to be provided by the agent implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant