From 78635914c1b998f62cf95aa03b47375e0c74b883 Mon Sep 17 00:00:00 2001 From: Kyle June Date: Thu, 29 Jan 2026 22:19:37 -0500 Subject: [PATCH] fix: Fix not found error handling and rendering errors server side --- deno.lock | 10 +-- src/_server.tsx | 35 ++++++----- src/client.test.tsx | 4 +- src/client.tsx | 15 +++++ src/server.test.tsx | 148 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 189 insertions(+), 23 deletions(-) diff --git a/deno.lock b/deno.lock index a45cda0..8492715 100644 --- a/deno.lock +++ b/deno.lock @@ -2308,7 +2308,7 @@ "jsr:@std/testing@^1.0.17", "jsr:@std/uuid@^1.1.0", "jsr:@udibo/esbuild-plugin-postcss@0.3", - "jsr:@udibo/juniper@~0.2.2", + "jsr:@udibo/juniper@0.3", "npm:@opentelemetry/api@^1.9.0", "npm:@tailwindcss/postcss@^4.1.18", "npm:@testing-library/react@^16.3.2", @@ -2358,7 +2358,7 @@ "jsr:@std/path@^1.1.4", "jsr:@std/streams@^1.0.17", "jsr:@std/testing@^1.0.17", - "jsr:@udibo/juniper@~0.2.2", + "jsr:@udibo/juniper@0.3", "npm:@opentelemetry/api@^1.9.0", "npm:@testing-library/react@^16.3.2", "npm:@types/react@^19.2.9", @@ -2374,7 +2374,7 @@ "jsr:@std/streams@^1.0.17", "jsr:@std/testing@^1.0.17", "jsr:@udibo/esbuild-plugin-postcss@0.3.0", - "jsr:@udibo/juniper@~0.2.2", + "jsr:@udibo/juniper@0.3", "npm:@opentelemetry/api@^1.9.0", "npm:@tailwindcss/postcss@^4.1.18", "npm:@testing-library/react@^16.3.2", @@ -2392,7 +2392,7 @@ "jsr:@std/path@^1.1.4", "jsr:@std/streams@^1.0.17", "jsr:@std/testing@^1.0.17", - "jsr:@udibo/juniper@~0.2.2", + "jsr:@udibo/juniper@0.3", "npm:@opentelemetry/api@^1.9.0", "npm:@tanstack/react-query@^5.90.20", "npm:@testing-library/react@^16.3.2", @@ -2409,7 +2409,7 @@ "jsr:@std/path@^1.1.4", "jsr:@std/streams@^1.0.17", "jsr:@std/testing@^1.0.17", - "jsr:@udibo/juniper@~0.2.2", + "jsr:@udibo/juniper@0.3", "npm:@opentelemetry/api@^1.9.0", "npm:@testing-library/react@^16.3.2", "npm:@types/react@^19.2.9", diff --git a/src/_server.tsx b/src/_server.tsx index 9daa4d1..cbad763 100644 --- a/src/_server.tsx +++ b/src/_server.tsx @@ -451,11 +451,10 @@ async function renderDocument( async function createErrorContext( error: Error, - dataRoutes: DataRouteObject[], + query: ReturnType["query"], request: Request, requestContext: RouterContextProvider, ): Promise { - const { query } = createStaticHandler(dataRoutes as unknown as RouteObject[]); const contextOrResponse = await query(request, { requestContext }); if (contextOrResponse instanceof Response) { @@ -465,19 +464,13 @@ async function createErrorContext( const context = contextOrResponse; if (!context.errors) context.errors = {}; - let errorRouteId = "/"; - for (let i = context.matches.length - 1; i >= 0; i--) { - const match = context.matches[i]; - const route = match.route; - if (route.hasErrorBoundary) { - errorRouteId = route.id ?? "/"; - break; - } - } - context.errors[errorRouteId] = error; + const routeContext = getRouteContext(); + if (routeContext?.routeId) { + context.errors[routeContext.routeId] ??= error; - if (context.loaderData) { - delete context.loaderData[errorRouteId]; + if (context.loaderData) { + delete context.loaderData[routeContext.routeId]; + } } return context; @@ -813,7 +806,7 @@ export function createHandlers< const requestContext = c.get("context"); const contextOrResponse = await createErrorContext( error, - dataRoutes, + query, c.req.raw, requestContext, ); @@ -828,7 +821,7 @@ export function createHandlers< dataRoutes, renderOptions, request: c.req.raw, - waitForAllReady: true, + waitForAllReady: isbot(c.req.header("user-agent")), presetError: error, }); } catch (renderError) { @@ -892,7 +885,15 @@ export function buildApp< ); } - routeApp.use(async (_c, next) => { + routeApp.use(async (c, next) => { + // Index routes should only set context for exact path matches + if (currentRouteId.endsWith("/index")) { + const parentPath = currentRouteId.slice(0, -6) || "/"; + const path = new URL(c.req.url).pathname; + if (path !== parentPath) { + return await next(); + } + } await routeContextStorage.run( { routeId: currentRouteId, isClientRoute: true }, async () => { diff --git a/src/client.test.tsx b/src/client.test.tsx index afe4d0e..4fa96f7 100644 --- a/src/client.test.tsx +++ b/src/client.test.tsx @@ -93,7 +93,9 @@ describe("Client", () => { assertEquals(client.rootRoute.path, "/"); assertEquals(client.routeFileMap.size, 8); - assertEquals(client.routeObjectMap.size, 8); + // 12 = 8 explicit routes + 4 implicit catchalls for routes without their own catchall + // Implicit catchalls: /about/[...], /blog/[...], /blog/create/[...], /blog/[id]/[...] + assertEquals(client.routeObjectMap.size, 12); assertEquals(client.routeObjects.length, 1); const routeObject = client.routeObjects[0]; diff --git a/src/client.tsx b/src/client.tsx index b0d0614..0654646 100644 --- a/src/client.tsx +++ b/src/client.tsx @@ -11,6 +11,7 @@ import { RouterProvider, } from "react-router"; import type { RouteObject } from "react-router"; +import { HttpError } from "@udibo/http-error"; import type { HtmlProps, RootRouteModule, RouteModule } from "./mod.ts"; import { @@ -213,6 +214,20 @@ export class Client { this.routeFileMap.set(catchallRouteId, route.catchall); this.routeObjectMap.set(catchallRouteId, catchallRouteObject); + } else { + // Add default catchall that throws 404 for unmatched routes + // This ensures errors bubble to the nearest ErrorBoundary + const catchallRouteId = generateRouteId(currentPath, "", "catchall"); + const catchallRouteObject: RouteObject = { + id: catchallRouteId, + path: "*", + loader: () => { + throw new HttpError(404, "Not found"); + }, + Component: () =>
, + }; + routeObjectChildren.push(catchallRouteObject); + this.routeObjectMap.set(catchallRouteId, catchallRouteObject); } if (routeObjectChildren.length > 0) { diff --git a/src/server.test.tsx b/src/server.test.tsx index efcb821..8a210c4 100644 --- a/src/server.test.tsx +++ b/src/server.test.tsx @@ -456,6 +456,154 @@ describe("createServer", () => { const html = await res.text(); assertStringIncludes(html, ""); }); + + it("should use nested error boundary for 404s within nested routes", async () => { + const client = new Client({ + path: "/", + main: { + default: () => , + ErrorBoundary: () =>
Root Error Boundary
, + }, + children: [ + { + path: "admin", + main: { + default: () => , + ErrorBoundary: () =>
Admin Error Boundary
, + }, + children: [ + { + path: "users", + main: () => Promise.resolve({ default: () =>
Users
}), + }, + ], + }, + ], + }); + + const server = createServer(import.meta.url, client, { + path: "/", + children: [ + { + path: "admin", + children: [ + { + path: "users", + }, + ], + }, + ], + }); + + // Request to a non-existent path under /admin should use admin's error boundary + const res = await server.request("http://localhost/admin/non-existent"); + assertEquals(res.status, 404); + const html = await res.text(); + assertStringIncludes(html, ""); + assertStringIncludes(html, "
Admin Error Boundary
"); + // Should NOT use root error boundary + assertEquals(html.includes("Root Error Boundary"), false); + }); + + it("should use root error boundary for 404s at root level", async () => { + const client = new Client({ + path: "/", + main: { + default: () => , + ErrorBoundary: () =>
Root Error Boundary
, + }, + children: [ + { + path: "admin", + main: { + default: () => , + ErrorBoundary: () =>
Admin Error Boundary
, + }, + }, + ], + }); + + const server = createServer(import.meta.url, client, { + path: "/", + children: [ + { + path: "admin", + }, + ], + }); + + // Request to a non-existent path at root level should use root's error boundary + const res = await server.request("http://localhost/non-existent"); + assertEquals(res.status, 404); + const html = await res.text(); + assertStringIncludes(html, ""); + assertStringIncludes(html, "
Root Error Boundary
"); + // Should NOT use admin error boundary + assertEquals(html.includes("Admin Error Boundary"), false); + }); + + it("should find nearest ancestor error boundary when route has no error boundary", async () => { + const client = new Client({ + path: "/", + main: { + default: () => , + ErrorBoundary: () =>
Root Error Boundary
, + }, + children: [ + { + path: "admin", + main: { + default: () => , + ErrorBoundary: () =>
Admin Error Boundary
, + }, + children: [ + { + path: "settings", + // No error boundary on settings + main: { + default: () => , + }, + children: [ + { + path: "profile", + main: () => + Promise.resolve({ default: () =>
Profile
}), + }, + ], + }, + ], + }, + ], + }); + + const server = createServer(import.meta.url, client, { + path: "/", + children: [ + { + path: "admin", + children: [ + { + path: "settings", + children: [ + { + path: "profile", + }, + ], + }, + ], + }, + ], + }); + + // Request to non-existent path under /admin/settings should bubble up to admin's error boundary + const res = await server.request( + "http://localhost/admin/settings/non-existent", + ); + assertEquals(res.status, 404); + const html = await res.text(); + assertStringIncludes(html, ""); + assertStringIncludes(html, "
Admin Error Boundary
"); + }); }); describe("mergeServerRoutes", () => {