Skip to content

Commit 705d89c

Browse files
committed
Give same error to queued queries as to active query when ending
and do so in the native Client as well.
1 parent 99ba060 commit 705d89c

File tree

4 files changed

+115
-74
lines changed

4 files changed

+115
-74
lines changed

lib/client.js

+24-21
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,22 @@ var Client = function (config) {
5353

5454
util.inherits(Client, EventEmitter)
5555

56+
Client.prototype._errorAllQueries = function (err) {
57+
const enqueueError = (query) => {
58+
process.nextTick(() => {
59+
query.handleError(err, this.connection)
60+
})
61+
}
62+
63+
if (this.activeQuery) {
64+
enqueueError(this.activeQuery)
65+
this.activeQuery = null
66+
}
67+
68+
this.queryQueue.forEach(enqueueError)
69+
this.queryQueue.length = 0
70+
}
71+
5672
Client.prototype._connect = function (callback) {
5773
var self = this
5874
var con = this.connection
@@ -127,21 +143,7 @@ Client.prototype._connect = function (callback) {
127143

128144
const connectedErrorHandler = (err) => {
129145
this._queryable = false
130-
131-
const enqueueError = (query) => {
132-
process.nextTick(() => {
133-
query.handleError(err, con)
134-
})
135-
}
136-
137-
if (this.activeQuery) {
138-
enqueueError(this.activeQuery)
139-
this.activeQuery = null
140-
}
141-
142-
this.queryQueue.forEach(enqueueError)
143-
this.queryQueue = []
144-
146+
this._errorAllQueries(err)
145147
this.emit('error', err)
146148
}
147149

@@ -192,17 +194,17 @@ Client.prototype._connect = function (callback) {
192194
})
193195

194196
con.once('end', () => {
195-
if (this.activeQuery) {
196-
var disconnectError = new Error('Connection terminated')
197-
this.activeQuery.handleError(disconnectError, con)
198-
this.activeQuery = null
199-
}
197+
const error = this._ending
198+
? new Error('Connection terminated')
199+
: new Error('Connection terminated unexpectedly')
200+
201+
this._errorAllQueries(error)
202+
200203
if (!this._ending) {
201204
// if the connection is ended without us calling .end()
202205
// on this client then we have an unexpected disconnection
203206
// treat this as an error unless we've already emitted an error
204207
// during connection.
205-
const error = new Error('Connection terminated unexpectedly')
206208
if (this._connecting && !this._connectionError) {
207209
if (callback) {
208210
callback(error)
@@ -213,6 +215,7 @@ Client.prototype._connect = function (callback) {
213215
connectedErrorHandler(error)
214216
}
215217
}
218+
216219
this.emit('end')
217220
})
218221

lib/native/client.js

+75-46
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,10 @@ var Client = module.exports = function (config) {
3232
})
3333

3434
this._queryQueue = []
35-
this._connected = false
35+
this._ending = false
3636
this._connecting = false
37+
this._connected = false
38+
this._queryable = true
3739

3840
// keep these on the object for legacy reasons
3941
// for the time being. TODO: deprecate all this jazz
@@ -52,50 +54,48 @@ Client.Query = NativeQuery
5254

5355
util.inherits(Client, EventEmitter)
5456

57+
Client.prototype._errorAllQueries = function (err) {
58+
const enqueueError = (query) => {
59+
process.nextTick(() => {
60+
query.native = this.native
61+
query.handleError(err)
62+
})
63+
}
64+
65+
if (this._hasActiveQuery()) {
66+
enqueueError(this._activeQuery)
67+
this._activeQuery = null
68+
}
69+
70+
this._queryQueue.forEach(enqueueError)
71+
this._queryQueue.length = 0
72+
}
73+
5574
// connect to the backend
5675
// pass an optional callback to be called once connected
5776
// or with an error if there was a connection error
58-
// if no callback is passed and there is a connection error
59-
// the client will emit an error event.
60-
Client.prototype.connect = function (cb) {
77+
Client.prototype._connect = function (cb) {
6178
var self = this
6279

63-
var onError = function (err) {
64-
if (cb) return cb(err)
65-
return self.emit('error', err)
66-
}
67-
68-
var result
69-
if (!cb) {
70-
var resolveOut, rejectOut
71-
cb = (err) => err ? rejectOut(err) : resolveOut()
72-
result = new global.Promise(function (resolve, reject) {
73-
resolveOut = resolve
74-
rejectOut = reject
75-
})
76-
}
77-
7880
if (this._connecting) {
7981
process.nextTick(() => cb(new Error('Client has already been connected. You cannot reuse a client.')))
80-
return result
82+
return
8183
}
8284

8385
this._connecting = true
8486

8587
this.connectionParameters.getLibpqConnectionString(function (err, conString) {
86-
if (err) return onError(err)
88+
if (err) return cb(err)
8789
self.native.connect(conString, function (err) {
88-
if (err) return onError(err)
90+
if (err) return cb(err)
8991

9092
// set internal states to connected
9193
self._connected = true
9294

9395
// handle connection errors from the native layer
9496
self.native.on('error', function (err) {
95-
// error will be handled by active query
96-
if (self._activeQuery && self._activeQuery.state !== 'end') {
97-
return
98-
}
97+
self._queryable = false
98+
self._errorAllQueries(err)
9999
self.emit('error', err)
100100
})
101101

@@ -110,12 +110,26 @@ Client.prototype.connect = function (cb) {
110110
self.emit('connect')
111111
self._pulseQueryQueue(true)
112112

113-
// possibly call the optional callback
114-
if (cb) cb()
113+
cb()
115114
})
116115
})
116+
}
117117

118-
return result
118+
Client.prototype.connect = function (callback) {
119+
if (callback) {
120+
this._connect(callback)
121+
return
122+
}
123+
124+
return new Promise((resolve, reject) => {
125+
this._connect((error) => {
126+
if (error) {
127+
reject(error)
128+
} else {
129+
resolve()
130+
}
131+
})
132+
})
119133
}
120134

121135
// send a query to the server
@@ -129,26 +143,43 @@ Client.prototype.connect = function (cb) {
129143
// optional string rowMode = 'array' for an array of results
130144
// }
131145
Client.prototype.query = function (config, values, callback) {
146+
var query
147+
var result
148+
132149
if (typeof config.submit === 'function') {
150+
result = query = config
133151
// accept query(new Query(...), (err, res) => { }) style
134152
if (typeof values === 'function') {
135153
config.callback = values
136154
}
137-
this._queryQueue.push(config)
138-
this._pulseQueryQueue()
139-
return config
155+
} else {
156+
query = new NativeQuery(config, values, callback)
157+
if (!query.callback) {
158+
let resolveOut, rejectOut
159+
result = new Promise((resolve, reject) => {
160+
resolveOut = resolve
161+
rejectOut = reject
162+
})
163+
query.callback = (err, res) => err ? rejectOut(err) : resolveOut(res)
164+
}
140165
}
141166

142-
var query = new NativeQuery(config, values, callback)
143-
var result
144-
if (!query.callback) {
145-
let resolveOut, rejectOut
146-
result = new Promise((resolve, reject) => {
147-
resolveOut = resolve
148-
rejectOut = reject
167+
if (!this._queryable) {
168+
query.native = this.native
169+
process.nextTick(() => {
170+
query.handleError(new Error('Client has encountered a connection error and is not queryable'))
171+
})
172+
return result
173+
}
174+
175+
if (this._ending) {
176+
query.native = this.native
177+
process.nextTick(() => {
178+
query.handleError(new Error('Client was closed and is not queryable'))
149179
})
150-
query.callback = (err, res) => err ? rejectOut(err) : resolveOut(res)
180+
return result
151181
}
182+
152183
this._queryQueue.push(query)
153184
this._pulseQueryQueue()
154185
return result
@@ -157,6 +188,9 @@ Client.prototype.query = function (config, values, callback) {
157188
// disconnect from the backend server
158189
Client.prototype.end = function (cb) {
159190
var self = this
191+
192+
this._ending = true
193+
160194
if (!this._connected) {
161195
this.once('connect', this.end.bind(this, cb))
162196
}
@@ -170,12 +204,7 @@ Client.prototype.end = function (cb) {
170204
})
171205
}
172206
this.native.end(function () {
173-
// send an error to the active query
174-
if (self._hasActiveQuery()) {
175-
var msg = 'Connection terminated'
176-
self._queryQueue.length = 0
177-
self._activeQuery.handleError(new Error(msg))
178-
}
207+
self._errorAllQueries(new Error('Connection terminated'))
179208
self.emit('end')
180209
if (cb) cb()
181210
})

test/integration/client/error-handling-tests.js

+1-4
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,9 @@ suite.test('query receives error on client shutdown', function (done) {
7373
client.query(new pg.Query(config), assert.calls(function (err, res) {
7474
assert(err instanceof Error)
7575
queryError = err
76+
done()
7677
}))
7778
setTimeout(() => client.end(), 50)
78-
client.once('end', () => {
79-
assert(queryError instanceof Error)
80-
done()
81-
})
8279
}))
8380
})
8481

test/integration/connection-pool/error-tests.js

+15-3
Original file line numberDiff line numberDiff line change
@@ -52,15 +52,23 @@ suite.test('connection-level errors cause queued queries to fail', (cb) => {
5252
const pool = new pg.Pool()
5353
pool.connect(assert.success((client, done) => {
5454
client.query('SELECT pg_terminate_backend(pg_backend_pid())', assert.calls((err) => {
55-
assert.equal(err.code, '57P01')
55+
if (helper.args.native) {
56+
assert.ok(err)
57+
} else {
58+
assert.equal(err.code, '57P01')
59+
}
5660
}))
5761

5862
pool.once('error', assert.calls((err, brokenClient) => {
5963
assert.equal(client, brokenClient)
6064
}))
6165

6266
client.query('SELECT 1', assert.calls((err) => {
63-
assert.equal(err.message, 'Connection terminated unexpectedly')
67+
if (helper.args.native) {
68+
assert.ok(/^server closed the connection unexpectedly/.test(err.message))
69+
} else {
70+
assert.equal(err.message, 'Connection terminated unexpectedly')
71+
}
6472

6573
done()
6674
pool.end()
@@ -73,7 +81,11 @@ suite.test('connection-level errors cause future queries to fail', (cb) => {
7381
const pool = new pg.Pool()
7482
pool.connect(assert.success((client, done) => {
7583
client.query('SELECT pg_terminate_backend(pg_backend_pid())', assert.calls((err) => {
76-
assert.equal(err.code, '57P01')
84+
if (helper.args.native) {
85+
assert.ok(err)
86+
} else {
87+
assert.equal(err.code, '57P01')
88+
}
7789
}))
7890

7991
pool.once('error', assert.calls((err, brokenClient) => {

0 commit comments

Comments
 (0)