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:

fetch-h2/lib/context-http1.ts

Line 341 in 1aed16d

agent: false,

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ä ***@***.***> 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 <#33 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AG3lJb7DRAAZKmJ6dNttMsNjfwhJcGOjks5vRkMegaJpZM4bOu7J> .

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

[grantila]

🎉 This issue has been resolved in version 2.2.0 🎉

The release is available on:

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

Your semantic-release bot 📦🚀