fix(attachments): read the storage service's new error envelope so gated downloads keep their friendly copy (objectstack#3675) - #2869
Merged
Conversation
…ted downloads keep their friendly copy (objectstack#3675)
`RecordAttachmentsPanel` maps the server's fail-closed 40x codes
(`AUTH_REQUIRED`, `ATTACHMENT_DOWNLOAD_DENIED`) to human copy by reading `code`
off the error body. It read the TOP LEVEL only. The storage service has moved
that code into the envelope its contract declares — `{ success: false, error:
{ code, message } }` — so the top-level read now returns `undefined` and every
gated download would have degraded from "You don't have access to download this
attachment." to the generic "Download failed (403)".
The download handler now reads `body?.error?.code ?? body?.code`, mirroring how
the success branch two lines below already reads `body?.url ?? body?.data?.url`.
Both dialects on purpose: the console ships independently of the server it
talks to, so a current console must keep understanding an older one — tolerance
is the right answer here, not a synchronized cutover.
The two existing tests now mock what the server actually sends, and a third
covers the legacy shape so the tolerance is an asserted property rather than an
accident. Mutation-checked: dropping the nested read fails the two
new-envelope cases.
The upload path needed no change — it stringifies the whole body into the error
message, so its substring match survives either shape.
Co-Authored-By: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
os-zhuang
marked this pull request as ready for review
July 27, 2026 14:43
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
2 participants
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.
Paired with objectstack-ai/objectstack#3687 (closes objectstack#3675). This PR is load-bearing for that one — without it, the server-side change silently degrades a user-facing message.
What breaks without this
RecordAttachmentsPanelmaps the server's fail-closed 40x codes (AUTH_REQUIRED,ATTACHMENT_DOWNLOAD_DENIED) to human copy. Its download handler read the code off the top level of the error body:The storage service has moved that code into the envelope its contract declares —
{ success: false, error: { code, message } }— so the top-level read now returnsundefined,friendlyErrormatches nothing, and every gated download falls through to the generic:instead of:
Silent, and only on the 401/403 paths — exactly the paths a user hits when something is already going wrong.
The fix
Both dialects on purpose. The console ships independently of the server it talks to, so a current console must keep understanding an older one; tolerance is the right answer here rather than a synchronized cutover, and it makes the merge order of the two PRs irrelevant. This mirrors what the success branch two lines below has always done:
body?.url ?? body?.data?.url.Tests
The two existing tests mocked the old server shape, so they would have kept passing while the real thing regressed — stale fiction, not coverage. They now mock what the server actually sends, and a third case covers the legacy shape so the tolerance is an asserted property rather than an accident.
Mutation-checked: reverting the fix to
body?.codefails the two new-envelope cases and passes the legacy one, which is the intended discrimination.packages/app-shell— 143 files, 1195 tests passed.Not changed
The upload path (
UploadProvider) needed nothing: it stringifies the whole body into the error message, sofriendlyError'sraw.includes(code)fallback matches either shape. Verified rather than assumed.🤖 Generated with Claude Code
https://claude.ai/code/session_01K35y3ovfWtCkBHYFCqUfAt
Generated by Claude Code