Skip to content

Commit 718584d

Browse files
authored
Bubble client middleware errors before next to the right boundary (#14138)
1 parent daa188a commit 718584d

File tree

4 files changed

+115
-10
lines changed

4 files changed

+115
-10
lines changed

.changeset/few-papayas-admire.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+
Bubble client-side middleware errors prior to `next` to the appropriate ancestor error boundary

integration/middleware-test.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,10 @@ test.describe("Middleware", () => {
508508
await page.waitForSelector('a:has-text("Link")');
509509

510510
(await page.getByRole("link"))?.click();
511-
await page.waitForSelector('h1:has-text("broken!")');
511+
// Bubbles to the root error boundary in SPA mode because every route has
512+
// a loader to load the client assets
513+
await page.waitForSelector('h1:has-text("Application Error")');
514+
await page.waitForSelector('pre:has-text("Error: broken!")');
512515

513516
appFixture.close();
514517
});
@@ -1145,7 +1148,10 @@ test.describe("Middleware", () => {
11451148
await page.waitForSelector('a:has-text("Link")');
11461149

11471150
(await page.getByRole("link"))?.click();
1148-
await page.waitForSelector('h1:has-text("broken!")');
1151+
// Bubbles to the root error boundary in SPA mode because every route has
1152+
// a loader to load the client assets
1153+
await page.waitForSelector('h1:has-text("Application Error")');
1154+
await page.waitForSelector('pre:has-text("Error: broken!")');
11491155

11501156
appFixture.close();
11511157
});

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

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1374,6 +1374,57 @@ describe("context/middleware", () => {
13741374
errors: null,
13751375
});
13761376
});
1377+
1378+
it("throwing from a middleware before next bubbles up to the highest route with a loader", async () => {
1379+
router = createRouter({
1380+
history: createMemoryHistory(),
1381+
routes: [
1382+
{
1383+
path: "/",
1384+
},
1385+
{
1386+
id: "a",
1387+
path: "/a",
1388+
hasErrorBoundary: true,
1389+
children: [
1390+
{
1391+
id: "b",
1392+
path: "b",
1393+
hasErrorBoundary: true,
1394+
loader: () => "B",
1395+
children: [
1396+
{
1397+
id: "c",
1398+
path: "c",
1399+
hasErrorBoundary: true,
1400+
children: [
1401+
{
1402+
id: "d",
1403+
path: "d",
1404+
hasErrorBoundary: true,
1405+
unstable_middleware: [
1406+
() => {
1407+
throw new Error("D ERROR");
1408+
},
1409+
],
1410+
loader: () => "D",
1411+
},
1412+
],
1413+
},
1414+
],
1415+
},
1416+
],
1417+
},
1418+
],
1419+
});
1420+
1421+
await router.navigate("/a/b/c/d");
1422+
1423+
expect(router.state.loaderData).toEqual({});
1424+
expect(router.state.errors).toEqual({
1425+
b: new Error("D ERROR"),
1426+
});
1427+
});
13771428
});
13781429
});
13791430

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

Lines changed: 51 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5369,13 +5369,49 @@ async function defaultDataStrategyWithMiddleware(
53695369
return defaultDataStrategy(args);
53705370
}
53715371

5372+
let didCallHandler = false;
53725373
return runClientMiddlewarePipeline(
53735374
args,
5374-
() => defaultDataStrategy(args),
5375-
(error, routeId) => ({ [routeId]: { type: "error", result: error } }),
5375+
() => {
5376+
didCallHandler = true;
5377+
return defaultDataStrategy(args);
5378+
},
5379+
(error, routeId) =>
5380+
clientMiddlewareErrorHandler(
5381+
error,
5382+
routeId,
5383+
args.matches,
5384+
didCallHandler,
5385+
),
53765386
);
53775387
}
53785388

5389+
function clientMiddlewareErrorHandler(
5390+
error: unknown,
5391+
routeId: string,
5392+
matches: AgnosticDataRouteMatch[],
5393+
didCallHandler: boolean,
5394+
): Record<string, DataStrategyResult> {
5395+
if (didCallHandler) {
5396+
return {
5397+
[routeId]: { type: "error", result: error },
5398+
};
5399+
} else {
5400+
// We never even got to the handlers, so we've got no data.
5401+
// Find the boundary at or above the source of the middleware
5402+
// error or the highest loader. We can't render any UI below
5403+
// the highest loader since we have no loader data available
5404+
let boundaryRouteId = findNearestBoundary(
5405+
matches,
5406+
matches.find((m) => m.route.id === routeId || m.route.loader)?.route.id ||
5407+
routeId,
5408+
).route.id;
5409+
return {
5410+
[boundaryRouteId]: { type: "error", result: error },
5411+
};
5412+
}
5413+
}
5414+
53795415
export async function runServerMiddlewarePipeline(
53805416
args: (
53815417
| LoaderFunctionArgs<unstable_RouterContextProvider>
@@ -5798,10 +5834,12 @@ async function callDataStrategyImpl(
57985834
) & {
57995835
matches: DataStrategyMatch[];
58005836
};
5837+
let didCallHandler = false;
58015838
return runClientMiddlewarePipeline(
58025839
typedDataStrategyArgs,
5803-
() =>
5804-
cb({
5840+
() => {
5841+
didCallHandler = true;
5842+
return cb({
58055843
...typedDataStrategyArgs,
58065844
fetcherKey,
58075845
unstable_runClientMiddleware: () => {
@@ -5810,10 +5848,15 @@ async function callDataStrategyImpl(
58105848
"`unstable_runClientMiddleware` handler",
58115849
);
58125850
},
5813-
}),
5814-
(error: unknown, routeId: string) => ({
5815-
[routeId]: { type: "error", result: error },
5816-
}),
5851+
});
5852+
},
5853+
(error, routeId) =>
5854+
clientMiddlewareErrorHandler(
5855+
error,
5856+
routeId,
5857+
matches,
5858+
didCallHandler,
5859+
),
58175860
);
58185861
};
58195862

0 commit comments

Comments
 (0)