fix(postgrest): decode PostgrestError details field - #1159
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.
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
Walkthrough
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 |
grdsdev
left a comment
grdsdev
left a comment
There was a problem hiding this comment.
Please, add a new details field to the error structure and deprecate the old detail, make detail a getter of the new details.
var detail: String? {
get { details }
}
No need to worry about someone storing the error type (which will break).
Renames the stored property to match the PostgREST wire key and the postgrest-js naming, keeping detail as a deprecated getter so existing call sites continue to compile.
|
Done in 6d947c1 — /// Additional detail about the error, as returned by PostgREST in the `details` field.
public let details: String?
@available(*, deprecated, renamed: "details")
public var detail: String? { details }Renaming the stored property means the synthesized One judgement call I'd like your read on: the public initializer also took a @available(*, deprecated, renamed: "init(details:hint:code:message:)")
public init(detail: String?, hint: String? = nil, code: String? = nil, message: String)It has no default value on
|
Coverage Report for CI Build 31087395821Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage decreased (-0.2%) to 83.826%Details
Uncovered Changes
Coverage Regressions35 previously-covered lines in 5 files lost coverage.
Coverage Stats
💛 - Coveralls |
|
Please fix both |
…ix spell check - Add pkey to dictionary.txt (Unknown word in PostgrestErrorTests.swift) - Register PostgrestError.details under database.query.from_table in sdk-compliance.yaml, since it's a new public symbol and no dedicated error-handling feature exists yet in the canonical registry
3 participants
What
PostgrestError.detailis alwaysnil. PostgREST sends the field asdetails(plural), the Swift property isdetail(singular), and there were noCodingKeysto bridge them — so the value has never bound.This adds the key mapping. The property name is unchanged.
Why it never worked
PostgrestClient.Configuration.jsonDecoderisJSONDecoder.supabase(), which configures only adateDecodingStrategy— there is nokeyDecodingStrategy, so nothing rewritesdetails→detailat decode time.PostgREST error bodies look like this:
{ "code": "23505", "details": "Key (id)=(1) already exists.", "hint": "Use a different id.", "message": "duplicate key value violates unique constraint \"users_pkey\"" }code,hintandmessagedecode fine. Onlydetailsilently drops, so the most actionable part of a constraint violation — which key collided — never reaches the caller.For parity:
postgrest-jsnames this propertydetails, matching the wire key.Verification
Reverting just the
CodingKeysblock fails the new test with the exact user-visible symptom:swift test --filter "HelpersTests|PostgRESTTests"— 223 tests passswift package diagnose-api-breaking-changes origin/main— no breaking changes in any module./scripts/format.sh— no-opTwo notes for review
Encoding changes too.
PostgrestErroris publicCodable, so it now writes"details"where it previously wrote"detail". Nothing in the SDK encodes this type, and the server is the only realistic producer, so symmetric round-tripping seems correct — but it is a behavior change for anyone who persisted an error with an older version. Happy to restrict the fix to a custominit(from:)and leave encoding alone if you'd rather avoid that.I did not rename
detail→details. Full naming parity withpostgrest-jswould be source-breaking for every user readingerror.detail. Mapping the coding key fixes the bug without touching the public surface.