Skip to content

Commit 0993e4b

Browse files
boromispbrianc
authored andcommitted
Added the missing connect_timeout and keepalives_idle config parameters (brianc#1847)
* Added the missing connect_timeout and keepalives_idle config parameters * Implementation and tests for keepAliveInitialDelayMillis and connectionTimeoutMillis [squashed 4]
1 parent 4b530a9 commit 0993e4b

File tree

7 files changed

+160
-7
lines changed

7 files changed

+160
-7
lines changed

lib/client.js

+12
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,15 @@ var Client = function (config) {
4444
stream: c.stream,
4545
ssl: this.connectionParameters.ssl,
4646
keepAlive: c.keepAlive || false,
47+
keepAliveInitialDelayMillis: c.keepAliveInitialDelayMillis || 0,
4748
encoding: this.connectionParameters.client_encoding || 'utf8'
4849
})
4950
this.queryQueue = []
5051
this.binary = c.binary || defaults.binary
5152
this.processID = null
5253
this.secretKey = null
5354
this.ssl = this.connectionParameters.ssl || false
55+
this._connectionTimeoutMillis = c.connectionTimeoutMillis || 0
5456
}
5557

5658
util.inherits(Client, EventEmitter)
@@ -83,6 +85,14 @@ Client.prototype._connect = function (callback) {
8385
}
8486
this._connecting = true
8587

