Skip to content

Commit eaecf05

Browse files
brophdawg11joseph0926
authored andcommitted
Propagate middleware thrown non-redirect responses to the boundary (#14182)
1 parent a447e8b commit eaecf05

File tree

4 files changed

+63
-16
lines changed

4 files changed

+63
-16
lines changed

.changeset/heavy-months-work.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"react-router": patch
3+
---
4+
5+
Propagate non-redirect Responses thrown from middleware to the error boundary on document/data requests

docs/how-to/middleware.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -445,7 +445,7 @@ const authMiddleware = async ({ request, context }) => {
445445

446446
### `next()` and Error Handling
447447

448-
React Router contains built-in error handling via the route [`ErrorBoundary`][ErrorBoundary] export. Just like when a `action`/`loader` throws (_mostly_), if a `middleware` throws it will be caught and handled at the appropriate [`ErrorBoundary`] and a [`Response`][Response] will be returned through the ancestor `next()` call. This means that the `next()` function should never throw and should always return a [`Response`][Response], so you don't need to worry about wrapping it in a try/catch.
448+
React Router contains built-in error handling via the route [`ErrorBoundary`][ErrorBoundary] export. Just like when a `action`/`loader` throws, if a `middleware` throws it will be caught and handled at the appropriate [`ErrorBoundary`] and a [`Response`][Response] will be returned through the ancestor `next()` call. This means that the `next()` function should never throw and should always return a [`Response`][Response], so you don't need to worry about wrapping it in a try/catch.
449449

450450
This behavior is important to allow middleware patterns such as automatically setting required headers on outgoing responses (i.e., committing a session) from a root `middleware`. If any error from a `middleware` caused `next()` to `throw`, we'd miss the execution of ancestor middlewares on the way out and those required headers wouldn't be set.
451451

@@ -473,8 +473,6 @@ export const unstable_middleware: Route.unstable_MiddlewareFunction[] =
473473
];
474474
```
475475

476-
<docs-info>We say _"mostly"_ above because there is a small/nuanced difference if you throw a non-redirect [`Response`][Response] in a `action`/`loader` versus throwing a [`Response`][Response] in a `middleware` (redirects behave the same).<br/><br/>Throwing a non-redirect response from a `middleware` will use that response directly and return it up through the parent `next()` call. This differs from the behavior in `action`/`loader` where that response will be converted to an [`ErrorResponse`][ErrorResponse] to be rendered by the [`ErrorBoundary`][ErrorBoundary].<br/><br/>The difference here is because `action`/`loader` are expected to return data which is then provided to components for rendering. But middleware is expected to return a [`Response`][Response] — so if you return or throw one, we will use it directly. If you want to throw an error with a status code to an [`ErrorBoundary`][ErrorBoundary] from middleware, you should use the [`data`][data] utility.</docs-info>
477-
478476
## Changes to `getLoadContext`/`AppLoadContext`
479477

480478
<docs-info>This only applies if you are using a custom server and a custom `getLoadContext` function</docs-info>

packages/react-router/__tests__/router/context-middleware-test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2485,6 +2485,34 @@ describe("context/middleware", () => {
24852485
});
24862486
});
24872487

2488+
it("handles thrown Responses at the ErrorBoundary", async () => {
2489+
let handler = createStaticHandler([
2490+
{
2491+
path: "/",
2492+
unstable_middleware: [
2493+
async (_, next) => {
2494+
throw new Response("Error", { status: 401 });
2495+
},
2496+
],
2497+
loader() {
2498+
return "INDEX";
2499+
},
2500+
},
2501+
]);
2502+
2503+
let request = new Request("http://localhost/");
2504+
let res = (await handler.query(request, {
2505+
unstable_generateMiddlewareResponse: async (q) =>
2506+
respondWithJson(await q(request)),
2507+
})) as Response;
2508+
2509+
let staticContext = (await res.json()) as StaticHandlerContext;
2510+
expect(staticContext.errors).toEqual({
2511+
"0": new ErrorResponseImpl(401, undefined, "Error"),
2512+
});
2513+
expect(staticContext.statusCode).toBe(401);
2514+
});
2515+
24882516
it("allows thrown redirects before next()", async () => {
24892517
let handler = createStaticHandler([
24902518
{

packages/react-router/lib/router/router.ts

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3647,10 +3647,25 @@ export function createStaticHandler(
36473647
return res;
36483648
},
36493649
async (error, routeId) => {
3650-
if (isResponse(error)) {
3650+
// Redirects propagate verbatim
3651+
if (isRedirectResponse(error)) {
36513652
return error;
36523653
}
36533654

3655+
// All other thrown responses during document/data request bubble
3656+
// to the boundary
3657+
if (isResponse(error)) {
3658+
try {
3659+
error = new ErrorResponseImpl(
3660+
error.status,
3661+
error.statusText,
3662+
await parseResponseBody(error),
3663+
);
3664+
} catch (e) {
3665+
error = e;
3666+
}
3667+
}
3668+
36543669
if (isDataWithResponseInit(error)) {
36553670
// Convert thrown data() values to ErrorResponses for the UI
36563671
error = dataWithResponseInitToErrorResponse(error);
@@ -6031,6 +6046,18 @@ async function callLoaderOrAction({
60316046
return result;
60326047
}
60336048

6049+
async function parseResponseBody(response: Response) {
6050+
let contentType = response.headers.get("Content-Type");
6051+
6052+
// Check between word boundaries instead of startsWith() due to the last
6053+
// paragraph of https://httpwg.org/specs/rfc9110.html#field.content-type
6054+
if (contentType && /\bapplication\/json\b/.test(contentType)) {
6055+
return response.body == null ? null : response.json();
6056+
}
6057+
6058+
return response.text();
6059+
}
6060+
60346061
async function convertDataStrategyResultToDataResult(
60356062
dataStrategyResult: DataStrategyResult,
60366063
): Promise<DataResult> {
@@ -6040,18 +6067,7 @@ async function convertDataStrategyResultToDataResult(
60406067
let data: any;
60416068

60426069
try {
6043-
let contentType = result.headers.get("Content-Type");
6044-
// Check between word boundaries instead of startsWith() due to the last
6045-
// paragraph of https://httpwg.org/specs/rfc9110.html#field.content-type
6046-
if (contentType && /\bapplication\/json\b/.test(contentType)) {
6047-
if (result.body == null) {
6048-
data = null;
6049-
} else {
6050-
data = await result.json();
6051-
}
6052-
} else {
6053-
data = await result.text();
6054-
}
6070+
data = await parseResponseBody(result);
60556071
} catch (e) {
60566072
return { type: ResultType.error, error: e };
60576073
}

0 commit comments

Comments
 (0)