Fix middleware error bubbling scenarios (#13538)

Author: brophdawg11

.changeset/tough-dancers-own.md

@@ -0,0 +1,5 @@
+---
+"react-router": patch
+---
+
+UNSTABLE: Fix a few bugs with error bubbling in middleware use-cases

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 to="/a/b">Link</Link>
+                return <Link to="/a/b/c/d">Link</Link>
               }
             `,
             "app/routes/a.tsx": js`
@@ -2158,7 +2158,7 @@ test.describe("Middleware", () => {
                 return null;
               }
               export default function Component() {
-                return <Outlet/>
+                return <Outlet />
               }
             `,
             "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 (
+                  <>
+                    <Link to="/a/b">/a/b</Link>
+                    <br/>
+                    <Link to="/a/b/c/d">/a/b/c/d</Link>
+                  </>
+                );
+              }
+            `,
+            "app/routes/a.tsx": js`
+              import { Outlet } from 'react-router'
+              export default function Component() {
+                return <Outlet/>
+              }
+              export function ErrorBoundary({ error }) {
+                return <><h1 data-error-a>A Error Boundary</h1><pre>{error.message}</pre></>
+              }
+            `,
+            "app/routes/a.b.tsx": js`
+              import { Link, Outlet } from 'react-router'
+              export function loader() {
+                return { message: "DATA" };
+              }
+              export default function Component({ loaderData }) {
+                return (
+                  <>
+                    <h2 data-ab>AB: {loaderData.message}</h2>
+                    <Link to="/a/b/c/d">/a/b/c/d</Link>
+                    <Outlet/>
+                  </>
+                );
+              }
+              export function shouldRevalidate() {
+                return false;
+              }
+            `,
+            "app/routes/a.b.c.tsx": js`
+              import { Outlet } from 'react-router'
+              export default function Component() {
+                return <Outlet/>
+              }
+              export function ErrorBoundary({ error }) {
+                return <><h1 data-error-c>C Error Boundary</h1><pre>{error.message}</pre></>
+              }
+            `,
+            "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 <Outlet/>
+              }
+            `,
+          },
+        },
+        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();
     });
 

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<DecodedSingleFetchResults>,
+  matches: DataStrategyFunctionArgs["matches"],
+  routesParams: Set<string>,
+  results: Record<string, DataStrategyResult>
+) {
+  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}"`);
   }
 }
 

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: {},