88+
var connectionTimeoutHandle
89+
if (this._connectionTimeoutMillis > 0) {
90+
connectionTimeoutHandle = setTimeout(() => {
91+
con._ending = true
92+
con.stream.destroy(new Error('timeout expired'))
93+
}, this._connectionTimeoutMillis)
94+
}
95+
8696
if (this.host && this.host.indexOf('/') === 0) {
8797
con.connect(this.host + '/.s.PGSQL.' + this.port)
8898
} else {
@@ -159,6 +169,7 @@ Client.prototype._connect = function (callback) {
159169
return
160170
}
161171
this._connectionError = true
172+
clearTimeout(connectionTimeoutHandle)
162173
if (callback) {
163174
return callback(err)
164175
}
@@ -196,6 +207,7 @@ Client.prototype._connect = function (callback) {
196207
con.removeListener('errorMessage', connectingErrorHandler)
197208
con.on('error', connectedErrorHandler)
198209
con.on('errorMessage', connectedErrorMessageHandler)
210+
clearTimeout(connectionTimeoutHandle)
199211

200212
// process possible callback argument to Client#connect
201213
if (callback) {

lib/connection-parameters.js

+19-2
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,22 @@ var ConnectionParameters = function (config) {
6666
this.fallback_application_name = val('fallback_application_name', config, false)
6767
this.statement_timeout = val('statement_timeout', config, false)
6868
this.query_timeout = val('query_timeout', config, false)
69+
70+
if (config.connectionTimeoutMillis === undefined) {
71+
this.connect_timeout = process.env.PGCONNECT_TIMEOUT || 0
72+
} else {
73+
this.connect_timeout = Math.floor(config.connectionTimeoutMillis / 1000)
74+
}
75+
76+
if (config.keepAlive === false) {
77+
this.keepalives = 0
78+
} else if (config.keepAlive === true) {
79+
this.keepalives = 1
80+
}
81+
82+
if (typeof config.keepAliveInitialDelayMillis === 'number') {
83+
this.keepalives_idle = Math.floor(config.keepAliveInitialDelayMillis / 1000)
84+
}
6985
}
7086

7187
// Convert arg to a string, surround in single quotes, and escape single quotes and backslashes
@@ -75,7 +91,7 @@ var quoteParamValue = function (value) {
7591

7692
var add = function (params, config, paramName) {
7793
var value = config[paramName]
78-
if (value) {
94+
if (value !== undefined && value !== null) {
7995
params.push(paramName + '=' + quoteParamValue(value))
8096
}
8197
}
@@ -87,8 +103,9 @@ ConnectionParameters.prototype.getLibpqConnectionString = function (cb) {
87103
add(params, this, 'port')
88104
add(params, this, 'application_name')
89105
add(params, this, 'fallback_application_name')
106+
add(params, this, 'connect_timeout')
90107

91-
var ssl = typeof this.ssl === 'object' ? this.ssl : { sslmode: this.ssl }
108+
var ssl = typeof this.ssl === 'object' ? this.ssl : this.ssl ? { sslmode: this.ssl } : {}
92109
add(params, ssl, 'sslmode')
93110
add(params, ssl, 'sslca')
94111
add(params, ssl, 'sslkey')

lib/connection.js

+4-3
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ var Connection = function (config) {
2121
config = config || {}
2222
this.stream = config.stream || new net.Socket()
2323
this._keepAlive = config.keepAlive
24+
this._keepAliveInitialDelayMillis = config.keepAliveInitialDelayMillis
2425
this.lastBuffer = false
2526
this.lastOffset = 0
2627
this.buffer = null
@@ -47,17 +48,17 @@ var Connection = function (config) {
4748
util.inherits(Connection, EventEmitter)
4849

4950
Connection.prototype.connect = function (port, host) {
51+
var self = this
52+
5053
if (this.stream.readyState === 'closed') {
5154
this.stream.connect(port, host)
5255
} else if (this.stream.readyState === 'open') {
5356
this.emit('connect')
5457
}
5558

56-
var self = this
57-
5859
this.stream.on('connect', function () {
5960
if (self._keepAlive) {
60-
self.stream.setKeepAlive(true)
61+
self.stream.setKeepAlive(true, self._keepAliveInitialDelayMillis)
6162
}
6263
self.emit('connect')
6364
})

lib/defaults.js

+7-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,13 @@ module.exports = {
5858
statement_timeout: false,
5959

6060
// max miliseconds to wait for query to complete (client side)
61-
query_timeout: false
61+
query_timeout: false,
62+
63+
connect_timeout: 0,
64+
65+
keepalives: 1,
66+
67+
keepalives_idle: 0
6268
}
6369

6470
var pgTypes = require('pg-types')
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
'use strict'
2+
const net = require('net')
3+
const buffers = require('../../test-buffers')
4+
const helper = require('./test-helper')
5+
6+
const suite = new helper.Suite()
7+
8+
const options = {
9+
host: 'localhost',
10+
port: 54321,
11+
connectionTimeoutMillis: 2000,
12+
user: 'not',
13+
database: 'existing'
14+
}
15+
16+
const serverWithConnectionTimeout = (timeout, callback) => {
17+
const sockets = new Set()
18+
19+
const server = net.createServer(socket => {
20+
sockets.add(socket)
21+
socket.once('end', () => sockets.delete(socket))
22+
23+
socket.on('data', data => {
24+
// deny request for SSL
25+
if (data.length === 8) {
26+
socket.write(Buffer.from('N', 'utf8'))
27+
// consider all authentication requests as good
28+
} else if (!data[0]) {
29+
socket.write(buffers.authenticationOk())
30+
// send ReadyForQuery `timeout` ms after authentication
31+
setTimeout(() => socket.write(buffers.readyForQuery()), timeout).unref()
32+
// respond with our canned response
33+
} else {
34+
socket.write(buffers.readyForQuery())
35+
}
36+
})
37+
})
38+
39+
let closing = false
40+
const closeServer = done => {
41+
if (closing) return
42+
closing = true
43+
44+
server.close(done)
45+
for (const socket of sockets) {
46+
socket.destroy()
47+
}
48+
}
49+
50+
server.listen(options.port, options.host, () => callback(closeServer))
51+
}
52+
53+
suite.test('successful connection', done => {
54+
serverWithConnectionTimeout(0, closeServer => {
55+
const timeoutId = setTimeout(() => {
56+
throw new Error('Client should have connected successfully but it did not.')
57+
}, 3000)
58+
59+
const client = new helper.Client(options)
60+
client.connect()
61+
.then(() => client.end())
62+
.then(() => closeServer(done))
63+
.catch(err => closeServer(() => done(err)))
64+
.then(() => clearTimeout(timeoutId))
65+
})
66+
})
67+
68+
suite.test('expired connection timeout', done => {
69+
serverWithConnectionTimeout(options.connectionTimeoutMillis * 2, closeServer => {
70+
const timeoutId = setTimeout(() => {
71+
throw new Error('Client should have emitted an error but it did not.')
72+
}, 3000)
73+
74+
const client = new helper.Client(options)
75+
client.connect()
76+
.then(() => client.end())
77+
.then(() => closeServer(() => done(new Error('Connection timeout should have expired but it did not.'))))
78+
.catch(err => {
79+
assert(err instanceof Error)
80+
assert(/timeout expired\s*/.test(err.message))
81+
closeServer(done)
82+
})
83+
.then(() => clearTimeout(timeoutId))
84+
})
85+
})

test/test-helper.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ var expect = function (callback, timeout) {
134134
assert.ok(executed,
135135
'Expected execution of function to be fired within ' + timeout +
136136
' milliseconds ' +
137-
+' (hint: export TEST_TIMEOUT=<timeout in milliseconds>' +
137+
' (hint: export TEST_TIMEOUT=<timeout in milliseconds>' +
138138
' to change timeout globally)' +
139139
callback.toString())
140140
}, timeout)

test/unit/client/set-keepalives.js

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
'use strict'
2+
const net = require('net')
3+
const pg = require('../../../lib/index.js')
4+
const helper = require('./test-helper')
5+
6+
const suite = new helper.Suite()
7+
8+
suite.test('setting keep alive', done => {
9+
const server = net.createServer(c => {
10+
c.destroy()
11+
server.close()
12+
})
13+
14+
server.listen(7777, () => {
15+
const stream = new net.Socket()
16+
stream.setKeepAlive = (enable, initialDelay) => {
17+
assert(enable === true)
18+
assert(initialDelay === 10000)
19+
done()
20+
}
21+
22+
const client = new pg.Client({
23+
host: 'localhost',
24+
port: 7777,
25+
keepAlive: true,
26+
keepAliveInitialDelayMillis: 10000,
27+
stream
28+
})
29+
30+
client.connect().catch(() => {})
31+
})
32+
})

0 commit comments

Comments
 (0)