Skip to content
GitLab
Projects Groups Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in / Register
  • N node-http-proxy
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 482
    • Issues 482
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 102
    • Merge requests 102
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Packages and registries
    • Packages and registries
    • Package Registry
    • Infrastructure Registry
  • Monitor
    • Monitor
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • http ... PARTY!
  • node-http-proxy
  • Merge requests
  • !488

Don't send requests with Connection:keep-alive if we have no agent.

  • Review changes

  • Download
  • Email patches
  • Plain diff
Closed Administrator requested to merge github/fork/glasser/keep-alive-requires-agent into caronte Sep 24, 2013
  • Overview 29
  • Commits 1
  • Pipelines 0
  • Changes 1

Created by: glasser

Avoids unhandleable ECONNRESETs.

I think this is arguably a Node http bug (in 0.10.18 at least), but: if you run

http.request({headers: {connection: 'keep-alive'}, agent: false});

During the ClientRequest constructor, self.shouldKeepAlive is set to false because there is no agent. But then it calls (indirectly) the storeHeader function (in http.js, which sets self.shouldKeepAlive to true because you specified the keep-alive header.

Then once the request is over responseOnEnd (in http.js) runs. Because shouldKeepAlive is true, we do NOT destroy the underlying socket. But we do remove its error listener. However, because we do NOT have an agent, nobody cares that we emit free, and the socket is essentially leaked.

That is, we continue to have an open socket to the target server, which has no error listener and which will never be cleaned up.

It's bad enough that this is a resource leak. But to make matters worse: let's say that the target server dies. This socket will emit an ECONNRESET error... and since there is no error listener on the socket (there's a listener on the ClientRequest! but not on the socket), bam, time for an incredibly confusing error to be thrown from the top level, probably crashing your process or triggering uncaughtException.

I think the best workaround here is to ensure that if we have no agent, then we don't send connection: keep-alive. This PR is one implementation of this.

(I'll work around this a different way in Meteor for now: by providing an explicit Agent.)

Assignee
Assign to
Reviewers
Request review from
Time tracking
Source branch: github/fork/glasser/keep-alive-requires-agent