fix: prevent Vercel Blob upload conflicts on retry#1606
Conversation
- Add addRandomSuffix and allowOverwrite to blob upload token options, preventing "blob already exists" errors on retry uploads - Use Promise.allSettled for resilient multi-file blob uploads - Surface meaningful error messages when blob uploads fail Co-Authored-By: Paperclip <noreply@paperclip.ing>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 7 minutes and 57 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR enhances sandbox file upload resilience by extending upload token configuration with Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38b6d5dfa6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
lib/sandboxes/uploadSandboxFiles.ts
Outdated
| const blobFiles = results | ||
| .filter( | ||
| (r): r is PromiseFulfilledResult<{ url: string; name: string }> => | ||
| r.status === "fulfilled", | ||
| ) |
There was a problem hiding this comment.
Surface rejected blob uploads in partial-success path
When a multi-file upload has mixed outcomes, this code keeps only fulfilled results and drops rejected uploads without recording them, so the function can return success even though some selected files never made it to blob storage. In that scenario result.errors remains empty for callers (e.g. the drop hook’s success toast path), which misreports the operation as fully successful and can cause silent data loss from the user’s perspective.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/sandboxes/uploadSandboxFiles.ts (1)
39-65:⚠️ Potential issue | 🟠 MajorPartial blob upload failures are swallowed instead of being returned to callers.
With
Promise.allSettled, rejected uploads are filtered out, but their messages are never added to the returnederrors. In mixed outcomes, this can hide failures from the UI even though some files failed at blob upload time.Proposed fix
export async function uploadSandboxFiles({ @@ }): Promise<{ uploaded: UploadedFile[]; errors?: string[] }> { const results = await Promise.allSettled( @@ const blobFiles = results @@ .map((r) => r.value); + + const blobUploadErrors = results + .filter((r): r is PromiseRejectedResult => r.status === "rejected") + .map((r) => + r.reason instanceof Error + ? r.reason.message + : String(r.reason ?? "Failed to upload file to temporary storage"), + ); if (blobFiles.length === 0) { - const firstError = results.find((r) => r.status === "rejected") as - | PromiseRejectedResult - | undefined; throw new Error( - firstError?.reason?.message || - "Failed to upload files to temporary storage", + blobUploadErrors[0] || "Failed to upload files to temporary storage", ); } @@ - return { - uploaded: data.uploaded || [], - errors: data.errors, - }; + const allErrors = [...blobUploadErrors, ...(data.errors ?? [])]; + return { + uploaded: data.uploaded || [], + ...(allErrors.length ? { errors: allErrors } : {}), + }; }As per coding guidelines,
lib/**/*.tsutility functions must provide proper error handling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/sandboxes/uploadSandboxFiles.ts` around lines 39 - 65, The current Promise.allSettled handling drops rejected upload results so partial failures are hidden; update the logic in uploadSandboxFiles (the block using Promise.allSettled, results, blobFiles and upload) to collect rejected entries from results, map them to structured error messages (including file.name and rejection reason), and if any rejections exist return or throw an AggregateError (or Error) that includes both the list of successfully uploaded blobFiles and a detailed array/string of failed files and their error messages so callers can surface partial-failure info to the UI.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@lib/sandboxes/uploadSandboxFiles.ts`:
- Around line 39-65: The current Promise.allSettled handling drops rejected
upload results so partial failures are hidden; update the logic in
uploadSandboxFiles (the block using Promise.allSettled, results, blobFiles and
upload) to collect rejected entries from results, map them to structured error
messages (including file.name and rejection reason), and if any rejections exist
return or throw an AggregateError (or Error) that includes both the list of
successfully uploaded blobFiles and a detailed array/string of failed files and
their error messages so callers can surface partial-failure info to the UI.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e7387b11-739e-47d0-8658-2984ad6bd153
📒 Files selected for processing (2)
app/api/sandbox/upload/route.tslib/sandboxes/uploadSandboxFiles.ts
- Remove allowOverwrite: true (redundant with addRandomSuffix) - Surface rejected blob uploads in the errors array so callers can report partial failures to the user Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
addRandomSuffix: trueandallowOverwrite: trueto blob upload token generation, preventing "blob already exists" errors on upload retriesPromise.allSettledfor multi-file blob uploads so individual failures don't break the entire batchTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit