fix(supabase): populate EncryptedSupabaseError.encryptionError (#626)#634
Conversation
🦋 Changeset detectedLatest commit: 663cfe9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
This PR fixes @cipherstash/stack-supabase so EncryptedSupabaseError.encryptionError is actually populated when an encrypt/decrypt step fails, allowing callers to branch on the underlying EncryptionError rather than relying on indirect heuristics.
Changes:
- Thread
EncryptionFailedError.encryptionErrorinto theexecute()catch-path error object (v2 + v3 via shared base behavior). - Tighten
EncryptionFailedError.encryptionErrorfromunknowntoEncryptionError, and synthesize anEncryptionErrorfor v3 contract-violation cases where no underlying failure exists. - Add regression tests asserting
encryptionErroris populated on real encryption failures (and remainsundefinedfor a non-encryption throw), plus a patch changeset.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/stack-supabase/src/query-builder.ts | Populate EncryptedSupabaseError.encryptionError when the caught error is an EncryptionFailedError; tighten the wrapper’s encryptionError type. |
| packages/stack-supabase/src/query-builder-v3.ts | Narrow encryptionFailure’s cause type to EncryptionError and synthesize one when missing (contract-violation paths). |
| packages/stack-supabase/tests/supabase-encryption-error.test.ts | Add regression coverage for #626 across v2 and v3 dialects. |
| .changeset/supabase-encryption-error.md | Patch changeset documenting the behavioral fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1be835a to
ef5d8c2
Compare
The query builder's catch block hardcoded `encryptionError: undefined`, so the typed `EncryptedSupabaseError.encryptionError` field was always empty and callers could only detect encryption failures indirectly (status/statusText or `.throwOnError()`). - Thread the wrapped `EncryptionError` through when the caught error is an `EncryptionFailedError`; leave it unset for plain PostgREST/API errors. - Tighten `EncryptionFailedError.encryptionError` from `unknown` to `EncryptionError` (all five throw sites already pass a `result.failure`). - v3 shares the base `execute()` catch; its `encryptionFailure` helper now synthesizes an `EncryptionError` for the contract-violation cases (bulk length mismatch, null envelope) that have no underlying failure object. - Add unit coverage for both dialects: populated on a genuine encryption failure, absent on a plain error. Closes #626 Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
…review) Address self-review findings on the #626 fix: - Test coverage: the v3 test routed through the shared base `encryptModel` path, so the v3-specific `encryptionFailure` synthesize branch this PR adds was unasserted (a regression to `cause` would not have failed any test). Replace it with a test that drives that branch directly via a bulk length-mismatch and asserts `error.encryptionError` is populated. Verified by mutation: reverting the branch turns the test red. - Comment: hedge the base catch-block comment — the wrapped value is the operation's `EncryptionError` OR, in v3, a synthesized one. - Style: collapse the two-line same-module `errors` import into the repo's combined `import { type X, Y }` idiom. Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
ef5d8c2 to
0990df7
Compare
…review) Copilot flagged that the v3 synthesize branch had two no-cause call sites (bulk length mismatch and null envelope) but only the failure-with-cause case was covered. The length-mismatch path is now tested; add the null-envelope path too so both no-cause call sites explicitly pin `error.encryptionError`. Claude-Session: https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w
Stacked on #630 (adapter package split).
Problem
The Supabase query builder's catch block hardcoded
encryptionError: undefined, so the typedEncryptedSupabaseError.encryptionErrorfield (packages/stack-supabase/src/types.ts) was alwaysundefined— even when the caught error was an encryption failure. Callers had no way to branch on it and were forced to detect encryption failures indirectly (status === 500 && statusText === 'Encryption Error', or.throwOnError()).Fix
execute()catch, thread the wrappedEncryptionErrorthrough when the caught error is anEncryptionFailedError; leaveencryptionErrorunset for plain PostgREST/API errors.EncryptionFailedError.encryptionErrorfromunknowntoEncryptionError— all five throw sites already pass aresult.failure/decrypted.failure.execute()catch, so it's fixed for free. ItsencryptionFailurehelper now synthesizes anEncryptionErrorfor the two contract-violation cases (bulk length mismatch, null envelope) that have no underlying failure object — a broken query encryption is still an encryption failure.Tests
New
supabase-encryption-error.test.tspins, for both dialects:error.encryptionErroris populated with the underlyingEncryptionError;error.encryptionErrorisundefinedwhileerror.messagestill carries the API error.Notes
stash-supabaseskill's caveat pointing at this bug lives in the unmerged PR docs(supabase): correct the stash-supabase skill #606, not on this branch; whoever merges docs(supabase): correct the stash-supabase skill #606 should drop it. The skill copy on this branch already documentserror.encryptionErrorcorrectly.@cipherstash/stack-supabasepatch.Closes #626
https://claude.ai/code/session_01MxTTPaPP16m6br7Hoab94w