fix(postgres-driver): handle connection termination errors gracefully#10277
fix(postgres-driver): handle connection termination errors gracefully#10277igorlukanin wants to merge 1 commit intomasterfrom
Conversation
github-actions
Bot
added
driver:postgres
There was a problem hiding this comment.
Pull request overview
This PR addresses unhandled error events from PostgreSQL connections that can crash the Node.js process when database connections are terminated unexpectedly (e.g., due to max connections being reached). The changes replace direct console.log calls with the proper databasePoolError method and add error handlers to individual pool clients.
- Replaces console.log statements with calls to
databasePoolErrormethod in pool error handlers - Adds error event listeners to individual pool clients to prevent unhandled errors from crashing the process
- Applies the same fix consistently across both PostgresDriver and QuestDriver
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/cubejs-postgres-driver/src/PostgresDriver.ts | Updated pool error handler to use databasePoolError and added client error handlers in stream() and queryResponse() methods |
| packages/cubejs-questdb-driver/src/QuestDriver.ts | Updated pool error handler to use databasePoolError and added client error handler in queryResponse() method |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Attach error handler to the client to prevent unhandled error events | ||
| // from crashing the process when connections are terminated unexpectedly. | ||
| // See: https://github.com/brianc/node-postgres/issues/2112 | ||
| conn.on('error', (err) => { |
There was a problem hiding this comment.
The error listener is being attached each time a connection is acquired from the pool, but it's never removed. When connections are reused from the pool, this will result in multiple identical error listeners accumulating on the same connection object, causing a memory leak. Consider using conn.once('error', ...) instead of conn.on('error', ...) to ensure the listener is automatically removed after the first error, or explicitly remove the listener in the finally block or release callback.
| conn.on('error', (err) => { | |
| conn.once('error', (err) => { |
| conn.on('error', (err) => { | ||
| this.databasePoolError(err); | ||
| }); |
There was a problem hiding this comment.
The error listener is being attached each time a connection is acquired from the pool, but it's never removed. When connections are reused from the pool, this will result in multiple identical error listeners accumulating on the same connection object, causing a memory leak. Consider using conn.once('error', ...) instead of conn.on('error', ...) to ensure the listener is automatically removed after the first error, or explicitly remove the listener in the finally block.
| // Attach error handler to the client to prevent unhandled error events | ||
| // from crashing the process when connections are terminated unexpectedly. | ||
| // See: https://github.com/brianc/node-postgres/issues/2112 | ||
| conn.on('error', (err) => { |
There was a problem hiding this comment.
The error listener is being attached each time a connection is acquired from the pool, but it's never removed. When connections are reused from the pool, this will result in multiple identical error listeners accumulating on the same connection object, causing a memory leak. Consider using conn.once('error', ...) instead of conn.on('error', ...) to ensure the listener is automatically removed after the first error, or explicitly remove the listener in the finally block.
| conn.on('error', (err) => { | |
| conn.once('error', (err) => { |
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## master #10277 +/- ##
===========================================
- Coverage 83.27% 55.17% -28.11%
===========================================
Files 248 221 -27
Lines 74448 17202 -57246
Branches 0 3521 +3521
===========================================
- Hits 61999 9491 -52508
+ Misses 12449 7216 -5233
- Partials 0 495 +495
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Add error handlers to pool clients to prevent unhandled error events from crashing the process when PostgreSQL connections are terminated unexpectedly (e.g., when max connections are reached). Fixes #10142
|
Closing this PR because it contains a memory leak, and this Thanks |
Add error handlers to pool clients to prevent unhandled error events from crashing the process when PostgreSQL connections are terminated unexpectedly (e.g., when max connections are reached).
Check List
Issue Reference this PR resolves
Fixes #10142
Conversation with Claude
claude.txt