DEV Community: Malbolged

Smashing a silent connection leak in node-postgres (pg-pool)

Malbolged — Fri, 17 Jul 2026 04:36:16 +0000

This is a submission for DEV's Summer Bug Smash: Clear the Lineup powered by Sentry.

I fixed a real, open performance/correctness bug in pg-pool (the connection pool that ships with
node-postgres, ~12.5k ⭐, downloaded tens of
millions of times a week), reproduced it against a real Postgres, hardened the fix with
Sentry and Google's Gemini in the loop, and added a regression test.

Issue: brianc/node-postgres#3543
PR: brianc/node-postgres#3711

Project Overview

node-postgres (pg) is the most widely used PostgreSQL client for Node.js. Almost everyone
who talks to Postgres from Node uses its built-in connection pool, pg-pool, directly or
through an ORM (Sequelize, TypeORM, Prisma's pg adapter, Knex…). When you set
connectionTimeoutMillis, you are telling the pool: "if getting me a connection takes longer
than this, give up and error." That guarantee is what this bug quietly breaks.

Bug Fix or Performance Improvement

Issue #3543 reports that when connectionTimeoutMillis fires while a new connection is being
established, a connection can still be opened on the Postgres side and is never cleaned up — the
pool keeps holding it, and with the native (pg-native) client the process can crash with exit
code 13.

The culprit is in Pool.newClient():

tid = setTimeout(() => {
  if (client.connection) {
    timeoutHit = true
    client.connection.stream.destroy()   // rip out the local socket
  } else if (!client.isConnected()) {
    timeoutHit = true
    client.end()
  }
}, this.options.connectionTimeoutMillis)

client.connect((err) => {
  if (tid) clearTimeout(tid)
  client.on('error', idleListener)
  if (err) { /* remove client, reject */ }
  else {
    // success path — never checks whether the timeout already fired
    return this._afterConnect(client, pendingItem, idleListener)
  }
})

Two things are wrong:

The success path never checks timeoutHit. If the connection finishes establishing right after the timeout has fired, client.connect(...) calls back with no error, so the pool happily returns/keeps a connection it had already "timed out" on.
stream.destroy() rips out the local socket instead of terminating the connection cleanly. The reporter's suggested one-liner (client.connection.end()) doesn't actually help on current pg — at the moment the timeout fires, client.connection is still falsy, so that branch never runs. (I verified this; more below.)

The net effect: connectionTimeoutMillis is silently violated — pool.connect() resolves
after the deadline instead of rejecting — and the timed-out connection is retained. In the
worst cases it deadlocks a max: 1 pool or crashes a pg-native process.

Code

The fix (packages/pg-pool/index.js):

       tid = setTimeout(() => {
+        this.log('ending client due to timeout')
+        timeoutHit = true
+        // Reject the checkout at the deadline and stop tracking this client
+        // immediately, so a connect that hangs (or never calls back) can neither
+        // delay the timeout past connectionTimeoutMillis nor leak into _clients.
+        this._clients = this._clients.filter((c) => c !== client)
+        if (!pendingItem.timedOut) {
+          pendingItem.timedOut = true
+          pendingItem.callback(new Error('Connection terminated due to connection timeout'), undefined, NOOP)
+        }
+        this._pulseQueue()
+        // Tear the half-open connection down cleanly (send a Terminate rather
+        // than ripping out the local socket) in the background, and silence the
+        // dying client's errors so they don't surface as an unhandled 'error'
+        // (which can crash the process, e.g. with pg-native).
+        client.removeAllListeners('error')
+        client.on('error', () => {})
         if (client.connection) {
-          this.log('ending client due to timeout')
-          timeoutHit = true
-          client.connection.stream.destroy()
+          client.connection.end()
         } else if (!client.isConnected()) {
-          this.log('ending client due to timeout')
-          timeoutHit = true
-          // force kill the node driver, and let libpq do its teardown
           client.end()
         }
       }, this.options.connectionTimeoutMillis)

         if (!pendingItem.timedOut) {
           pendingItem.callback(err, undefined, NOOP)
         }
+      } else if (timeoutHit) {
+        // Race: the connection finished establishing after the timeout already
+        // rejected the checkout and removed this client. Just close the
+        // late-completing connection cleanly so no backend is leaked.
+        this.log('client connected after timeout, discarding')
+        client.removeAllListeners('error')
+        client.on('error', () => {})
+        client.end()
       } else {

Plus a deterministic regression test in packages/pg-pool/test/connection-timeout.js.

My Improvements

I don't trust an open issue until I've reproduced it, so I stood up a throwaway PostgreSQL 18
cluster and built a harness around the reporter's trigger (delay the first real connect past
the timeout).

Rigorous leak measurement. Counting "idle backends with query_start IS NULL" is misleading —
it also catches a legitimately checked-out client that just hasn't run a query yet. So I tagged
the pool's connections with a unique application_name and compared what the pool thinks it
holds (pool.totalCount) against the backends Postgres actually holds for the pool:

