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

Limit maxSockets for client connections #130

Closed
wants to merge 1 commit 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
6 changes: 5 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -310,5 +310,9 @@ any):
Default = `"latest"`
* `couchToken` {Object} A token for use with
[couch-login](https://npmjs.org/package/couch-login).
* `sessionToken` {string} A random identifier for this set of client requests.
* `sessionToken` {String} A random identifier for this set of client requests.
Default = 8 random hexadecimal bytes.
* `maxSockets` {Number} The maximum number of connections that will be open per
origin (unique combination of protocol:host:port). Passed to the
[httpAgent](https://nodejs.org/api/http.html#http_agent_maxsockets).
Default = 50
1 change: 1 addition & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ function RegClient (config) {
if (typeof this.config.retry.factor !== 'number') this.config.retry.factor = 10
if (typeof this.config.retry.minTimeout !== 'number') this.config.retry.minTimeout = 10000
if (typeof this.config.retry.maxTimeout !== 'number') this.config.retry.maxTimeout = 60000
if (typeof this.config.maxSockets !== 'number') this.config.maxSockets = 50
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The number 50 is pretty arbitrary but seemed to perform okay. Maybe the default should be lower or higher though. I've tried a few different values with a big shrinkwrap install and it didn't seem to make much difference past 20.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I expect we'll want the npm side of this patch to pass through a value and to have a default there too, since this is the sort of thing where the best value is likely heavily impacted by network conditions.


this.config.userAgent = this.config.userAgent || 'node/' + process.version
this.config.defaultTag = this.config.defaultTag || 'latest'
Expand Down
33 changes: 16 additions & 17 deletions lib/initialize.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@ var HttpsAgent = require('https').Agent

var pkg = require('../package.json')

var httpAgent
var httpsAgent

module.exports = initialize

function initialize (uri, method, accept, headers) {
Expand All @@ -24,7 +21,7 @@ function initialize (uri, method, accept, headers) {
cert: this.config.ssl.certificate,
key: this.config.ssl.key,
ca: this.config.ssl.ca,
agent: getAgent(uri.protocol, this.config)
agent: getAgent.call(this, uri.protocol)
}

// allow explicit disabling of proxy in environment via CLI
Expand Down Expand Up @@ -55,28 +52,30 @@ function initialize (uri, method, accept, headers) {
return opts
}

function getAgent (protocol, config) {
function getAgent (protocol) {
if (protocol === 'https:') {
if (!httpsAgent) {
httpsAgent = new HttpsAgent({
if (!this.httpsAgent) {
this.httpsAgent = new HttpsAgent({
keepAlive: true,
localAddress: config.proxy.localAddress,
rejectUnauthorized: config.ssl.strict,
ca: config.ssl.ca,
cert: config.ssl.certificate,
key: config.ssl.key
maxSockets: this.config.maxSockets,
localAddress: this.config.proxy.localAddress,
rejectUnauthorized: this.config.ssl.strict,
ca: this.config.ssl.ca,
cert: this.config.ssl.certificate,
key: this.config.ssl.key
})
}

return httpsAgent
return this.httpsAgent
} else {
if (!httpAgent) {
httpAgent = new HttpAgent({
if (!this.httpAgent) {
this.httpAgent = new HttpAgent({
keepAlive: true,
localAddress: config.proxy.localAddress
maxSockets: this.config.maxSockets,
localAddress: this.config.proxy.localAddress
})
}

return httpAgent
return this.httpAgent
}
}
34 changes: 34 additions & 0 deletions test/initialize.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
var test = require('tap').test
var url = require('url')

// var server = require('./lib/server.js')
var Client = require('../')
Expand Down Expand Up @@ -29,6 +30,39 @@ test('defaulted initialization', function (t) {

var HttpAgent = require('http').Agent
t.ok(options.agent instanceof HttpAgent, 'got an HTTP agent for an HTTP URL')
t.equal(options.agent.maxSockets, 50, 'maxSockets set to a reasonable default')

t.end()
})

test('intializing with maxSockets set works for http', function (t) {
var client = new Client({ maxSockets: Infinity })
var options = client.initialize(
url.parse('http://localhost:1337/'),
'GET',
'application/json',
{}
)

var HttpAgent = require('http').Agent
t.ok(options.agent instanceof HttpAgent, 'got an HTTP agent for an HTTP URL')
t.equal(options.agent.maxSockets, Infinity, 'request uses configured value for maxSockets')

t.end()
})

test('intializing with maxSockets set works for https', function (t) {
var client = new Client({ maxSockets: Infinity })
var options = client.initialize(
url.parse('https://localhost:1337/'),
'GET',
'application/json',
{}
)

var HttpsAgent = require('https').Agent
t.ok(options.agent instanceof HttpsAgent, 'got an HTTPS agent for an HTTPS URL')
t.equal(options.agent.maxSockets, Infinity, 'request uses configured value for maxSockets')

t.end()
})
Expand Down