-
-
Notifications
You must be signed in to change notification settings - Fork 19
fix: redirect and 404 responses in RSC requests #290
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix: redirect and 404 responses in RSC requests #290
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes RSC (React Server Component) redirect and 404 handling for POST requests and various redirect scenarios. The main goal is to properly handle redirects in middleware (to local routes, external URLs, and API routes) and missing routes in RSC requests, addressing CORS errors and improving error handling.
Key changes:
- Added new client handlers (
RedirectHandlerandReloadHandler) to handle RSC redirects and 404s on the client side - Modified redirect logic to differentiate between RSC component requests and regular requests, preventing premature HTTP redirect responses for RSC requests
- Enhanced 404 handling by triggering page reloads for RSC requests so custom error components can be rendered with proper HTTP status codes
- Removed error boundary support from file-router due to RSC serialization limitations
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test/apps/file-router.spec.mjs | Restructured tests into describe/it blocks and added comprehensive test coverage for new redirect and 404 scenarios |
| packages/react-server/server/render-rsc.jsx | Core RSC rendering changes: saves request body early to prevent re-reading, handles middleware redirect errors, and adds action redirect logic with rewrite support |
| packages/react-server/server/redirects.mjs | Updated RedirectError digest format and added isRedirectError helper; conditionally creates redirect responses based on request type |
| packages/react-server/lib/plugins/file-router/entrypoint.jsx | Removed error boundary support, updated 404 handling to use ReloadHandler for RSC requests, and simplified component loading |
| packages/react-server/lib/http/middleware.mjs | Improved abort handling for client disconnects with better error suppression and cleanup logic |
| packages/react-server/lib/dev/ssr-handler.mjs | Added redirect detection for middleware errors and passes them to render for proper RSC serialization |
| packages/react-server/client/ReloadHandler.jsx | New client component that triggers a full page reload to get proper 404 error rendering |
| packages/react-server/client/RedirectHandler.jsx | New client component that performs client-side redirects for URLs from RSC redirect errors |
| packages/react-server/client/ErrorBoundary.jsx | Removed redirect handling logic that used digest-based URL extraction (now handled by RedirectHandler) |
| packages/react-server/client/ClientProvider.jsx | Added manual redirect handling and error throwing for non-2xx RSC responses |
| examples/file-router/pages/index.tsx | Added test links for various redirect and 404 scenarios |
| examples/file-router/pages/GET.api-redirect.server.ts | New API endpoint that redirects to external URL for testing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Handle redirect errors from actions | ||
| let redirectHandled = false; | ||
| if (renderContext.flags.isRSC && isRedirectError(error)) { | ||
| const redirectUrl = error.url || error.digest?.slice(9); // Extract URL from digest "Location=..." |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fallback error.digest?.slice(9) assumes the digest format is always "Location=..." but the RedirectError constructor at line 16 in redirects.mjs sets digest = \Location=${url}`without including the status code. The logic should rely onerror.urlwhich is always available on RedirectError instances. Consider simplifying to justconst redirectUrl = error.url;`
| const redirectUrl = error.url || error.digest?.slice(9); // Extract URL from digest "Location=..." | |
| const redirectUrl = error.url; // Always use error.url for RedirectError |
| fallback(), | ||
| loading(), | ||
| ]); | ||
| ] = await Promise.all([lazy(), fallback(), loading()]); |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fallback() component is still being loaded in Promise.all() but the result (_FallbackComponent) is never used. Consider removing it from the array to avoid unnecessary loading: ] = await Promise.all([lazy(), loading()]);
|
@lazarv I will need your expertise to get this finished 😊 |
|
Sorry @aheissenberger for the late review. As I was reviewing this PR, I ended up leaning towards an RSC native approach which I implemented in #292. I'll still need to investigate why production build test is failing on Windows, otherwise it should be good to go. Redirect implementation with RSC could be still better by leveraging server-side re-render for the page instead of handling the redirect error then fetching the RSC payload for the new location. |
fixes the following rsc POST request specific use cases:
fixes: #280 , #289
The tests have for the file-router have been extended to cover all new use cases.
The following cases are handled but could be better: