fix(backend): split Drizzle error fingerprints and handle transient PG failures - #3183
fix(backend): split Drizzle error fingerprints and handle transient PG failures#3183riderx wants to merge 6 commits into
Conversation
|
Warning Review limit reached
On-demand reviews are free for the next 26 days. After that, they cost $0.25 per reviewed file. Or wait 7 minutes for your next included review. View limit detailsLimit details: You’ve used all 2 included reviews currently available. Your 53 included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (10)
Comment |
Merging this PR will not alter performance
Comparing 
Footnotes
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 10 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 10 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…G failures PostHog grouped unrelated DrizzleQueryErrors (manifest vs notification_app_settings) because exception fingerprints ignored the failed query. notification_app_settings only had 5 real failures in 90 days; the rest were manifest query noise. - Add shared pg_errors helpers for transient detection and table-based Drizzle fingerprints - Return 503 upstream_unavailable for transient Drizzle errors without Discord/PostHog alerts - Attach pg_error_cause to PostHog exception payloads for faster triage Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
- Classify transient native pg errors before the Drizzle guard - Fingerprint Drizzle errors by table plus PostgreSQL code - Parse quoted schema-qualified table names correctly - Send only pg_error_code to PostHog (no user-supplied cause text) - Drop 08P01 from transient SQLSTATE set (protocol violations stay visible) Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
- Add isDatabaseOriginError/isTransientDatabaseError to avoid masking unrelated upstream network failures as database outages - Handle quickError originalCause before HTTP alert path - Include pgErrorCode in transient error logs for triage Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
- Validate SQLSTATE shape instead of any five-character code - Detect pg connect failures via syscall/port metadata, not only :5432 - Keep unrelated upstream network and custom error codes out of 503 path Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
…tion - Replace non-db port denylist with explicit PG port allowlist (5432/6543/6432) - Match db.*.supabase hostnames instead of any supabase API URL - Preserve :5432 detection for message-only wrapped connection errors Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
8bf37c4 to
82e23ac
Compare
Add 54322 to postgres port detection so transient local database connection failures still return the retryable 503 path. Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
|
TorichanCapgo
left a comment
TorichanCapgo
left a comment
There was a problem hiding this comment.
CI green, all cubic review threads resolved. Local Supabase port 54322 added to PG connect allowlist.
3 participants
Summary (AI generated)
019f3c00-5ad8-7e12-881d-743797f0bbc4reporting ~5.5kDrizzleQueryErrorhits onnotification_app_settingsnotification_app_settingsfailures in the last 90 days (allGET /notifications/settings); ~32.7k sibling events weremanifestquery failures grouped into the same issuegetNotificationSettings()already returns defaults when no row existspg_errorshelpers and fixed PostHog exception fingerprinting to include the queried table / PG code503 upstream_unavailablewithout Discord/PostHog alerts54322in PG connect allowlist for dev/CI transient DB detectionMotivation (AI generated)
The PostHog issue looked like a high-volume
notification_app_settingsregression, but the inflated count came from overly broad Drizzle exception fingerprints that merged unrelated failed queries (mostlymanifest). The handful of real notification settings failures were sporadic transient Postgres connection errors, not a logic bug ingetNotificationSettings().Business Impact (AI generated)
Test Plan (AI generated)
bunx vitest run tests/pg-errors.unit.test.ts tests/on-error-posthog.unit.test.ts tests/posthog.unit.test.ts tests/rbac-permission-infra-errors.unit.test.tsbun lintGenerated with AI
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.