diff --git a/.changeset/createrequesthandler-drop-logger.md b/.changeset/createrequesthandler-drop-logger.md new file mode 100644 index 00000000..39ccbcf1 --- /dev/null +++ b/.changeset/createrequesthandler-drop-logger.md @@ -0,0 +1,6 @@ +--- +"@farbenmeer/tapi": minor +"@farbenmeer/bunny": patch +--- + +Drop the `logger` option from `createRequestHandler`. Configure the logger on the API definition instead via `defineApi({ logger })`. Bunny installs a default logger on the API when one isn't provided. diff --git a/packages/1-tapi/src/client/cache.ts b/packages/1-tapi/src/client/cache.ts index 0c4239a3..15e6552a 100644 --- a/packages/1-tapi/src/client/cache.ts +++ b/packages/1-tapi/src/client/cache.ts @@ -1,4 +1,5 @@ import { EXPIRES_AT_HEADER, TAGS_HEADER } from "../shared/constants.js"; +import type { Logger } from "../shared/logger.js"; import type { Observable } from "./client-types.js"; import { handleResponse } from "./handle-response.js"; @@ -18,14 +19,10 @@ interface CacheEntry { timeout: ReturnType | null; } -interface Hooks { - error: (error: unknown) => void | Promise; -} - interface Options { minTTL?: number; maxOverdueTTL?: number; - hooks?: Partial; + logger?: Logger; } const DEFAULT_MIN_TTL = 5 * 1000; @@ -36,14 +33,12 @@ export class Cache { private tagIndex = new Map>(); private minTTL: number; private maxOverdueTTL: number; - private hooks: Hooks; + private errorLog: (error: unknown) => void | Promise; constructor(options: Options) { this.minTTL = options.minTTL ?? DEFAULT_MIN_TTL; this.maxOverdueTTL = options.maxOverdueTTL ?? DEFAULT_MAX_OVERDUE_TTL; - this.hooks = { - error: options.hooks?.error ?? console.error, - }; + this.errorLog = options.logger?.error ?? console.error; } private emplace(url: string, fetch: () => Promise) { @@ -172,7 +167,7 @@ export class Cache { } // there is a current valid value, we'll keep that an drop the error entry.next = undefined; - await this.hooks.error(error); + await this.errorLog(error); } finally { if (entry.subscriptions.size === 0) { // clear response after minTTL if no subscribers active diff --git a/packages/1-tapi/src/client/create-fetch-client.ts b/packages/1-tapi/src/client/create-fetch-client.ts index b2876ed3..af88a851 100644 --- a/packages/1-tapi/src/client/create-fetch-client.ts +++ b/packages/1-tapi/src/client/create-fetch-client.ts @@ -3,6 +3,7 @@ import { INVALIDATIONS_ROUTE, TAGS_HEADER, } from "../shared/constants.js"; +import type { Logger } from "../shared/logger.js"; import type { MaybePromise } from "../shared/maybe-promise.js"; import type { Path as BasePath } from "../shared/path.js"; import type { BaseRoute } from "../shared/route.js"; @@ -40,15 +41,11 @@ async function listenForInvalidations(url: string, cache: Cache) { } } -interface Hooks { - error?: (error: unknown) => void | Promise; -} - interface Options { fetch?: (url: string, init: RequestInit) => Promise; minTTL?: number; maxOverdueTTL?: number; - hooks?: Hooks; + logger?: Logger; invalidationsUrl?: string | false; } @@ -60,7 +57,7 @@ export function createFetchClient< const cache = new Cache({ minTTL: options.minTTL, maxOverdueTTL: options.maxOverdueTTL, - hooks: options.hooks, + logger: options.logger, }); const invalidationsUrl = @@ -138,7 +135,7 @@ export function createFetchClient< res .then((res) => handleResponse(res)) .catch(async (error) => { - await options?.hooks?.error?.(error); + await options?.logger?.error?.(error); throw error; }), { diff --git a/packages/1-tapi/src/client/index.ts b/packages/1-tapi/src/client/index.ts index 394b9abc..4585acd3 100644 --- a/packages/1-tapi/src/client/index.ts +++ b/packages/1-tapi/src/client/index.ts @@ -3,3 +3,4 @@ export { createFetchClient } from "./create-fetch-client.js"; export type { GetRoute, PostRoute } from "./route-types.js"; export { HttpError } from "../shared/http-error.js"; export { INVALIDATIONS_ROUTE } from "../shared/constants.js"; +export type { Logger } from "../shared/logger.js"; diff --git a/packages/1-tapi/src/server/create-request-handler.test.ts b/packages/1-tapi/src/server/create-request-handler.test.ts index f6d5d7f1..4f8239fa 100644 --- a/packages/1-tapi/src/server/create-request-handler.test.ts +++ b/packages/1-tapi/src/server/create-request-handler.test.ts @@ -69,7 +69,7 @@ describe("createRequestHandler", () => { test("returns 500 for arbitrary errors in handler", async () => { const errorHook = vi.fn(); const sut = createRequestHandler( - defineApi().route("/", { + defineApi({ logger: { error: errorHook } }).route("/", { GET: defineHandler( { authorize: () => true, @@ -79,7 +79,6 @@ describe("createRequestHandler", () => { }, ), }), - { hooks: { error: errorHook } }, ); const response = await sut(new Request("http://localhost:3000")); expect(response.status).toBe(500); @@ -89,7 +88,7 @@ describe("createRequestHandler", () => { test("returns 400 and zod issues for validation errors", async () => { const errorHook = vi.fn(); const sut = createRequestHandler( - defineApi().route("/", { + defineApi({ logger: { error: errorHook } }).route("/", { POST: defineHandler( { authorize: () => true, @@ -103,7 +102,6 @@ describe("createRequestHandler", () => { }, ), }), - { hooks: { error: errorHook } }, ); const response = await sut( new Request("http://localhost:3000", { @@ -156,7 +154,7 @@ describe("createRequestHandler", () => { test("auth data is available on the request object", async () => { const errorHook = vi.fn(); const sut = createRequestHandler( - defineApi().route("/", { + defineApi({ logger: { error: errorHook } }).route("/", { GET: defineHandler( { authorize: () => "foo", @@ -166,7 +164,6 @@ describe("createRequestHandler", () => { }, ), }), - { hooks: { error: errorHook } }, ); const response = await sut(new Request("http://localhost:3000")); expect(await response.json()).toEqual({ auth: "foo" }); diff --git a/packages/1-tapi/src/server/create-request-handler.ts b/packages/1-tapi/src/server/create-request-handler.ts index bc5816c8..f917c274 100644 --- a/packages/1-tapi/src/server/create-request-handler.ts +++ b/packages/1-tapi/src/server/create-request-handler.ts @@ -17,9 +17,6 @@ import { streamRevalidatedTags } from "./revalidation-stream.js"; interface Options { /** the root path for all API routes */ basePath?: string; - hooks?: { - error?: (error: unknown) => MaybePromise; - }; /** the default maximum time-to-live (TTL) for cached responses */ defaultTTL?: number; } @@ -37,11 +34,10 @@ export function createRequestHandler( api: ApiDefinition>>, options: Options = {}, ) { - const errorHook = - options.hooks?.error ?? + const errorLog = + api.logger?.error ?? ((error) => { console.error(error); - return error; }); const basePath = options.basePath ?? ""; @@ -93,7 +89,7 @@ export function createRequestHandler( } } catch (error) { // caches errors while retrieving from cache - errorHook(error); + errorLog(error); } const handler = route[req.method]; @@ -124,17 +120,17 @@ export function createRequestHandler( tags: res.cache.tags ?? [], }) // catches errors while caching if cache.set is async (redis cache) - .catch(errorHook); + .catch(errorLog); } catch (error) { // catches errors while caching if cache.set is sync (in-memory cache) - errorHook(error); + errorLog(error); } } } return res; } catch (error) { // catches errors while actually handling the request - await errorHook(error); + await errorLog(error); return handleError(error); } } @@ -161,14 +157,14 @@ export function createRequestHandler( res.cache.tags, clientId ? { clientId: clientId.value } : undefined, ) - .catch(errorHook); + .catch(errorLog); } catch (error) { - errorHook(error); + errorLog(error); } } return res; } catch (error) { - await errorHook(error); + await errorLog(error); return handleError(error); } } @@ -200,14 +196,14 @@ export function createRequestHandler( res.cache.tags, clientId ? { clientId: clientId.value } : undefined, ) - .catch(errorHook); + .catch(errorLog); } catch (error) { - errorHook(error); + errorLog(error); } } return res; } catch (error) { - await errorHook(error); + await errorLog(error); return handleError(error); } } diff --git a/packages/1-tapi/src/server/define-api.ts b/packages/1-tapi/src/server/define-api.ts index 03e0c0d1..35a4815d 100644 --- a/packages/1-tapi/src/server/define-api.ts +++ b/packages/1-tapi/src/server/define-api.ts @@ -1,20 +1,23 @@ import type { MaybePromise } from "../shared/maybe-promise.js"; +import type { Logger } from "../shared/logger.js"; import type { Path as BasePath, StrictParams } from "../shared/path.js"; import type { Route } from "../shared/route.js"; import { type Cache, PubSub } from "./cache.js"; interface Options { cache?: Cache; + logger?: Logger; } export function defineApi(options: Options = {}) { - return new ApiDefinition({}, options?.cache ?? new PubSub()); + return new ApiDefinition({}, options?.cache ?? new PubSub(), options?.logger); } export class ApiDefinition> { constructor( public routes: Routes, public cache: Cache, + public logger?: Logger, ) {} async invalidate(tags: string[]) { diff --git a/packages/1-tapi/src/server/index.ts b/packages/1-tapi/src/server/index.ts index 8b8d80be..c5c8d024 100644 --- a/packages/1-tapi/src/server/index.ts +++ b/packages/1-tapi/src/server/index.ts @@ -11,3 +11,4 @@ export type { CookieStore } from "./cookie-store.js"; export { streamRevalidatedTags } from "./revalidation-stream.js"; export { PubSub, type Cache } from "./cache.js"; export { INVALIDATIONS_ROUTE } from "../shared/constants.js"; +export type { Logger } from "../shared/logger.js"; diff --git a/packages/1-tapi/src/shared/logger.ts b/packages/1-tapi/src/shared/logger.ts new file mode 100644 index 00000000..3852f698 --- /dev/null +++ b/packages/1-tapi/src/shared/logger.ts @@ -0,0 +1,5 @@ +import type { MaybePromise } from "./maybe-promise.js"; + +export interface Logger { + error?: (error: unknown) => MaybePromise; +} diff --git a/packages/1-tapi/src/worker/index.ts b/packages/1-tapi/src/worker/index.ts index 354b309a..18a24a76 100644 --- a/packages/1-tapi/src/worker/index.ts +++ b/packages/1-tapi/src/worker/index.ts @@ -1,12 +1,16 @@ import { isMutation } from "../shared/is-mutation"; +import type { Logger } from "../shared/logger"; import { getCachedEntry, getMetadata } from "./cache"; import { mutateAndInvalidate } from "./mutate-and-invalidate"; import { serveFromNetwork } from "./serve-from-network"; export { listenForInvalidations } from "./revalidation-stream"; export { cleanup } from "./cleanup"; export type { CleanupOptions } from "./cleanup"; +export type { Logger } from "../shared/logger"; + +export async function handleTapiRequest(req: Request, options?: { logger?: Logger }) { + const errorLog = options?.logger?.error ?? ((err: unknown) => console.error("TApi Worker fetch failed", err)); -export async function handleTapiRequest(req: Request) { if (isMutation(req)) { return mutateAndInvalidate(req); } else { @@ -30,7 +34,7 @@ export async function handleTapiRequest(req: Request) { return serveFromNetwork(req); } catch (error) { // probably network not available, serve old response - console.error("TApi Worker fetch failed", error); + errorLog(error); return cachedResponse; } } diff --git a/packages/3-bunny/src/cli/dev.ts b/packages/3-bunny/src/cli/dev.ts index 7102574a..de27236d 100644 --- a/packages/3-bunny/src/cli/dev.ts +++ b/packages/3-bunny/src/cli/dev.ts @@ -13,6 +13,7 @@ import path from "node:path"; import { createServer } from "vite"; import react from "@vitejs/plugin-react"; import { loadEnv } from "../load-env.js"; +import { bunnyLogger } from "../server/bunny-logger.js"; import { fromResponse, toRequest } from "../server/node-http-adapter.js"; import { readConfig } from "./read-config.js"; import { parseURL } from "../server/parse-url.js"; @@ -70,14 +71,10 @@ export const dev = new Command() async function reload(entryPoint: string) { console.log("Loading", entryPoint); const { api } = await import(entryPoint); + if (!api.logger) api.logger = bunnyLogger; tapi.api = api; tapi.apiRequestHandler = createRequestHandler(api, { basePath: "/api", - hooks: { - error: (error) => { - console.error(error); - }, - }, }); const packageJson = JSON.parse( await readFile(path.join(process.cwd(), "package.json"), "utf8"), diff --git a/packages/3-bunny/src/server/bunny-logger.ts b/packages/3-bunny/src/server/bunny-logger.ts new file mode 100644 index 00000000..bc2d6063 --- /dev/null +++ b/packages/3-bunny/src/server/bunny-logger.ts @@ -0,0 +1,7 @@ +import type { Logger } from "@farbenmeer/tapi/server"; + +export const bunnyLogger: Logger = { + error: (error) => { + console.error(error); + }, +}; diff --git a/packages/3-bunny/src/server/create-bunny-app.ts b/packages/3-bunny/src/server/create-bunny-app.ts index 15f4366b..7a931ef8 100644 --- a/packages/3-bunny/src/server/create-bunny-app.ts +++ b/packages/3-bunny/src/server/create-bunny-app.ts @@ -10,6 +10,7 @@ import { readFileSync } from "node:fs"; import path from "node:path"; import serveStatic from "serve-static"; import { loadEnv } from "../load-env.js"; +import { bunnyLogger } from "./bunny-logger.js"; import { fromResponse, toRequest } from "./node-http-adapter.js"; import type { Cache } from "@farbenmeer/tapi/server"; import type { ServerConfig } from "../config.js"; @@ -31,16 +32,12 @@ export function createBunnyApp({ }: BunnyServerOptions) { loadEnv("production"); const app = connect(); - const apiRequestHandler = api().then(async ({ api }) => - createRequestHandler(api, { + const apiRequestHandler = api().then(async ({ api }) => { + if (!api.logger) api.logger = bunnyLogger; + return createRequestHandler(api, { basePath: "/api", - hooks: { - error: (error) => { - console.error(error); - }, - }, - }), - ); + }); + }); let openApiJson: string | undefined; app.use(async (req, res, next) => { diff --git a/packages/3-vite-plugin-tapi/src/index.ts b/packages/3-vite-plugin-tapi/src/index.ts index d50c7741..dc6cf3aa 100644 --- a/packages/3-vite-plugin-tapi/src/index.ts +++ b/packages/3-vite-plugin-tapi/src/index.ts @@ -88,14 +88,16 @@ export default function tapi(options: TapiPluginOptions = {}): Plugin { `[vite-plugin-tapi] ${resolvedEntry} must export \`api\` (an ApiDefinition).`, ); } - currentHandler = createRequestHandler(mod.api, { - basePath, - hooks: { + if (!mod.api.logger) { + mod.api.logger = { error: (error) => { if (error instanceof Error) vite.ssrFixStacktrace(error); console.error(error); }, - }, + }; + } + currentHandler = createRequestHandler(mod.api, { + basePath, }); }; diff --git a/website/src/content/docs/bunny/conventions/api.md b/website/src/content/docs/bunny/conventions/api.md index 948225f4..10fbda70 100644 --- a/website/src/content/docs/bunny/conventions/api.md +++ b/website/src/content/docs/bunny/conventions/api.md @@ -57,3 +57,17 @@ It provides three options: - `MemoryCache`: A simple in-memory cache implementation. Should be sufficient for single-server deployments. - `FileCache`: A file-based cache implementation. Persists data as an sqlite database. Use this for single-server deployments where a lot of data needs to be cached or the cache needs to be persisted across server restarts. - `RedisCache`: A Redis-based cache implementation. Use this for multi-server deployments. + +### Logging + +By default, Bunny installs a logger that forwards request handler errors to `console.error`. To send errors elsewhere (Sentry, Pino, etc.), pass a `logger` to `defineApi`: + +```ts +export const api = defineApi({ + logger: { + error: (error) => reportToSentry(error), + }, +}); +``` + +See [`defineApi`](/tapi/reference/defineapi) for the full `Logger` interface. diff --git a/website/src/content/docs/tapi/reference/defineApi.md b/website/src/content/docs/tapi/reference/defineApi.md index f43f7364..60b190a8 100644 --- a/website/src/content/docs/tapi/reference/defineApi.md +++ b/website/src/content/docs/tapi/reference/defineApi.md @@ -20,7 +20,7 @@ export const api = defineApi() ## Signature ```ts -function defineApi(options?: { cache?: Cache }): ApiDefinition<{}> +function defineApi(options?: { cache?: Cache; logger?: Logger }): ApiDefinition<{}> ``` Returns an empty `ApiDefinition` instance. @@ -30,6 +30,31 @@ Returns an empty `ApiDefinition` instance. | Option | Type | Default | Description | | --- | --- | --- | --- | | `cache` | `Cache` | `new PubSub()` | The cache/pub-sub instance used for tag-based revalidation. Defaults to an in-process `PubSub`. Pass a `RedisCache` or other shared implementation when running multiple server instances. | +| `logger` | `Logger` | `console.error` | Logger used by the request handler to report errors thrown by route handlers and cache operations. When omitted, errors are logged via `console.error`. Bunny installs its own default logger if you don't provide one. | + +### Logger interface + +```ts +interface Logger { + error?: (error: unknown) => void | Promise; +} +``` + +Pass any object that implements this shape — for example, a Pino or Winston instance, or a custom function that ships errors to your observability platform. + +:::note +The `Logger` interface currently only exposes `error`. Additional methods (`log`, `info`, `warn`, …) may be added in future minor releases. Implementations should treat unknown methods as optional and ignore them. +::: + +```ts +import { defineApi } from "@farbenmeer/tapi/server"; + +export const api = defineApi({ + logger: { + error: (error) => reportToSentry(error), + }, +}); +``` ## Methods