fix(postgrest): throw on multi-row result from maybeSingle when throw-on-error is enabled - #1180
Open
grdsdev wants to merge 2 commits into
Open
fix(postgrest): throw on multi-row result from maybeSingle when throw-on-error is enabled#1180grdsdev wants to merge 2 commits into
grdsdev wants to merge 2 commits into
Conversation
PostgREST sends the detail text under the "details" key, but PostgrestError declares the property as "detail" with no CodingKeys, and the shared decoder sets no key strategy. Every PostgREST error therefore lost its detail text. Map detail to the "details" wire key so the value survives decoding, and encoding round-trips to the same shape. Mirrors #1159 (AndroidPoet), needed here so maybeSingle()'s multi-row fix in the next commit can rely on error.details actually decoding. Rebase to drop this commit once #1159 lands on main.
…-on-error is enabled
maybeSingle() swallowed PGRST116 unconditionally, so a query that matched
more than one row silently returned nil instead of surfacing the error --
hiding a real bug where the query should have been scoped to match at most
one row. Zero rows still returns nil; more than one row now throws, same as
single() already does.
PostgREST reports both cases with the same error code, distinguishable only
via the details message ("Results contain N rows, ..."), so maybeSingle now
inspects that to decide whether to swallow or rethrow.
Fixes #1170
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughSummary by CodeRabbit
Walkthrough
Sequence Diagram(s)sequenceDiagram
participant PostgrestBuilder
participant PostgREST
participant PostgrestError
PostgrestBuilder->>PostgREST: Execute maybeSingle request
PostgREST-->>PostgrestBuilder: Return data or PGRST116 error
PostgrestBuilder->>PostgrestError: Parse details
PostgrestError-->>PostgrestBuilder: Identify zero-row or multiple-row result
PostgrestBuilder-->>PostgrestBuilder: Return nil or rethrow error
Assessment against linked issues
Out-of-scope changes
Possibly related PRs
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 |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What
maybeSingle()unconditionally swallowedPGRST116("no/too many rows"), so a query that unexpectedly matched more than one row silently returnednilinstead of surfacing an error — hiding a real bug where the query should have been scoped to match at most one row.nil(unchanged)PostgrestError, matching the behavior ofsingle()PostgREST reports both cases with the same
PGRST116code; the row count is only distinguishable via thedetailsmessage ("Results contain N rows, application/vnd.pgrst.object+json requires 1 row"), somaybeSingle()now inspects that to decide whether to swallow or rethrow.Fixes #1170
Depends on #1159
This fix relies on
PostgrestError.detailsactually decoding from the server response — which it doesn't today onmain(see #1159: the wire key is"details", but the property/synthesized decoding key wasdetail, so it silently dropped).The first commit on this branch (
fix(helpers): decode PostgREST error details field) mirrors #1159's change so this PR is self-contained and testable independently. Once #1159 merges intomain, rebasing this branch will produce a trivial no-op conflict on that one commit (identical change) — drop it and keep the second commit.Verification
swift test --filter "PostgRESTTests|HelpersTests"— 229 tests passswift package diagnose-api-breaking-changes origin/main— no breaking changes in any module./scripts/format.sh— no-opAcceptance criteria
maybeSingle()behavior