http1 requests do not reuse sockets due to Connection: close header

[pietermees]

Node automatically sends Connection: close when not using an Agent: https://github.com/nodejs/node/blob/master/lib/_http_client.js#L262

fetch-h2 disables the agent on http1 requests: https://github.com/grantila/fetch-h2/blob/1aed16d06bfcb9c1a4c2b6498a5a964d593fc19f/lib/context-http1.ts#L341

This means that http1 connections effectively are not reused at this time and every subsequent request creates a new Socket.

[grantila]

Right, I tried to build a test for this, but the default http server in Node.js doesn't close upon this header. This should be overrideable by the context, i.e. if you specify in the context options:

{
    http1: {
        keepAlive: true
    }
}

It currently defaults to false, I'm thinking of changing this to true, as that would likely make more sense.

[grantila]

I think that would fix it, but I'm unsure exactly how Node.js handles re-using connections or if fetch-h2 would have to explicitly have to set the Connection header perhaps, which would be sad.

[pietermees]

Unfortunately, I’ve already set that option and it does not affect the behavior. Node simply checks for the agent (see references code) and if it’s disabled it automatically injects a connection close header.

I can take a look over the weekend at creating a test case for this as a starting point? On Wed, Feb 27, 2019 at 3:34 AM Gustaf Räntilä notifications@github.com wrote:

I think that would fix it, but I'm unsure exactly how Node.js handles re-using connections or if fetch-h2 would have to explicitly have to set the Connection header perhaps, which would be sad.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/grantila/fetch-h2/issues/33#issuecomment-467771501, or mute the thread https://github.com/notifications/unsubscribe-auth/AG3lJb7DRAAZKmJ6dNttMsNjfwhJcGOjks5vRkMegaJpZM4bOu7J .

-- We're hiring! Join http://bit.ly/2Bd80Y4 the Zentrick family.

[grantila]

:tada: This issue has been resolved in version 2.2.0 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket: