Skip to content
Open
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
13 changes: 13 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,19 @@ function fastifyPostgres (fastify, options, next) {
delete options.name

const pool = new pg.Pool(options)
// pg-pool re-emits idle client errors here; unhandled, it crashes the process
pool.on('error', () => {})
// fires for idle and checked-out clients, so log only here
pool.on('connect', (client) => {
let errored = false
client.on('error', (err) => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

maybe once instead of errored?

Suggested change
client.on('error', (err) => {
client.once('error', (err) => {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, I tried it first since it reads cleaner. The problem is the checked-out case: pg-pool removes its idleListener while a client is checked out, so ours is the only 'error' listener on it. With once, it's gone after the first error, and the follow-up ECONNRESET / "Connection terminated unexpectedly" get emitted with no listener at all, which crashes the process. The checked-out test fails exactly that way with once (uncaught read ECONNRESET).

So the listener has to stay attached and just ignore the follow-ups. pg-pool solves the same thing for idle clients by swapping in a no-op listener after the first error. I can do it that way here too if you'd prefer it over the flag.

// a dropped connection emits follow-ups like 'Connection terminated unexpectedly'; like pg-pool, log only the first
if (errored) return
errored = true
fastify.log.error({ err, name }, 'pg client error')
})
})

const db = {
connect: pool.connect.bind(pool),
pool,
Expand Down
78 changes: 78 additions & 0 deletions test/initialization.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -237,3 +237,81 @@ test('const result = await fastify.pg and a fastify.pg custom namespace should e
t.assert.deepStrictEqual(result.rowCount, 1)
t.assert.deepStrictEqual(resultOne.rowCount, 1)
})

test('Should log instead of crashing when an idle client errors', { timeout: 10000 }, async (t) => {
t.plan(4)

const errorLogs = []
const stream = {
write (line) {
const log = JSON.parse(line)
if (log.level === 50) errorLogs.push(log)
}
}

const fastify = Fastify({ logger: { stream } })
t.after(() => fastify.close())

await fastify.register(fastifyPostgres, {
connectionString
})
await fastify.ready()

const client = await fastify.pg.connect()
const { rows } = await client.query('SELECT pg_backend_pid() AS pid')
const ended = new Promise((resolve) => client.once('end', resolve))
client.release()
t.assert.strictEqual(fastify.pg.pool.idleCount, 1)

const admin = new fastify.pg.Client({ connectionString })
await admin.connect()
await admin.query('SELECT pg_terminate_backend($1)', [rows[0].pid])
await admin.end()
await ended

t.assert.strictEqual(errorLogs.length, 1)
t.assert.strictEqual(errorLogs[0].err.code, '57P01')

const result = await fastify.pg.query('SELECT 1 AS one')
t.assert.strictEqual(result.rows[0].one, 1)
})

test('Should log instead of crashing when a checked-out client errors', { timeout: 10000 }, async (t) => {
t.plan(4)

const errorLogs = []
const stream = {
write (line) {
const log = JSON.parse(line)
if (log.level === 50) errorLogs.push(log)
}
}

const fastify = Fastify({ logger: { stream } })

await fastify.register(fastifyPostgres, {
connectionString
})
await fastify.ready()

const client = await fastify.pg.connect()
t.after(() => {
client.release()
return fastify.close()
})
const { rows } = await client.query('SELECT pg_backend_pid() AS pid')
const ended = new Promise((resolve) => client.once('end', resolve))

const admin = new fastify.pg.Client({ connectionString })
await admin.connect()
await admin.query('SELECT pg_terminate_backend($1)', [rows[0].pid])
await admin.end()
await ended

t.assert.strictEqual(errorLogs.length, 1)
t.assert.strictEqual(errorLogs[0].err.code, '57P01')
await t.assert.rejects(client.query('SELECT 1'), /not queryable/)

const result = await fastify.pg.query('SELECT 1 AS one')
t.assert.strictEqual(result.rows[0].one, 1)
})
Comment thread
Eomm marked this conversation as resolved.
Resolved
Loading