Skip to content

Conversation

@pyramation
Copy link
Contributor

@pyramation pyramation commented Dec 24, 2025

Summary

Adds an error handler to pg pools to prevent unhandled 'error' events from crashing Node.js when database connections are terminated during cleanup.

When pgpm test-packages runs dropDatabase to clean up test databases, it terminates all connections via pg_terminate_backend(). This causes idle pool connections to receive a 57P01 (admin_shutdown) error. Without an error handler, Node.js crashes and dumps the entire pg Client object to the console - producing hundreds of lines of useless output instead of a meaningful error message.

The fix:

  • Attaches an error handler to each pool when created
  • Logs expected cleanup errors (57P01) at debug level
  • Logs unexpected errors at error level
  • Prevents the unhandled error from crashing the process

Why this is safe (does NOT swallow real errors):
The handler only catches errors emitted on IDLE pooled connections via the EventEmitter pattern. Query errors (pool.query(), client.query()) are returned via Promise rejection and bubble up through async/await as normal exceptions - they are NOT affected by this handler.

Updates since last revision

Added extensive inline documentation explaining:

  • Why the error handler exists (idle connection errors crash Node.js)
  • Why it's safe (does NOT swallow query errors - those use Promise rejection)
  • The two different error paths in pg-pool (query errors vs idle connection errors)
  • When this handler fires (during pg_terminate_backend cleanup)
  • PostgreSQL error codes handled (57P01 admin_shutdown)

Review & Testing Checklist for Human

  • Review the inline documentation in pg.ts to verify the explanation of pg-pool's two error paths (Promise rejection for queries vs EventEmitter for idle connections) is accurate
  • After merge, publish a new pgpm version and update constructive-db CI to use it
  • Run pgpm test-packages in constructive-db to confirm the fix produces clean output

Test plan: After publishing the new pgpm version, re-run CI on constructive-db PR #108 and verify all modules pass without the pg Client object dump.

Notes

This is a companion fix for constructive-db PR #108 which replaces the custom test-all-packages.js script with pgpm test-packages.

Local testing confirmed: ran pgpm test-packages against constructive-db using the locally built pgpm with this fix - all 37 modules passed successfully.

Link to Devin run: https://app.devin.ai/sessions/ef6ccaafd0f44b358f6b529e50306865
Requested by: Dan Lynch ([email protected]) / @pyramation

When connections are terminated during database cleanup (e.g., dropDatabase),
the pg pool receives an 'error' event that was previously unhandled, causing
Node.js to crash and dump the entire pg Client object to the console.

This fix adds an error handler to each pool that:
- Logs expected cleanup errors (57P01 admin_shutdown) at debug level
- Logs unexpected errors at error level
- Prevents the unhandled error from crashing the process

Co-Authored-By: Dan Lynch <[email protected]>
@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Adds excruciating detail explaining:
- Why the error handler exists (idle connection errors crash Node.js)
- Why it's safe (does NOT swallow query errors - those use Promise rejection)
- The two different error paths in pg-pool
- When this handler fires (during pg_terminate_backend cleanup)
- PostgreSQL error codes handled (57P01 admin_shutdown)

Co-Authored-By: Dan Lynch <[email protected]>
@pyramation pyramation closed this Dec 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants