Skip to content

Commit 0e0ff2a

Browse files
committed
fix(pg): send Close and Sync when bind serialization throws
When connection.bind() throws during prepare(), the catch block previously called handleError() without sending any protocol messages. Since PARSE was already buffered (due to cork), the server processes it and waits for more messages, leaving the connection hung. Send Close to clean up the parsed statement and Sync to return the connection to a usable state.
1 parent a11064c commit 0e0ff2a

2 files changed

Lines changed: 90 additions & 0 deletions

File tree

packages/pg/lib/query.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,10 @@ class Query extends EventEmitter {
228228
valueMapper: utils.prepareValue,
229229
})
230230
} catch (err) {
231+
// we should close parse to avoid leaking connections
232+
connection.close({ type: 'S', name: this.name })
233+
connection.sync()
234+
231235
this.handleError(err, connection)
232236
return
233237
}
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
'use strict'
2+
const helper = require('./test-helper')
3+
const Query = require('../../../lib/query')
4+
const assert = require('assert')
5+
6+
const suite = new helper.Suite()
7+
8+
const bindError = new Error('TEST: Throw in bind')
9+
10+
const setupClient = function () {
11+
const client = helper.client()
12+
const con = client.connection
13+
const calls = { parse: 0, sync: 0, describe: 0, execute: 0, close: 0 }
14+
15+
con.parse = function () {
16+
calls.parse++
17+
}
18+
con.bind = function () {
19+
throw bindError
20+
}
21+
con.describe = function () {
22+
calls.describe++
23+
assert.fail('describe should not be called when bind throws')
24+
}
25+
con.execute = function () {
26+
calls.execute++
27+
assert.fail('execute should not be called when bind throws')
28+
}
29+
con.close = function () {
30+
calls.close++
31+
}
32+
con.sync = function () {
33+
calls.sync++
34+
}
35+
36+
return { client, con, calls }
37+
}
38+
39+
suite.test('calls callback with error when bind throws', function (done) {
40+
const { client, con, calls } = setupClient()
41+
con.emit('readyForQuery')
42+
client.query(
43+
new Query({
44+
text: 'select $1',
45+
values: ['x'],
46+
callback: function (err) {
47+
assert.equal(err, bindError)
48+
assert.equal(calls.sync, 1, 'sync should be called once')
49+
assert.equal(calls.describe, 0, 'describe should not be called')
50+
assert.equal(calls.execute, 0, 'execute should not be called')
51+
done()
52+
},
53+
})
54+
)
55+
})
56+
57+
suite.test('emits error event when bind throws (no callback)', function (done) {
58+
const { client, con, calls } = setupClient()
59+
con.emit('readyForQuery')
60+
const query = new Query({
61+
text: 'select $1',
62+
values: ['x'],
63+
})
64+
query.on('error', function (err) {
65+
assert.equal(err, bindError)
66+
assert.equal(calls.sync, 1, 'sync should be called once')
67+
done()
68+
})
69+
client.query(query)
70+
})
71+
72+
suite.test('send close when bind throws', function (done) {
73+
const { client, con, calls } = setupClient()
74+
con.emit('readyForQuery')
75+
client.query(
76+
new Query({
77+
text: 'select $1',
78+
values: ['x'],
79+
callback: function (err) {
80+
assert.equal(err, bindError)
81+
assert.equal(calls.close, 1, 'close should be called')
82+
done()
83+
},
84+
})
85+
)
86+
})

0 commit comments

Comments
 (0)