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

Connection keep alive behavior. #271

Open
Borewit opened this issue Jan 28, 2019 · 12 comments
Open

Connection keep alive behavior. #271

Borewit opened this issue Jan 28, 2019 · 12 comments

Comments

@Borewit
Copy link

Borewit commented Jan 28, 2019

Regarding the 'Connection' header

Unless you're running an old version of Node (< 0.11.4), by default Needle won't set the Connection header on requests, yielding Node's default behaviour of keeping the connection alive with the target server. This speeds up inmensely the process of sending several requests to the same host.

On older versions, however, this has the unwanted behaviour of preventing the runtime from exiting, either because of a bug or 'feature' that was changed on 0.11.4. To overcome this Needle does set the 'Connection' header to 'close' on those versions, however this also means that making new requests to the same host doesn't benefit from Keep-Alive.

So if you're stuck on 0.10 or even lower and want full speed, you can simply set the Connection header to 'Keep-Alive' by using { connection: 'Keep-Alive' }. Please note, though, that an event loop handler will prevent the runtime from exiting so you'll need to manually call process.exit() or the universe will collapse.

Tomas, I don't think that information is accurate.
To me it looks like the default behavior of Node (tested with v10.13.0, seen it with older versions as well) is not to enable keep-alive (sending Connection: close):

GET /ws/2/artist/?query=Stromae&offset=&limit=&fmt=json HTTP/1.1
accept: */*
user-agent: Needle/2.2.4 (Node.js v10.13.0; win32 x64)
host: test.musicbrainz.org
Connection: close

Tested both with & without HTTP-proxy assigned, consistent behavior.

The only way I found to keep the connection alive is by assigning an Http(s)Agent:

const httpAgent = new http.Agent({keepAlive: true});
needle('get', 'http://something', null, {
        user_agent: httpAgent
      });

const httpsAgent = new https.Agent({keepAlive: true});
needle('get', 'https://something', null, {
        user_agent: httpsAgent
      });

This doesn't seem to work in combination with an http-proxy.
Axios suffers from the same limitation: axios/axios#1981

@tomas
Copy link
Owner

tomas commented Jan 30, 2019

@Borewit That's odd. I'll take a look.

@tomas
Copy link
Owner

tomas commented Jan 30, 2019

Hmm, looks like you're right. I was looking at the test suite regarding the 'Connection' header and found this.

It's been a while since I looked at the keep-alive behaviour, but the behaviour in general seems a bit confusing. Check out this thread.

I guess we'll have to update the docs. Do you have any suggestions regarding our implementation?

@Borewit
Copy link
Author

Borewit commented Feb 4, 2019

Not directly Tomás,
So far I only managed with request to esteblish and keep up a cookie based authentication session (where the cookie handling requires customization).
I suspect this has to do with the keep alive behaviour.

But I keep it in mind, if I have time...

@smartpunter
Copy link

Unfortunately, needle still doesn't support keep-alive connections under the hood.
I guess, the flag 'keepAlive': true, should be set in needle.defaults, as well as in per-request options so the connection would be reused by next requests.

@tomas
Copy link
Owner

tomas commented May 1, 2019 via email

@rafiqlightwala
Copy link

rafiqlightwala commented Apr 22, 2020

Has this been resolved?
Edit: If I use "Connection":"Keep-Alive" in the headers will it work?

@Xubor
Copy link

Xubor commented May 24, 2020

I can confirm that keep-alive is working in https (not tested with plain http). You must create agent with keepAlive set to true, see https://nodejs.org/api/http.html#http_new_agent_options and then use it as 'agent' option in all requests.

Example:

const agent = new require('https').Agent({ keepAlive: true });
...
needle('get',req,{ agent: agent }) . then...

@tomas probably it will be nice to clarify this in documentation.

@tomas
Copy link
Owner

tomas commented May 24, 2020

Good point. Would you submit a PR? :)

@bastoune
Copy link

Hi all,
Does somebody have any news about this bug/feature ?
I am facing an issue with requests on a loop opening too many connections to a another server.

It would be nice if needle behaviour could be the same as the Node's default behaviour.

@Al-Kostryukov
Copy link
Contributor

Been using needle for 2 years and thought connection is kept alive by default. Until today when my colleague found that connection closes. I was referencing "Regarding the 'Connection' header" part in docs, but was confused. Found this issue and created PR to clarify this behaviour. #378

@tomas
Copy link
Owner

tomas commented Oct 2, 2021

Thanks for the PR @Al-Kostryukov. I'll see if I can simplify the use of keep-alive without having to initialize a http agent.

@tomas
Copy link
Owner

tomas commented Oct 2, 2021

Ok so after diving a bit deeper I think I found out what is going on.

Turns out that if you pass one or more of the TLS options (e.g. key, cert, rejectUnauthorized, etc) and do not pass a custom Agent, then Needle will set agent: false when calling http/https.request(), in order for Node to initialize a new, one-time Agent with the given options (apparently this was necessary for TLS options to work when I wrote that part of the code).

Using a one-time Agent means no connection pooling is done, which is the default behaviour of Node and Needle when using keep-alive (which is the default unless you set the connection header to close).

I just pushed a new branch that removes the agent: false logic and also allows setting a custom, global agent via defaults({ agent: myCustomAgent }). So keep-alive should now work out of the box either with or without custom TLS options, and there's also an easier way to reuse your custom agent between Needle calls if needed.

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

7 participants