refactor(server): replace hand-rolled withTimeout, merge web handlers, clean checks

- health.ts: swap hand-rolled withTimeout for es-toolkit's (fn-taking API); remove
  redundant inner try/catches from checkDatabase/checkStorage since runCheck catches
  all errors (findings 13, health cleanup)
- web.ts: merge handleWebApp/handleWebAppHead into one function; method is the only
  difference — isHead determines body presence (finding 14)
- app.ts: collapse two separate GET/HEAD wildcard routes into app.on(["GET","HEAD"])
- Update web.test.ts and app.test.ts to drop handleWebAppHead references

Claude-Session: https://claude.ai/code/session_012Bnvt1MghwHj4qQRxuQUGa
This commit is contained in:
Amruth Pillai
2026-07-04 20:39:14 +02:00
parent 0fb81ad772
commit f2ec6a499f
5 changed files with 24 additions and 61 deletions
-4
View File
@@ -19,7 +19,6 @@ const mocks = vi.hoisted(() => ({
handleLlms: vi.fn(),
serveWebDistStatic: vi.fn(),
handleWebApp: vi.fn(),
handleWebAppHead: vi.fn(),
}));
vi.mock("./auth", () => ({
@@ -60,7 +59,6 @@ vi.mock("../static/seo", () => ({
vi.mock("../static/web", () => ({
serveWebDistStatic: mocks.serveWebDistStatic,
handleWebApp: mocks.handleWebApp,
handleWebAppHead: mocks.handleWebAppHead,
}));
vi.mock("../mcp/handler", () => ({
@@ -91,7 +89,6 @@ beforeEach(() => {
mocks.handleLlms.mockReturnValue(new Response("llms"));
mocks.serveWebDistStatic.mockResolvedValue(undefined);
mocks.handleWebApp.mockResolvedValue(new Response("web"));
mocks.handleWebAppHead.mockReturnValue(new Response(null));
});
describe("createApp", () => {
@@ -138,6 +135,5 @@ describe("createApp", () => {
expect(handler).toHaveBeenCalledWith({ head: method === "HEAD" });
expect(mocks.serveWebDistStatic).not.toHaveBeenCalled();
expect(mocks.handleWebApp).not.toHaveBeenCalled();
expect(mocks.handleWebAppHead).not.toHaveBeenCalled();
});
});
+2 -3
View File
@@ -12,7 +12,7 @@ import { handleRpc } from "../rpc/handler";
import { handleSchemaJson } from "../static/schema";
import { handleLlms, handleRobots, handleSitemap } from "../static/seo";
import { handleUpload } from "../static/uploads";
import { handleWebApp, handleWebAppHead, serveWebDistStatic } from "../static/web";
import { handleWebApp, serveWebDistStatic } from "../static/web";
import { handleAuth, handleOAuth } from "./auth";
import { handleHealth } from "./health";
import { handleResumePdfDownload } from "./resume-pdf";
@@ -47,8 +47,7 @@ export function createApp() {
app.on(["GET", "HEAD"], "/llms.txt", (c) => handleLlms({ head: c.req.method === "HEAD" }));
app.use("/*", serveWebDistStatic);
app.on(["GET"], "/*", (c) => handleWebApp(c.req.raw));
app.on(["HEAD"], "/*", (c) => handleWebAppHead(c.req.raw));
app.on(["GET", "HEAD"], "/*", (c) => handleWebApp(c.req.raw));
return app;
}
+8 -39
View File
@@ -1,4 +1,5 @@
import { sql } from "drizzle-orm";
import { withTimeout } from "es-toolkit";
import { getStorageService } from "@reactive-resume/api/features/storage";
import { db } from "@reactive-resume/db/client";
@@ -11,30 +12,12 @@ type CheckResult = {
[key: string]: unknown;
};
function getErrorMessage(error: unknown): string {
if (error instanceof Error) return error.message;
return "Unknown error";
}
async function withTimeout<T>(promise: Promise<T>, timeoutMs: number): Promise<T> {
let timeoutId: NodeJS.Timeout | undefined;
const timeout = new Promise<never>((_, reject) => {
timeoutId = setTimeout(() => reject(new Error(`Timed out after ${timeoutMs}ms`)), timeoutMs);
});
try {
return await Promise.race([promise, timeout]);
} finally {
if (timeoutId) clearTimeout(timeoutId);
}
}
// ponytail: es-toolkit withTimeout takes a fn, not a promise — call site passes check (not check())
async function runCheck(check: () => Promise<object>): Promise<CheckResult> {
const startedAt = performance.now();
try {
const data = await withTimeout(check(), HEALTHCHECK_TIMEOUT_MS);
const data = await withTimeout(check, HEALTHCHECK_TIMEOUT_MS);
const latencyMs = Math.round(performance.now() - startedAt);
const result = data as { status?: string };
if (result.status === "unhealthy") return { ...(data as object), status: "unhealthy", latencyMs };
@@ -42,34 +25,20 @@ async function runCheck(check: () => Promise<object>): Promise<CheckResult> {
} catch (error) {
return {
status: "unhealthy",
error: getErrorMessage(error),
error: error instanceof Error ? error.message : "Unknown error",
latencyMs: Math.round(performance.now() - startedAt),
};
}
}
// ponytail: inner try/catches removed; runCheck's outer catch handles all errors
async function checkDatabase() {
try {
await db.execute(sql`SELECT 1`);
return { status: "healthy" };
} catch (error) {
return {
status: "unhealthy",
error: error instanceof Error ? error.message : "Unknown error",
};
}
await db.execute(sql`SELECT 1`);
return { status: "healthy" };
}
async function checkStorage() {
try {
const storageService = getStorageService();
return await storageService.healthcheck();
} catch (error) {
return {
status: "unhealthy",
error: error instanceof Error ? error.message : "Unknown error",
};
}
return getStorageService().healthcheck();
}
export async function handleHealth() {
+5 -3
View File
@@ -15,7 +15,7 @@ vi.mock("@hono/node-server/serve-static", () => ({
serveStatic: vi.fn(() => vi.fn()),
}));
const { handleWebApp, handleWebAppHead } = await import("./web");
const { handleWebApp } = await import("./web");
describe("web app fallback classification", () => {
beforeEach(() => {
@@ -90,8 +90,10 @@ describe("web app fallback classification", () => {
});
it("mirrors fallback status and headers for HEAD without a body", async () => {
const knownResponse = handleWebAppHead(new Request("https://example.com/dashboard"));
const unknownResponse = handleWebAppHead(new Request("https://example.com/unknown/extra/path"));
const knownResponse = await handleWebApp(new Request("https://example.com/dashboard", { method: "HEAD" }));
const unknownResponse = await handleWebApp(
new Request("https://example.com/unknown/extra/path", { method: "HEAD" }),
);
expect(knownResponse.status).toBe(200);
expect(knownResponse.headers.get("Content-Type")).toBe("text/html; charset=UTF-8");
+9 -12
View File
@@ -74,23 +74,20 @@ function notFoundResponse(options: { head?: boolean; noindex?: boolean } = {}) {
});
}
// ponytail: GET and HEAD share the same routing logic; method determines body presence
export async function handleWebApp(request: Request) {
const isHead = request.method === "HEAD";
const pathname = new URL(request.url).pathname;
if (!isNoindexShellPath(pathname) && isAssetPath(pathname)) return new Response("Not Found", { status: 404 });
if (!isNoindexShellPath(pathname) && isAssetPath(pathname)) {
return new Response(isHead ? null : "Not Found", { status: 404 });
}
const headers = getFallbackResponseHeaders(pathname);
if (!headers) return notFoundResponse({ noindex: true });
if (!headers) return notFoundResponse({ head: isHead, noindex: true });
if (isHead) return new Response(null, { status: 200, headers });
const html = await fs.readFile(indexHtmlPath, "utf-8");
return new Response(html, { headers });
}
export function handleWebAppHead(request: Request) {
const pathname = new URL(request.url).pathname;
if (!isNoindexShellPath(pathname) && isAssetPath(pathname)) return new Response(null, { status: 404 });
const headers = getFallbackResponseHeaders(pathname);
if (!headers) return notFoundResponse({ head: true, noindex: true });
return new Response(null, { status: 200, headers });
}