diff --git a/apps/hub/src/hub-error-handler.test.ts b/apps/hub/src/hub-error-handler.test.ts index fc007dddb..67d6d3537 100644 --- a/apps/hub/src/hub-error-handler.test.ts +++ b/apps/hub/src/hub-error-handler.test.ts @@ -1,38 +1,39 @@ // Exercises `hubErrorHandler` through a real Hono app rather than a // hand-rolled `Context` double, so the assertions cover exactly what -// `app.onError` actually receives and returns. -import { describe, expect, test } from "bun:test"; +// `app.onError` actually receives and returns. Errors are reported +// through `@corbits/error-sink`'s real `reportError`, captured the same +// way `packages/error-sink/src/index.test.ts` captures its own sink. +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { Hono } from "hono"; -import type { getLogger } from "@intx/log"; +import { configureSync, resetSync } from "@intx/log"; import { hubErrorHandler } from "./hub-error-handler"; -/** A minimal stand-in for logtape's tagged-template `Logger`, capturing - * every `.error` call's rendered message for assertions. */ -function fakeLogger(): { - logger: ReturnType; - messages: string[]; -} { - const messages: string[] = []; - const logger = { - error: (strings: TemplateStringsArray, ...values: unknown[]) => { - messages.push( - strings.reduce( - (acc, part, i) => - acc + part + (i < values.length ? String(values[i]) : ""), - "", - ), - ); +let records: { properties: Record }[]; + +function installCapturingSink(): void { + records = []; + configureSync({ + reset: true, + sinks: { + capture: (record) => { + records.push(record as { properties: Record }); + }, }, - } as unknown as ReturnType; - return { logger, messages }; + loggers: [ + { category: ["errors"], sinks: ["capture"], lowestLevel: "debug" }, + { category: ["logtape", "meta"], sinks: [], lowestLevel: "warning" }, + ], + }); } +beforeEach(() => installCapturingSink()); +afterEach(() => resetSync()); + describe("hubErrorHandler", () => { - test("logs the failing route and answers a generic 500 for an ordinary error", async () => { - const { logger, messages } = fakeLogger(); + test("reports the failing route through reportError and answers a generic 500 with a refId", async () => { const app = new Hono(); - app.onError(hubErrorHandler(logger)); + app.onError(hubErrorHandler()); app.get("/boom", () => { throw new Error("definition asset never materialized"); }); @@ -40,17 +41,23 @@ describe("hubErrorHandler", () => { const res = await app.request("/boom"); expect(res.status).toBe(500); - const body = (await res.json()) as { error: { code: string } }; + const body = (await res.json()) as { + error: { code: string; refId: string }; + }; expect(body.error.code).toBe("internal_error"); - expect(messages.length).toBe(1); - expect(messages[0]).toContain("/boom"); - expect(messages[0]).toContain("definition asset never materialized"); + expect(typeof body.error.refId).toBe("string"); + expect(body.error.refId.length).toBeGreaterThan(0); + + expect(records).toHaveLength(1); + const properties = records[0]?.properties; + expect(properties?.operation).toBe("hub.unhandled_route_error"); + expect(properties?.refId).toBe(body.error.refId); + expect(properties?.extra).toEqual({ path: "/boom", method: "GET" }); }); - test("maps a guidance-bearing error to a 422 with its own message", async () => { - const { logger } = fakeLogger(); + test("maps a guidance-bearing error to a 422 with its own message and a refId", async () => { const app = new Hono(); - app.onError(hubErrorHandler(logger)); + app.onError(hubErrorHandler()); app.get("/launch", () => { class NamedLaunchError extends Error { readonly guidance = "Reduce it to a single step and try again."; @@ -66,11 +73,15 @@ describe("hubErrorHandler", () => { expect(res.status).toBe(422); const body = (await res.json()) as { - error: { code: string; message: string }; + error: { code: string; message: string; refId: string }; }; expect(body.error.code).toBe("MultiStepFoldUnsupportedError"); expect(body.error.message).toBe( "definition wfd_research is not single-step (2 steps)", ); + expect(typeof body.error.refId).toBe("string"); + expect(body.error.refId.length).toBeGreaterThan(0); + expect(records).toHaveLength(1); + expect(records[0]?.properties.refId).toBe(body.error.refId); }); }); diff --git a/apps/hub/src/hub-error-handler.ts b/apps/hub/src/hub-error-handler.ts index dd549781e..e2bc0b3bb 100644 --- a/apps/hub/src/hub-error-handler.ts +++ b/apps/hub/src/hub-error-handler.ts @@ -5,10 +5,11 @@ // workflow's launch body failing to read, a definition asset that never // materialized — stays invisible until someone reports it from the UI. // This handler is the one place every such exception is guaranteed to be -// logged, and it maps a named consumer-facing error to a real 4xx rather +// reported, and it maps a named consumer-facing error to a real 4xx rather // than folding it into a generic message. import type { Context } from "hono"; -import { getLogger } from "@intx/log"; +import type { TenantEnv } from "@intx/hub-api"; +import { reportError } from "@corbits/error-sink"; /** * Duck-typed rather than an `instanceof` allowlist: any error carrying a @@ -28,17 +29,32 @@ function hasGuidance(err: unknown): err is Error & { guidance: string } { } /** - * Builds the handler passed to `app.onError`. Takes the logger as a - * parameter (rather than constructing one internally) so a test can - * inject a fake and assert on what got logged. + * `app.onError` runs for routes mounted both inside and outside the + * platform's tenant middleware, so `c`'s `Variables` aren't statically + * known here; this borrows the same `TenantEnv` a tenant-scoped route + * types `c.get("tenant")` with (see `packages/access-policy/src/routes.ts`) + * to read `tenant.id` when a tenant-scoped route set it, without claiming + * the wider type up front. */ -export function hubErrorHandler(log: ReturnType) { +function extractTenantId(c: Context): string | undefined { + return (c as unknown as Context).var.tenant?.id; +} + +/** Builds the handler passed to `app.onError`. */ +export function hubErrorHandler() { return (err: unknown, c: Context): Response | Promise => { - const message = err instanceof Error ? err.message : String(err); - log.error`Unhandled error on ${c.req.method} ${c.req.path}: ${message}`; + const tenantId = extractTenantId(c); + const refId = reportError(err, { + operation: "hub.unhandled_route_error", + ...(tenantId !== undefined ? { tenantId } : {}), + extra: { path: c.req.path, method: c.req.method }, + }); if (hasGuidance(err)) { - return c.json({ error: { code: err.name, message: err.message } }, 422); + return c.json( + { error: { code: err.name, message: err.message, refId } }, + 422, + ); } return c.json( @@ -46,6 +62,7 @@ export function hubErrorHandler(log: ReturnType) { error: { code: "internal_error", message: "Something went wrong. Please try again.", + refId, }, }, 500, diff --git a/apps/hub/src/index.ts b/apps/hub/src/index.ts index 483887d8e..049f618ec 100644 --- a/apps/hub/src/index.ts +++ b/apps/hub/src/index.ts @@ -1101,8 +1101,8 @@ export async function createHub(config: HubConfig) { // Without this, any exception escaping a route (extension or platform // alike) falls through to Hono's built-in handler: a bare 500 with - // nothing logged. See `hubErrorHandler`'s own doc comment. - app.onError(hubErrorHandler(getLogger(["hub", "error"]))); + // nothing reported. See `hubErrorHandler`'s own doc comment. + app.onError(hubErrorHandler()); // Extension routes mount under the tenant prefix, inside the // platform's native tenant middleware, so every extension handler