Skip to content

fix: preserve PostgreSQL failures through cleanup - #525

Merged
medz merged 1 commit into
mainfrom
fix/postgres-failure-recovery
Oct 9, 2026
Merged

medz merged 1 commit into
mainfrom
fix/postgres-failure-recovery

Conversation

@medz

@medz medz commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Problem and behavior

A direct PostgreSQL callback could catch a backend failure and return, then automatic rollback replaced the original server error with “connection is not open”. Preserve the first issued SQL error and its stack when rollback also fails; still discard the broken connection. Callback failures retain their existing priority.

Raise the minimum native postgres version from 3.5.12 to 3.5.20. In a real isolated cancellation failure, the old version let a timed-out query succeed and emitted an unhandled SocketException. The new version reports a catchable failure, replaces the connection and completes subsequent work. Use the upstream cancellation and hard-timeout fixes rather than introducing an ORM timeout/retry layer. Upstream changelog documents the cancellation, cleanup and deadline corrections.

Document native query/connect and server statement deadlines, including the application-callback boundary of close(). Public query APIs and generated clients remain unchanged.

Refs #516

Validation

  • Actual before/after regression: direct terminated-backend callback returned the original 57P01 error and stack after the fix; before it returned the cleanup error.
  • Five real PostgreSQL cases: server statement_timeout, native client queryTimeout, terminated transaction backend, caught direct-backend failure, and close while admitted work is interrupted. Each checks rollback, expired scopes, exact cause where relevant, connection replacement or close completion, and subsequent usable work. Termination targets only the backend observed executing pg_sleep through a separate control connection.
  • 26 focused existing/new PostgreSQL and runtime checks passed; analyzer clean. Full real SQLite/PostgreSQL CI is required before merge.
  • Independent local cancellation probe deleted only its own socket symlink, leaving the real server/socket intact. 3.5.12: query succeeded with one unhandled SocketException. 3.5.20: query failed, no zone errors, replacement backend, next SELECT succeeded. Both runs and teardown were bounded.
  • Lockfile changes only the native postgres dependency. No generated API or migration history changes.

Summary by CodeRabbit

  • Bug Fixes
    • When automatic transaction rollback fails, the original SQL error and its stack trace are now preserved when available. If no earlier SQL error exists, the rollback error is reported.
  • Documentation
    • Clarified PostgreSQL query and connection timeouts, transaction-scoped statement deadlines, and that these limits do not cover application work inside a transaction callback.
    • Documented that closing the database waits for active callbacks to finish.

@medz medz added the bug Something isn't working label Oct 9, 2026
@netlify

netlify Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for prisma-dart canceled.

Name Link
🔨 Latest commit aee7120
🔍 Latest deploy log https://app.netlify.com/projects/prisma-dart/deploys/6ac88084cdb8b80008dd74b6

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-09T05:52:31.248222Z aee7120 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 31539974-6707-4c52-bcd8-6333e864b1a6
📥 Commits

Reviewing files that changed from the base of the PR and between 8b3c3eb and aee7120.

⛔ Files ignored due to path filters (1)
  • pubspec.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • doc/README.md
  • lib/src/postgres/driver.dart
  • pubspec.yaml
  • test/postgres_recovery_test.dart

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.


📝 Walkthrough

Walkthrough

The PostgreSQL driver now preserves the first SQL failure and its stack trace when rollback fails. Documentation describes PostgreSQL timeout settings and close behavior. Opt-in integration tests cover timeout handling, backend termination, and admitted work draining during close.

Changes

PostgreSQL transaction recovery

Layer / File(s) Summary
Preserve SQL failures during cleanup
lib/src/postgres/driver.dart, pubspec.yaml, doc/README.md
When rollback fails, the driver rethrows the first recorded SQL failure and its stack trace, if available. Documentation describes this behavior, PostgreSQL timeout settings, and how close() waits for admitted callbacks. The PostgreSQL dependency constraint changes from ^3.5.12 to ^3.5.20.
Exercise PostgreSQL recovery and close behavior
test/postgres_recovery_test.dart
Opt-in integration tests cover server and client query timeouts, terminated backends, rollback and replacement connections, callback error propagation, and close() draining admitted work.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to aee71

No actionable issue remains from this review. Run the required real PostgreSQL CI checks before merging.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the primary change: preserving the original PostgreSQL failure when cleanup also fails.
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit watched the queries sleep,
While rollback kept their errors deep.
The first stack trace stayed in its place,
New connections joined the race.
Close waited till the work was through.

Comment @coderabbitai help to get the list of available commands.

@medz
medz merged commit d73fe50 into main Oct 9, 2026
6 checks passed
@medz medz added type:engine dependencies Pull requests that update a dependency file platform:dart labels Oct 9, 2026
@medz
medz deleted the fix/postgres-failure-recovery branch October 9, 2026 06:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working dependencies Pull requests that update a dependency file platform:dart type:engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant