You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Cloudflare Observability shows frequent getAppOwnerPostgres - PostgreSQL Error events across multiple plugin regions and apps. The inspected invocation failed in about 84 ms, but the existing log only preserved the outer DrizzleQueryError; the nested node-postgres error and SQLSTATE were lost.
This is intentionally a logging-only change. It does not change database queries, retries, app classification, on-premise responses, status caching, or endpoint behavior.
What changed
Recursively serialize Drizzle/node-postgres cause chains up to eight levels, with path-scoped circular-reference protection.
Capture PostgreSQL diagnostics including SQLSTATE code, severity, detail, hint, position, internal query context, schema/table/column/type/constraint, and server file/line/routine.
Capture socket/TLS diagnostics including errno, syscall, address, port, host, OpenSSL fields, status, command, and aggregate connection attempts.
Preserve the SQL query and parameter count while redacting raw parameter values from structured fields, messages, and stacks.
Add filterable diagnostics for the Hyperdrive/database binding, Worker/runtime, request and Ray ID, colo/region, affected app ID, and plan actions.
Bound cause depth, structured depth, object keys, arrays, and aggregate errors so the failure path cannot create unbounded logging work.
This should distinguish replica restart/failover errors, stale or terminated connections, Hyperdrive/network failures, schema/query errors, and resource-related PostgreSQL failures without another instrumentation PR.
Validation
bunx vitest run tests/plugin-pg-error-logging.unit.test.ts tests/plugin-runtime-isolation.unit.test.ts tests/channel-self-pg-client.unit.test.ts - 20 tests passed
Adversarial coverage includes prototype-pollution keys, accessor side effects, cycles, shared aggregate branches, oversized aggregates, primitive throws, hostile error getters, and secret parameter redaction.
Summary by CodeRabbit
Bug Fixes
Improved handling of database, network, and runtime errors, including nested and aggregate failures.
Added safeguards for circular, deeply nested, malformed, or inaccessible error details.
Sensitive query parameter values are now redacted while retaining useful diagnostic information.
Database-related failures now include structured request, runtime, application, and worker context to support clearer troubleshooting and more reliable operational logging.
It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.
Use the following commands to manage reviews:
@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.
Use the checkboxes below for quick actions:
▶️ Resume reviews
🔍 Trigger review
📝 Walkthrough
Walkthrough
The PostgreSQL logging utility now serializes nested errors with bounds, cycle handling, and query-value redaction. logPgError records structured request, runtime, database, worker, and application diagnostics. Tests cover these behaviors and edge cases.
Changes
PostgreSQL error diagnostics and serialization
Layer / File(s)
Summary
Bounded serialization and structured logging supabase/functions/_backend/plugin_runtime/utils/pg.ts
serializePostgresError captures PostgreSQL, network, aggregate, cause, and primitive error data with recursive bounds, cycle detection, unsafe-key filtering, and query-parameter redaction. logPgError records structured diagnostics. getAppOwnerPostgres passes app ID and plan actions on database failure.
Error serialization and logging validation tests/plugin-pg-error-logging.unit.test.ts
The title clearly and concisely describes the addition of verbose PostgreSQL error diagnostics.
Description check
✅ Passed
The description explains the logging-only scope, diagnostic details, security controls, and validation steps, but omits the template headings and checklist.
Docstring Coverage
✅ Passed
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 docstrings
Create stacked PR
Commit on current branch
Comment @coderabbitai help to get the list of available commands.
Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.
CI status: the remaining Run unit tests failure reproduces directly on main at b5a9950 in tests/private-analytics-validation.unit.test.ts; it expects the old countDevices signature. The exact expectation update already exists in green PR #2805. The unrelated backend shard-2 upstream-response flake passed on rerun. I am keeping this PR logging-only as requested and will re-run/rebase once #2805 lands.
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@supabase/functions/_backend/plugin_runtime/utils/pg.ts`:
- Around line 531-537: Update the error traversal function containing the seen
cycle guard so seen tracks only the current recursion path: add the error before
descending into causes and remove it when that traversal branch completes,
including on early-return paths as needed. Preserve circular detection for true
ancestor cycles while allowing shared error objects in separate branches to be
fully serialized with their SQLSTATE and network fields.
- Around line 592-607: Update the diagnostics payload construction around the
diagnostics object so caller-provided diagnostics are nested under a dedicated
key rather than spread at the top level. Preserve the reserved version,
functionName, databaseSource, workerSource, runtime, and request values, and
update the expected payload in the plugin PG error-logging unit test to match
the nested structure.
- Around line 501-509: Update serializePostgresError and its field serialization
path to sanitize object and array values, including self-referential query,
reason, and opensslErrorStack fields, before formatLogArgs emits them. Reuse or
extend serializePrimitiveErrorValue with cycle detection and depth protection,
replacing unsupported or cyclic values with safe placeholders while preserving
existing primitive, bigint, symbol, and function handling.
In `@tests/plugin-pg-error-logging.unit.test.ts`:
- Around line 195-212: Add test cases alongside the existing
serializePostgresError coverage for primitive thrown values, including null and
strings, and for an error object with a getter that throws when its properties
are read. Assert the expected serialized fallback output for both branches,
using serializePostgresError directly and preserving the existing
circular/deep-chain tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
Push a commit to this branch (recommended)
Create a new PR with the fixes
ℹ️ Review info⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9b63abe0-e56c-4910-a91a-b58355970f3e
📥 Commits
Reviewing files that changed from the base of the PR and between b5a9950 and 6d19d9e.
Caller diagnostics still overwrite reserved keys inside the nested diagnostics object.
The payload now nests version, functionName, databaseSource, workerSource, runtime, and request under a diagnostics key, but the caller-supplied diagnostics parameter is still spread (...diagnostics) as a sibling of those reserved fields at Line 607. A caller that passes any of those key names (for example version or request) silently replaces the structured values, breaking log filters that depend on a stable schema.
Nest the caller-supplied data under its own key instead of spreading it alongside the reserved fields.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@supabase/functions/_backend/plugin_runtime/utils/pg.ts` around lines 578 -
609, Update logPgError so caller-supplied diagnostics are nested under a
dedicated key within the diagnostics payload rather than spread alongside the
reserved version, functionName, databaseSource, workerSource, runtime, and
request fields. Preserve the structured reserved values and use the existing
diagnostics parameter as the nested caller-data value.
seen still tracks the whole traversal instead of the current path.
seen.add(error) at Line 537 is never followed by a corresponding delete before the function returns at Line 575. When two branches (for example two AggregateError children or a shared connection error) reference the same object, the second branch is reported as { circular: true } and loses its SQLSTATE and network fields, even though it is not an ancestor cycle.
Scope seen to the current recursion path by removing the error from seen right before returning serialized.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@supabase/functions/_backend/plugin_runtime/utils/pg.ts` around lines 519 -
576, Update serializePostgresError so seen tracks only the active recursion
path: after serializing all fields, aggregate errors, and cause, remove the
current error from seen immediately before returning serialized. Preserve
circular detection for ancestor cycles while allowing shared objects in separate
branches to be fully serialized.
Coverage gaps for the primitive-throw and unreadable-property branches remain.
serializePostgresError still has two untested branches that run on the production catch path: the non-object throw branch (null, strings, bigint, and so on) and the branch where a property getter throws. Add cases for both, alongside the existing circular and depth-truncation tests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/plugin-pg-error-logging.unit.test.ts` around lines 195 - 212, Extend
the serializePostgresError coverage in the existing cause-chain test area to
exercise primitive/non-object thrown values, including representative null,
string, and bigint inputs, and an error object with a getter that throws when
its properties are read. Assert each result matches the serializer’s expected
fallback representation for those branches.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@supabase/functions/_backend/plugin_runtime/utils/pg.ts`:
- Around line 452-488: Update the POSTGRES_ERROR_FIELDS serialization loop to
recursively sanitize every field value using the same cycle and depth protection
already used for cause and errors, rather than only normalizing primitive types.
Ensure self-referencing objects and arrays cannot reach cloudlogErr unsafely,
and preserve the existing query/parameter redaction behavior for driver-provided
query values.
---
Duplicate comments:
In `@supabase/functions/_backend/plugin_runtime/utils/pg.ts`:
- Around line 578-609: Update logPgError so caller-supplied diagnostics are
nested under a dedicated key within the diagnostics payload rather than spread
alongside the reserved version, functionName, databaseSource, workerSource,
runtime, and request fields. Preserve the structured reserved values and use the
existing diagnostics parameter as the nested caller-data value.
- Around line 519-576: Update serializePostgresError so seen tracks only the
active recursion path: after serializing all fields, aggregate errors, and
cause, remove the current error from seen immediately before returning
serialized. Preserve circular detection for ancestor cycles while allowing
shared objects in separate branches to be fully serialized.
In `@tests/plugin-pg-error-logging.unit.test.ts`:
- Around line 195-212: Extend the serializePostgresError coverage in the
existing cause-chain test area to exercise primitive/non-object thrown values,
including representative null, string, and bigint inputs, and an error object
with a getter that throws when its properties are read. Assert each result
matches the serializer’s expected fallback representation for those branches.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
Push a commit to this branch (recommended)
Create a new PR with the fixes
ℹ️ Review info⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3136a105-5dbd-4e95-843d-f83b4148eb66
📥 Commits
Reviewing files that changed from the base of the PR and between b5a9950 and 6d19d9e.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Left a non-blocking comment — Cursor Bugbot was not present after the first poll so that signal was skipped; no approval-policy files applied, and this exceeds the low-risk approval threshold because it expands plugin-runtime error serialization and logged diagnostics surface. Reviewer assigned.
Sent by Cursor Approval Agent: Pull Request Approver External
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@supabase/functions/_backend/plugin_runtime/utils/pg.ts`:
- Around line 646-649: Update the aggregateErrors handling in
serializePostgresError to serialize only the first MAX_POSTGRES_LOG_ARRAY_ITEMS
entries, matching serializePostgresLogValue’s array behavior, and mark
serialized.errors as truncated when additional errors exist. Preserve recursive
serialization for the retained entries.
- Around line 529-589: Reduce cognitive complexity in serializePostgresLogValue
by extracting its Array.isArray handling and object-key serialization into
separate helper functions. Move the existing truncation, recursive
serialization, redaction, key-limit, and error-handling behavior into those
helpers, then have serializePostgresLogValue delegate to them while preserving
all current outputs and cycle/depth tracking.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
Push a commit to this branch (recommended)
Create a new PR with the fixes
ℹ️ Review info⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6ae5b9a8-4e6a-4f0a-9931-69704d6a87aa
📥 Commits
Reviewing files that changed from the base of the PR and between 6d19d9e and fc26799.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Left a non-blocking comment — Cursor Bugbot was not present after the first poll so that signal was skipped; no approval-policy files applied, and this exceeds the low-risk approval threshold because it expands plugin-runtime error serialization and the logged diagnostics surface. Human review still needed; reviewer already assigned.
Sent by Cursor Approval Agent: Pull Request Approver External
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Left a non-blocking comment — Cursor Bugbot was not present after the first poll so that signal was skipped; no approval-policy files applied, and this exceeds the low-risk approval threshold because it expands plugin-runtime error serialization and the logged diagnostics surface. Human review still needed; reviewer already assigned.
Sent by Cursor Approval Agent: Pull Request Approver External
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Left a non-blocking comment — Cursor Bugbot was not present after the first poll so that signal was skipped; no approval-policy files applied, and this exceeds the low-risk approval threshold because it expands plugin-runtime error serialization and the logged diagnostics surface. Human review still needed; reviewer already assigned.
Sent by Cursor Approval Agent: Pull Request Approver External
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@supabase/functions/_backend/plugin_runtime/utils/pg.ts`:
- Around line 679-744: Reduce the cognitive complexity of serializePostgresError
by extracting the name, message, stack, POSTGRES_ERROR_FIELDS, params, aggregate
errors, and cause collection into a dedicated helper. Have the helper populate
the provided serialized record using the existing readErrorProperty,
serialization, and recursive calls so behavior remains identical, while
serializePostgresError retains only orchestration and error-handling flow.
- Around line 509-535: readErrorProperty and readOwnLogProperty are duplicate
implementations; remove readOwnLogProperty and update all its call sites to use
readErrorProperty instead, keeping a single shared property-reading function.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
Push a commit to this branch (recommended)
Create a new PR with the fixes
ℹ️ Review info⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e40cf15f-786c-431d-b6ce-33a01f92e8e5
📥 Commits
Reviewing files that changed from the base of the PR and between fc26799 and 3a3fc7a.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Left a non-blocking comment — Cursor Bugbot was not present after the first poll so that signal was skipped; no approval-policy files applied, and this exceeds the low-risk approval threshold because it expands plugin-runtime error serialization and the logged diagnostics surface. Human review still needed; reviewer already assigned.
Sent by Cursor Approval Agent: Pull Request Approver External
The reason will be displayed to describe this comment to others. Learn more.
Risk: medium. Left a non-blocking comment — Cursor Bugbot was not present after the first poll so that signal was skipped; no approval-policy files applied, and this exceeds the low-risk approval threshold because it expands plugin-runtime error serialization and the logged diagnostics surface. Human review still needed; reviewer already assigned.
Sent by Cursor Approval Agent: Pull Request Approver External
The reason will be displayed to describe this comment to others. Learn more.
Risk: medium. Approved — Cursor Bugbot was not present after the first poll, so that signal was skipped; no approval-policy files applied. This remains a bounded plugin error-path logging change with unit tests; reviewer already assigned, no additional reviewers added.
Sent by Cursor Approval Agent: Pull Request Approver
The catch at Line 1358 also catches errors from runPair and closeClient. An error from either PostgreSQL query therefore reaches Line 1362 and reruns both queries on the primary client. This adds an implicit retry, can amplify database load, and prevents the outer handler from logging the original error.
Catch only client acquisition and initialization failures. Keep runPair and client cleanup outside that fallback catch.
Proposed fix
- try {- const parallelClient = await getPgClient(c, true)- try {- const drizzleParallel = getDrizzleClient(parallelClient, { logger: false })- return await runPair(drizzleClient, drizzleParallel)- }- finally {- await closeClient(c, parallelClient)- }- }- catch {- // Latency opt only — fall back to serial queries on the primary client.- }+ let parallelClient: Awaited<ReturnType<typeof getPgClient>> | null = null+ try {+ parallelClient = await getPgClient(c, true)+ }+ catch {+ // Latency opt only — fall back to serial queries on the primary client.+ }+ if (parallelClient !== null) {+ try {+ const drizzleParallel = getDrizzleClient(parallelClient, { logger: false })+ return await runPair(drizzleClient, drizzleParallel)+ }+ finally {+ await closeClient(c, parallelClient)+ }+ }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@supabase/functions/_backend/plugin_runtime/utils/pg.ts` around lines 1348 -
1362, Restrict the fallback catch around getPgClient and getDrizzleClient so it
handles only parallel-client acquisition or initialization failures. Keep
runPair and closeClient outside that catch, ensuring query or cleanup errors
propagate to the outer handler instead of triggering the serial fallback at the
final runPair call.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@supabase/functions/_backend/plugin_runtime/utils/pg.ts`:
- Around line 1348-1362: Restrict the fallback catch around getPgClient and
getDrizzleClient so it handles only parallel-client acquisition or
initialization failures. Keep runPair and closeClient outside that catch,
ensuring query or cleanup errors propagate to the outer handler instead of
triggering the serial fallback at the final runPair call.
ℹ️ Review info⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5f520d9d-fbe5-4287-aa9e-a793160ed15a
📥 Commits
Reviewing files that changed from the base of the PR and between 81d4e4a and bd118da.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
GitHub Actions
Inactive
Context
Cloudflare Observability shows frequent
getAppOwnerPostgres - PostgreSQL Errorevents across multiple plugin regions and apps. The inspected invocation failed in about 84 ms, but the existing log only preserved the outerDrizzleQueryError; the nested node-postgres error and SQLSTATE were lost.This is intentionally a logging-only change. It does not change database queries, retries, app classification, on-premise responses, status caching, or endpoint behavior.
What changed
causechains up to eight levels, with path-scoped circular-reference protection.code, severity, detail, hint, position, internal query context, schema/table/column/type/constraint, and server file/line/routine.Security and privacy
__proto__,constructor, andprototypekeys before they reach log processors.Expected diagnostic value
The next occurrence should expose fields such as:
This should distinguish replica restart/failover errors, stale or terminated connections, Hyperdrive/network failures, schema/query errors, and resource-related PostgreSQL failures without another instrumentation PR.
Validation
bunx vitest run tests/plugin-pg-error-logging.unit.test.ts tests/plugin-runtime-isolation.unit.test.ts tests/channel-self-pg-client.unit.test.ts- 20 tests passedbun lint:backendbun run typecheck:backendbunx wrangler deploy --config cloudflare_workers/plugin/wrangler.jsonc --env prod_eu --dry-runSummary by CodeRabbit