From f02bd33a00a0f67604d6f5718f3c10a0a25addb3 Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Mon, 27 May 2024 18:42:23 +0200 Subject: [PATCH] fix: error handling in fs routes --- src/app_test.tsx | 16 +++++ src/context.ts | 6 +- src/plugins/fs_routes/mod.ts | 15 +++-- src/plugins/fs_routes/mod_test.tsx | 68 ++++++++++++++++++++++ src/plugins/fs_routes/render_middleware.ts | 21 ++++++- src/runtime/server/preact_hooks.tsx | 4 +- 6 files changed, 121 insertions(+), 9 deletions(-) diff --git a/src/app_test.tsx b/src/app_test.tsx index b77da4e5ef8..b6e413e3bf0 100644 --- a/src/app_test.tsx +++ b/src/app_test.tsx @@ -470,3 +470,19 @@ Deno.test("FreshApp - sets error on context", async () => { expect(thrown[0][0]).toEqual(thrown[0][1]); expect(thrown[1][0]).toEqual(thrown[1][1]); }); + +Deno.test("FreshApp - pass stuff in ctx.render()", async () => { + const app = new App<{ text: string }>() + .get("/", (ctx) => { + return ctx.render(
ok
, { + status: 416, + headers: { "X-Foo": "foo" }, + }); + }); + + const server = new FakeServer(await app.handler()); + const res = await server.get("/"); + await res.body?.cancel(); + expect(res.status).toEqual(416); + expect(res.headers.get("X-Foo")).toEqual("foo"); +}); diff --git a/src/context.ts b/src/context.ts index 5c4fb5bebf5..88e6c5dda01 100644 --- a/src/context.ts +++ b/src/context.ts @@ -149,7 +149,11 @@ export class FreshReqContext implements FreshContext { : new Headers(); headers.set("Content-Type", "text/html; charset=utf-8"); - const responseInit: ResponseInit = { status: init.status ?? 200, headers }; + const responseInit: ResponseInit = { + status: init.status ?? 200, + headers, + statusText: init.statusText, + }; let partialId = ""; if (this.url.searchParams.has("fresh-partial")) { diff --git a/src/plugins/fs_routes/mod.ts b/src/plugins/fs_routes/mod.ts index 19ea5ef6c24..1d3fb2ce253 100644 --- a/src/plugins/fs_routes/mod.ts +++ b/src/plugins/fs_routes/mod.ts @@ -161,6 +161,8 @@ export async function fsRoutes( const stack: InternalRoute[] = []; let hasApp = false; + const specialPaths = new Set(); + for (let i = 0; i < routeModules.length; i++) { const routeMod = routeModules[i]; const normalized = routeMod.path; @@ -251,10 +253,15 @@ export async function fsRoutes( } let parent = mod.path.slice(0, -"_error".length); parent = parent === "/" ? "*" : parent + "*"; - app.all( - parent, - errorMiddleware(errorComponents, handler), - ); + + // Add error route as it's own route + if (!specialPaths.has(mod.path)) { + specialPaths.add(mod.path); + app.all( + parent, + errorMiddleware(errorComponents, handler), + ); + } middlewares.push(errorMiddleware(errorComponents, handler)); continue; } diff --git a/src/plugins/fs_routes/mod_test.tsx b/src/plugins/fs_routes/mod_test.tsx index 262e377ba54..693b2aa6361 100644 --- a/src/plugins/fs_routes/mod_test.tsx +++ b/src/plugins/fs_routes/mod_test.tsx @@ -941,6 +941,74 @@ Deno.test("fsRoutes - async route components returning response", async () => { expect(text).toEqual("index"); }); +Deno.test( + "fsRoutes - returns response code from error route", + async () => { + const server = await createServer<{ text: string }>({ + "routes/_app.tsx": { + default: (ctx) => { + return ( +
+ _app/ +
+ ); + }, + }, + "routes/_error.tsx": { + default: () =>
fail
, + }, + "routes/index.tsx": { + default: () =>
index
, + }, + "routes/bar.tsx": { + default: () =>
index
, + }, + "routes/foo/index.tsx": { + default: () =>
foo/index
, + }, + "routes/foo/_error.tsx": { + default: () => { + throw new Error("fail"); + }, + }, + "routes/foo/bar.tsx": { + default: () =>
foo/index
, + }, + }); + + let res = await server.get("/fooa"); + await res.body?.cancel(); + expect(res.status).toEqual(404); + + res = await server.get("/foo/asdf"); + await res.body?.cancel(); + expect(res.status).toEqual(500); + }, +); + +Deno.test( + "fsRoutes - set headers from handler", + async () => { + const server = await createServer<{ text: string }>({ + "routes/index.tsx": { + handler: (ctx) => { + return ctx.render(

hello

, { + headers: { "X-Foo": "123" }, + status: 418, + statusText: "I'm a fresh teapot", + }); + }, + }, + }); + + const res = await server.get("/"); + await res.body?.cancel(); + expect(res.status).toEqual(418); + expect(res.statusText).toEqual("I'm a fresh teapot"); + expect(res.headers.get("X-Foo")).toEqual("123"); + }, +); + Deno.test("fsRoutes - sortRoutePaths", () => { let routes = [ "/foo/[id]", diff --git a/src/plugins/fs_routes/render_middleware.ts b/src/plugins/fs_routes/render_middleware.ts index 85a39286cab..c41207b93de 100644 --- a/src/plugins/fs_routes/render_middleware.ts +++ b/src/plugins/fs_routes/render_middleware.ts @@ -2,6 +2,7 @@ import { type AnyComponent, h, type RenderableProps, type VNode } from "preact"; import type { MiddlewareFn } from "../../middlewares/mod.ts"; import type { HandlerFn, PageResponse } from "../../handlers.ts"; import type { PageProps } from "../../runtime/server/mod.tsx"; +import { HttpError } from "../../error.ts"; export type AsyncAnyComponent

= { ( @@ -20,6 +21,7 @@ export function renderMiddleware( | AsyncAnyComponent> >, handler: HandlerFn | undefined, + init?: ResponseInit | undefined, ): MiddlewareFn { return async (ctx) => { let result: PageResponse | undefined; @@ -69,6 +71,23 @@ export function renderMiddleware( } } - return ctx.render(vnode!); + let status: number | undefined = init?.status; + if ( + ctx.error !== null && ctx.error !== undefined + ) { + if ( + ctx.error instanceof HttpError + ) { + status = ctx.error.status; + } else { + status = 500; + } + } + + return ctx.render(vnode!, { + status, + statusText: init?.statusText, + headers: init?.headers, + }); }; } diff --git a/src/runtime/server/preact_hooks.tsx b/src/runtime/server/preact_hooks.tsx index 74a9fadc478..6acbe3482c9 100644 --- a/src/runtime/server/preact_hooks.tsx +++ b/src/runtime/server/preact_hooks.tsx @@ -26,9 +26,7 @@ import { import type { BuildCache } from "../../build_cache.ts"; import { BUILD_ID } from "../build_id.ts"; import { DEV_ERROR_OVERLAY_URL } from "../../constants.ts"; -import { - getCodeFrame, -} from "../../dev/middlewares/error_overlay/code_frame.tsx"; +import { getCodeFrame } from "../../code_frame.tsx"; import * as colors from "@std/fmt/colors"; import { escape as escapeHtml } from "@std/html"; import { HttpError } from "../../error.ts";