variant	first `pool.connect()`	`connect` events	`pool.totalCount`	server backends	never-used backends
buggy	resolved after ~1150ms (timeout = 1000ms!)	2	2	2	1
reporter's 1-line fix	resolved after ~1150ms	2	2	2	1
my fix	rejected at ~1000ms	1	1	1	0

The headline defect on the pure-JS client is precisely this: the timeout is not honored and the
timed-out connection is retained. (The dramatic orphaned-backend / exit-13 crash the reporter
saw is specific to pg-native and proxied poolers like PgBouncer, which I couldn't fully
reproduce without a native build — so I'm careful to present those as reporter-observed, not
claimed.)

Testing. The project's own connection-timeout suite goes from 9 to 10 passing with my
change (the only failing test needs the native addon, which isn't built in my environment). My
new regression test — "does not retain a connection that establishes after the timeout" — is
deterministic (a 200ms connect vs a 100ms timeout): it fails on the original code (resolves
with no error) and passes on the fixed code (rejects, totalCount === 0).

Best Use of Sentry

The most interesting part of this bug is that, before the fix, the failure is invisible.
pool.connect() resolves successfully — there is no exception, no log, nothing for an
observability tool to catch — while connections quietly pile up on the database. So I wired a tiny
reproduction into @sentry/node to show the difference the fix makes to observability, not just
correctness.

Sentry auto-instrumented pg/pg-pool with zero effort from me, so each checkout shows up as a
db.pool.connect trace with nested pg.connect / pg.query spans. The two runs sit side by side
in the issue stream:

Before the fix, there is no exception to catch. The best I can do is detect the violation
myself and report it:

[before-fix (buggy)] connectionTimeoutMillis (1000ms) exceeded (1154ms) but pool.connect() RESOLVED — silent timeout; server backends=1, pool believes=1
After the fix, the timeout is a first-class, monitorable exception:

Connection terminated due to connection timeout

tags: outcome: rejected, scenario: after-fix (fixed)
context pool_timeout: connectionTimeoutMillis: 1000, elapsedMs: 1012, pool_totalCount: 0

That elapsedMs: 1012 against a 1000ms deadline is the whole fix in one number — the checkout now
fails at the deadline instead of resolving at ~1150ms, and pool_totalCount: 0 proves nothing
was retained. The root span drops from 1.16s (buggy) to 1.04s (fixed) for the same work.

The trace is my favourite artifact here. The before-fix waterfall shows a single
db.pool.connect root span lasting 1.16s — already longer than the 1000ms timeout it was
supposed to honor — and inside it, two pg.connect spans. That's the over-connecting rendered
as a picture: one checkout, two backends.

Seer, honestly

I ran Seer (Sentry's AI root-cause) on the timeout exception, and it independently landed on my
core finding without being told:

"pool.fixed.js correctly enforces the timeout (unlike pool.buggy.js which silently resolved at
~1100ms, violating the timeout contract)"

It reconstructed the whole sequence — connect exceeds connectionTimeoutMillis, setTimeout fires
at 1000ms, timeoutHit=true, the error gets wrapped — and even produced accurate reproduction
steps. That's a genuinely useful second opinion on a concurrency bug.

One detail it got wrong, and it's worth saying so: Seer reported the teardown as
client.connection.stream.destroy(). That's the stock pg-pool in node_modules — the fixed
file doesn't call stream.destroy() at all. It had reached for the nearest matching source rather
than the file the frames actually came from. It's a good reminder that AI root-cause is a strong
lead, not a verdict: the conclusion was right, one supporting detail wasn't, and only reading the
code told me which was which.

That's the real value story: the fix converts a silent, un-monitorable timeout violation into a
clean, captured, alertable error — one Seer can actually reason about.

Best Use of Google AI

I used Gemini (via Google AI Studio) as a reviewer at two points, and it materially improved
the fix.

First I asked it to locate the race and explain why the reporter's proposed one-liner is
insufficient. It correctly identified that "the else (success) path in the connect callback
does not check timeoutHit… this registers the timed-out client as active/idle in the pool,
leaking the backend connection."

Then I gave it my first draft of the fix and asked it to try to break it. It found three real
problems I had missed:

Deferred rejection — my draft only rejected once the connect callback fired, so the caller failed at deadline + socket-teardown time, not at the deadline.
Leak if the callback never fires — because cleanup lived in the connect callback, a hung connect would leak the client into _clients forever.
Unhandled 'error' crash risk — ending a still-connecting client emits an 'error'; if the pool has an error listener and the app doesn't, the process crashes. This is almost certainly the mechanism behind the reporter's pg-native exit-13.

I rewrote the fix to reject and remove the client immediately at the deadline, and to strip
error listeners + attach a silent catch-all before tearing the connection down in the background.
Then I re-ran the full test suite to confirm the hardened version still passes everything. (Full
prompt/response transcript included as a screenshot below.)

Using an AI to adversarially review my own patch — and verifying every suggestion against the
real test suite rather than trusting it — is what made this fix production-grade instead of just
"passing the repro."

(Screenshot to add: the AI Studio conversation, especially Gemini's critique of the draft fix.)

Wrapping up

Issue: brianc/node-postgres#3543
PR: brianc/node-postgres#3711
Reproduced against real Postgres 18, fixed, regression-tested (suite 10/10 minus the native-only test), and hardened with Sentry + Gemini in the loop.

Thanks for reading — and thanks to the reporter (@nicholaswold) for a beautifully detailed issue.

The new customer who took down everyone: an ORM include that ate our CRM

Malbolged — Fri, 17 Jul 2026 04:36:15 +0000

This is a submission for DEV's Summer Bug Smash: Smash Stories powered by Sentry.

A real production war story about how
one endpoint, one include too deep, and one very enthusiastic new customer combined into a
global outage — and what actually fixed it (after the first fix didn't).

The 15 minutes where everything was down

It started the way these things always do: a new customer onboarded, we imported their data, and
minutes later everyone's app started falling over. Not just the new customer — the whole
multi-tenant CRM. Dashboards spun. Deal pages hung. Then requests started timing out across the
board.

The trigger was data volume. The new account came in with a lot of history — far more
opportunities, contacts, activities, and line items than our typical tenant. Nothing about that
should take down other customers… except it did.

Following the smoke

The outage lined up almost perfectly with traffic to one endpoint: the one that returns the full
details of an opportunity/deal. On paper it's a read. In practice it had grown, over a couple
of years, into a monster.

It was a single Sequelize query with a tree of includes:

Opportunity.findByPk(id, {
  include: [
    { model: Account, include: [Contact, Address] },
    { model: LineItem, include: [Product, { model: PriceTier, include: [Currency] }] },
    { model: Activity, include: [User, Note] },
    { model: Stage, include: [Pipeline] },
    // …and more
  ],
})

Every include became a JOIN (or a follow-up query), and several of them pulled in their own
nested includes. For a normal-sized deal it was fine. For the new customer's fattest
opportunities — hundreds of line items, each with products and price tiers, plus a long activity
history — a single "show me this deal" call was fanning out into an enormous result set and a
pile of un-indexed lookups. The query blew past our HTTP timeout, the request died, the client
retried, and the retries piled onto an already-struggling database. Connections backed up, and
because every tenant shared the same pool, everyone felt it.

Fix #1: the one that felt good and wasn't

Under pressure, we did the obvious thing: we raised the timeout. The endpoint stopped
"failing," the alarms went quiet, and we told ourselves we'd bought time to do it properly.

That was a band-aid on a severed artery. All we'd done was let the slow query run longer while
holding a connection hostage the entire time. A week or two later, with a bit more data and a bit
more concurrency, the exact same failure came back — except now each hung request lingered even
longer before dying, so the pile-up was worse. Raising a timeout doesn't make a query fast; it
just changes how long you wait to find out it's slow.

Fix #2: the one that actually held

The real fix had two halves.

1. Stop returning everything at once. We split the single mega-endpoint into several focused
calls. The deal page didn't actually need every nested activity, note, and price tier on first
paint — it needed the core opportunity, and then the related collections lazily, as the user
scrolled to them. So we replaced the one-query-to-rule-them-all with:

a lean GET /opportunity/:id that returns just the opportunity and its immediate summary, and
separate, paginated endpoints for the heavy children (/opportunity/:id/line-items, /activities, etc.), each returning only the columns the UI renders.

That alone collapsed the worst-case payload from "the entire deal graph" to something bounded and
predictable, and it meant a single fat deal could no longer monopolize a connection for seconds.

2. Index the paths we actually query. The child lookups were hitting foreign keys and filter
columns that weren't indexed, so they degraded from index seeks to sequential scans as the tables
grew — which is exactly why the problem only showed up with a large tenant. We added the missing
indexes (the opportunity_id foreign keys on the child tables, plus the columns we filtered and
sorted on) so those per-collection queries stayed fast regardless of how much history a customer
had.

Split the work, return only what's needed, and index the access paths — and the outage stopped
recurring. We haven't seen it since. (I'm knocking on wood as I type this.)

What I took away from it

A timeout increase is not a fix. It's a way to postpone the same failure into a worse one. If raising a limit makes the symptom disappear, the real bug is still there — you've just muted the alarm.
include depth is a loaded gun in any ORM. Every convenient nested include is a JOIN or an N+1 waiting to matter. It's invisible at small scale and catastrophic at large scale.
In multi-tenant systems, one customer's data volume is everyone's problem when they share a connection pool. "It's slow for one account" quietly means "it's fragile for all of them."
Slowness usually lives at the access path. The endpoint looked like an application problem; the fix was mostly shape of the data returned + indexes. Measure before you rewrite.

The unglamorous version of the lesson: return less, and make sure the database can find what you
ask for. Both halves mattered — decomposition kept any one request bounded, and indexing kept
each of those smaller requests fast as the data grew.