diff --git a/.changeset/tough-dancers-own.md b/.changeset/tough-dancers-own.md
new file mode 100644
index 0000000000..a7134c568d
--- /dev/null
+++ b/.changeset/tough-dancers-own.md
@@ -0,0 +1,5 @@
+---
+"react-router": patch
+---
+
+UNSTABLE: Fix a few bugs with error bubbling in middleware use-cases
diff --git a/integration/middleware-test.ts b/integration/middleware-test.ts
index d433ea0ac5..b59b01a114 100644
--- a/integration/middleware-test.ts
+++ b/integration/middleware-test.ts
@@ -2140,7 +2140,7 @@ test.describe("Middleware", () => {
"app/routes/_index.tsx": js`
import { Link } from 'react-router'
export default function Component({ loaderData }) {
- return Link
+ return Link
}
`,
"app/routes/a.tsx": js`
@@ -2158,7 +2158,7 @@ test.describe("Middleware", () => {
return null;
}
export default function Component() {
- return
+ return
}
`,
"app/routes/a.b.c.tsx": js`
@@ -2192,6 +2192,109 @@ test.describe("Middleware", () => {
expect(await page.locator("h1").textContent()).toBe("A Error Boundary");
expect(await page.locator("pre").textContent()).toBe("broken!");
+ await app.goto("/");
+ await app.clickLink("/a/b/c/d");
+ expect(await page.locator("h1").textContent()).toBe("A Error Boundary");
+ expect(await page.locator("pre").textContent()).toBe("broken!");
+
+ appFixture.close();
+ });
+
+ test("bubbles errors on the way down up to the deepest error boundary when loaders aren't revalidating", async ({
+ page,
+ }) => {
+ let fixture = await createFixture(
+ {
+ files: {
+ "react-router.config.ts": reactRouterConfig({
+ middleware: true,
+ }),
+ "vite.config.ts": js`
+ import { defineConfig } from "vite";
+ import { reactRouter } from "@react-router/dev/vite";
+
+ export default defineConfig({
+ build: { manifest: true, minify: false },
+ plugins: [reactRouter()],
+ });
+ `,
+ "app/routes/_index.tsx": js`
+ import { Link } from 'react-router'
+ export default function Component({ loaderData }) {
+ return (
+ <>
+ /a/b
+
+ /a/b/c/d
+
+ );
+ }
+ `,
+ "app/routes/a.tsx": js`
+ import { Outlet } from 'react-router'
+ export default function Component() {
+ return
+ }
+ export function ErrorBoundary({ error }) {
+ return <>A Error Boundary
{error.message}
+ }
+ `,
+ "app/routes/a.b.tsx": js`
+ import { Link, Outlet } from 'react-router'
+ export function loader() {
+ return { message: "DATA" };
+ }
+ export default function Component({ loaderData }) {
+ return (
+ <>
+ AB: {loaderData.message}
+ /a/b/c/d
+
+
+ );
+ }
+ export function shouldRevalidate() {
+ return false;
+ }
+ `,
+ "app/routes/a.b.c.tsx": js`
+ import { Outlet } from 'react-router'
+ export default function Component() {
+ return
+ }
+ export function ErrorBoundary({ error }) {
+ return <>C Error Boundary
{error.message}
+ }
+ `,
+ "app/routes/a.b.c.d.tsx": js`
+ import { Outlet } from 'react-router'
+ export const unstable_middleware = [() => { throw new Error("broken!") }]
+ export const loader = () => null;
+ export default function Component() {
+ return
+ }
+ `,
+ },
+ },
+ UNSAFE_ServerMode.Development
+ );
+
+ let appFixture = await createAppFixture(
+ fixture,
+ UNSAFE_ServerMode.Development
+ );
+
+ let app = new PlaywrightFixture(appFixture, page);
+ await app.goto("/");
+ await app.clickLink("/a/b");
+ await page.waitForSelector("[data-ab]");
+ expect(await page.locator("[data-ab]").textContent()).toBe("AB: DATA");
+
+ await app.clickLink("/a/b/c/d");
+ await page.waitForSelector("[data-error-c]");
+ expect(await page.locator("h1").textContent()).toBe("C Error Boundary");
+ expect(await page.locator("pre").textContent()).toBe("broken!");
+
appFixture.close();
});
diff --git a/packages/react-router/lib/dom/ssr/single-fetch.tsx b/packages/react-router/lib/dom/ssr/single-fetch.tsx
index 964e8f02f7..63f5ac9fbc 100644
--- a/packages/react-router/lib/dom/ssr/single-fetch.tsx
+++ b/packages/react-router/lib/dom/ssr/single-fetch.tsx
@@ -24,6 +24,8 @@ import type { DataRouteMatch } from "../../context";
export const SingleFetchRedirectSymbol = Symbol("SingleFetchRedirect");
+class SingleFetchNoResultError extends Error {}
+
export type SingleFetchRedirectResult = {
redirect: string;
status: number;
@@ -466,9 +468,56 @@ async function singleFetchLoaderNavigationStrategy(
await resolvePromise;
+ await bubbleMiddlewareErrors(
+ singleFetchDfd.promise,
+ args.matches,
+ routesParams,
+ results
+ );
+
return results;
}
+// If a middleware threw on the way down, we won't have data for our requested
+// loaders and they'll resolve to `SingleFetchNoResultError` results. If this
+// happens, take the highest error we find in our results (which is a middleware
+// error if no loaders ever ran), and assign to these missing routes and let
+// the router bubble accordingly
+async function bubbleMiddlewareErrors(
+ singleFetchPromise: Promise,
+ matches: DataStrategyFunctionArgs["matches"],
+ routesParams: Set,
+ results: Record
+) {
+ try {
+ let middlewareError: unknown;
+ let fetchedData = await singleFetchPromise;
+
+ if ("routes" in fetchedData) {
+ for (let match of matches) {
+ if (match.route.id in fetchedData.routes) {
+ let routeResult = fetchedData.routes[match.route.id];
+ if ("error" in routeResult) {
+ middlewareError = routeResult.error;
+ break;
+ }
+ }
+ }
+ }
+
+ if (middlewareError !== undefined) {
+ Array.from(routesParams.values()).forEach((routeId) => {
+ if (results[routeId].result instanceof SingleFetchNoResultError) {
+ results[routeId].result = middlewareError;
+ }
+ });
+ }
+ } catch (e) {
+ // No-op - this logic is only intended to process successful responses
+ // If the `.data` failed, the routes will handle those errors themselves
+ }
+}
+
// Fetcher loader calls are much simpler than navigational loader calls
async function singleFetchLoaderFetcherStrategy(
args: DataStrategyFunctionArgs,
@@ -694,12 +743,16 @@ function unwrapSingleFetchResult(
}
let routeResult = result.routes[routeId];
- if ("error" in routeResult) {
+ if (routeResult == null) {
+ throw new SingleFetchNoResultError(
+ `No result found for routeId "${routeId}"`
+ );
+ } else if ("error" in routeResult) {
throw routeResult.error;
} else if ("data" in routeResult) {
return routeResult.data;
} else {
- throw new Error(`No response found for routeId "${routeId}"`);
+ throw new Error(`Invalid response found for routeId "${routeId}"`);
}
}
diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts
index ee41ea8e79..3b5c97f9b1 100644
--- a/packages/react-router/lib/router/router.ts
+++ b/packages/react-router/lib/router/router.ts
@@ -3614,19 +3614,26 @@ export function createStaticHandler(
dataRoutes,
renderedStaticContext,
error,
- findNearestBoundary(matches!, routeId).route.id
+ skipLoaderErrorBubbling
+ ? routeId
+ : findNearestBoundary(matches, routeId).route.id
)
);
} else {
// We never even got to the handlers, so we've got no data -
// just create an empty context reflecting the error.
- // Find the boundary at or above the highest loader. We can't
- // render any UI below there since we have no loader data available
- let loaderIdx = matches!.findIndex((m) => m.route.loader);
- let boundary =
- loaderIdx >= 0
- ? findNearestBoundary(matches!, matches![loaderIdx].route.id)
- : findNearestBoundary(matches!);
+
+ // Find the boundary at or above the source of the middleware
+ // error or the highest loader. We can't render any UI below
+ // the highest loader since we have no loader data available
+ let boundaryRouteId = skipLoaderErrorBubbling
+ ? routeId
+ : findNearestBoundary(
+ matches,
+ matches.find(
+ (m) => m.route.id === routeId || m.route.loader
+ )?.route.id || routeId
+ ).route.id;
return respond({
matches: matches!,
@@ -3635,7 +3642,7 @@ export function createStaticHandler(
loaderData: {},
actionData: null,
errors: {
- [boundary.route.id]: error,
+ [boundaryRouteId]: error,
},
statusCode: isRouteErrorResponse(error) ? error.status : 500,
actionHeaders: {},