-
Notifications
You must be signed in to change notification settings - Fork 1.9k
fix(postgres-driver): handle connection termination errors gracefully #10277
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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) => { |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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); | ||
| }); |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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) => { |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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).
Check List
Issue Reference this PR resolves
Fixes #10142
Conversation with Claude
claude.txt