hubErrorHandler: report unhandled route errors through reportError

apps/hub/src/hub-error-handler.test.ts

@@ -1,56 +1,63 @@
 // 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<typeof getLogger>;
-  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<string, unknown> }[];
+
+function installCapturingSink(): void {
+  records = [];
+  configureSync({
+    reset: true,
+    sinks: {
+      capture: (record) => {
+        records.push(record as { properties: Record<string, unknown> });
+      },
     },
-  } as unknown as ReturnType<typeof getLogger>;
-  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");
     });
 
     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);
   });
 });

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,24 +29,40 @@ 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<typeof getLogger>) {
+function extractTenantId(c: Context): string | undefined {
+  return (c as unknown as Context<TenantEnv>).var.tenant?.id;
+}
+
+/** Builds the handler passed to `app.onError`. */
+export function hubErrorHandler() {
   return (err: unknown, c: Context): Response | Promise<Response> => {
-    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(
       {
         error: {
           code: "internal_error",
           message: "Something went wrong. Please try again.",
+          refId,
         },
       },
       500,

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