octokit / octokit.js

The all-batteries-included GitHub SDK for Browsers, Node.js, and Deno.
MIT License
7.03k stars 1.03k forks source link

End or destroy socket on timeout #292

Closed alanshaw closed 8 years ago

alanshaw commented 9 years ago

When a request times out, the callback is called with an appropriate error (https://github.com/mikedeboer/node-github/blob/bae796eba78ab1500ceeec22ca084c8572da0724/index.js#L812-L816).

However, the socket remains open:

The user must manually end() or destroy() the socket https://nodejs.org/api/net.html#net_socket_settimeout_timeout_callback

In a test I ran the socket eventually hung up after 2 minutes (you see the debug error message in the console if you wait). We should probably free up the socket as soon as possible. I vote we add req.abort() to that timeout callback, which should safely destroy the socket immediately.

Test code:

var GitHubApi = require('github')

var server = require('http').createServer(() => {
  // Just...hang
})

var port = 3000

server.listen(port, () => {
  console.log(server.address())

  var github = new GitHubApi({
    version: '3.0.0',
    debug: true,
    protocol: 'http',
    host: 'localhost',
    port: port,
    pathPrefix: '/api/v3',
    timeout: 2000
  })

  github.user.getFollowingFromUser({user: "foo"}, console.log)
})
maxime-helen-sb commented 8 years ago

Same issue on my side with similar tests.