Skip to content
This repository has been archived by the owner on Aug 11, 2021. It is now read-only.

remove socket error handler after request is done #142

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ node_js:
- "0.10"
- "0.8"
- iojs
- "4"
- "6"
- stable
script: "npm test"
sudo: false
before_install:
Expand Down
18 changes: 16 additions & 2 deletions lib/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,16 @@ function regRequest (uri, params, cb_) {
}

function makeRequest (uri, params, cb_) {
var cb = once(cb_)
var socket
var cb = once(function (er, parsed, raw, response) {
if (socket) {
// The socket might be returned to a pool for re-use, so don’t keep
// the 'error' listener from here attached.
socket.removeListener('error', cb)
}

return cb_(er, parsed, raw, response)
})

var parsed = url.parse(uri)
var headers = {}
Expand Down Expand Up @@ -149,8 +158,13 @@ function makeRequest (uri, params, cb_) {
var req = request(opts, decodeResponseBody(done))

req.on('error', cb)

// This should not be necessary, as the HTTP implementation in Node
// passes errors occurring on the socket to the request itself. Being overly
// cautious comes at a low cost, though.
req.on('socket', function (s) {
s.on('error', cb)
socket = s
socket.on('error', cb)
})

if (params.body && (params.body instanceof Stream)) {
Expand Down
8 changes: 8 additions & 0 deletions test/lib/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@ if (!global.setImmediate || !require('timers').setImmediate) {
}
}

// See https://github.com/npm/npm-registry-client/pull/142 for background.
// Note: `process.on('warning')` only works with Node >= 6.
process.on('warning', function (warning) {
if (/Possible EventEmitter memory leak detected/.test(warning.message)) {
throw new Error('There should not be any EventEmitter memory leaks')
}
})

module.exports = {
port: server.port,
registry: REGISTRY,
Expand Down