Translate getDocument PDO exceptions#925
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesDocument read safety
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PHPStan (2.2.5)Composer install failed: this project depends on private packages that require authentication (e.g. GitLab/GitHub, Laravel Nova, etc.). Instead, run PHPStan in a CI/CD pipeline where you can use custom packages — our pipeline remediation tool can use the PHPStan output from your CI/CD pipeline. 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. Comment |
Greptile SummaryThis PR translates PDO failures from SQL document reads through adapter-specific exception mapping.
Confidence Score: 5/5The PR appears safe to merge, with the changed exception translation and cleanup paths behaving consistently with the adapter contract. The primary PDO failure is retained across cursor cleanup, translated only after cleanup, and PostgreSQL document reads now use the existing execution hook that applies and resets statement timeouts. Important Files Changed
Reviews (1): Last reviewed commit: "(fix): translate getDocument PDO excepti..." | Re-trigger Greptile |
What does this PR do?
Translate PDO failures from
SQL::getDocument()through the adapter-specific exception mapper.A request timeout can fire while
Database::find()resolves collection metadata.getDocument()previously leaked the rawPDOException, so Appwrite returned a generic HTTP 500 instead of the expected database-timeout 408. The result depended on whether collection metadata was already cached.The query now uses the adapter execution hook, always attempts cursor cleanup, and preserves the primary operation exception if cleanup also fails. This also ensures Postgres applies and resets its statement timeout on this path.
Tests
Summary by CodeRabbit
Bug Fixes
Tests