chore: gracefully handle dev callback failed #1081
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
juliusmarminge
added
the
❗ infra changes required
|
Warning Rate limit exceeded@juliusmarminge has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 42 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (2)
WalkthroughThe pull request introduces enhancements to error handling and logging mechanisms across multiple files in the UploadThing SDK. The changes focus on improving error reporting, particularly in development environments, by adding more detailed error tracking in the upload handling process. Modifications include the addition of a new middleware for session validation, improvements to the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
📦 Bundle size comparison
|
More templates
commit: |
juliusmarminge
added
the
✅ infra changes implemented
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
playground/middleware.ts (1)
9-18: Handle potential errors fromgetSessionto prevent middleware failureIf
getSession()throws an error, it could cause the middleware to fail unexpectedly. Wrap the call togetSession()in a try-catch block to handle potential errors and respond appropriately.Apply this diff to handle errors from
getSession:export default (async (req) => { if (req.nextUrl.pathname !== "/") { + let sesh; + try { const sesh = await getSession(); + } catch (error) { + console.error("Failed to get session:", error); + return NextResponse.json({ error: "Internal Server Error" }, { status: 500 }); + } if (!sesh) { return NextResponse.json({ error: "Forbidden" }, { status: 403 }); } } return NextResponse.next(); }) satisfies NextMiddleware;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/uploadthing/src/internal/handler.ts(3 hunks)packages/uploadthing/src/internal/upload.browser.ts(2 hunks)playground/middleware.ts(1 hunks)
🔇 Additional comments (4)
packages/uploadthing/src/internal/upload.browser.ts (2)
3-3: Import statement approved
The addition of hasProperty and isRecord from "effect/Predicate" is appropriate for the enhanced error handling logic.
41-61: Handle non-JSON responses gracefully
Since xhr.responseType is set to "json", if the server returns a non-JSON response, xhr.response might be null. Ensure that the code handles scenarios where the response is not valid JSON to prevent unexpected errors.
Please verify how the code behaves when the server returns a non-JSON response and handle it appropriately.
packages/uploadthing/src/internal/handler.ts (1)
662-669: Confirm proper error propagation in handleDevStreamError usage
When forwarding callback requests, handleDevStreamError is invoked upon a ResponseError. Ensure that any errors thrown within handleDevStreamError are properly caught to prevent unhandled promise rejections.
Please confirm that handleDevStreamError handles its internal errors gracefully and that they are appropriately logged without causing unhandled exceptions.
playground/middleware.ts (1)
20-22: Verify the matcher patterns to ensure correct route matching
The matcher patterns in the middleware configuration may unintentionally include or exclude certain routes. Double-check the regular expressions to confirm they match the intended routes and exclude static assets or Next.js internals.
Please ensure that:
- Static files and assets (e.g., images, CSS, JS files) are excluded.
- Next.js internal routes (e.g.,
/_next) are excluded. - Desired API routes and pages are correctly matched.
Requires https://github.com/pingdotgg/uploadthing-infra/pull/615
Closes #1078
instead of silently throwing and holding the client upload, a failed dev hook now reports back to UT if the callback failed for any reason
Summary by CodeRabbit
New Features
Bug Fixes
Documentation