Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/createrequesthandler-drop-logger.md
Original file line number Diff line number Diff line change
@@ -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.
15 changes: 5 additions & 10 deletions packages/1-tapi/src/client/cache.ts
Original file line number Diff line number Diff line change
@@ -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";

Expand All @@ -18,14 +19,10 @@ interface CacheEntry {
timeout: ReturnType<typeof setTimeout> | null;
}

interface Hooks {
error: (error: unknown) => void | Promise<void>;
}

interface Options {
minTTL?: number;
maxOverdueTTL?: number;
hooks?: Partial<Hooks>;
logger?: Logger;
}

const DEFAULT_MIN_TTL = 5 * 1000;
Expand All @@ -36,14 +33,12 @@ export class Cache {
private tagIndex = new Map<string, Set<string>>();
private minTTL: number;
private maxOverdueTTL: number;
private hooks: Hooks;
private errorLog: (error: unknown) => void | Promise<void>;

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<Response>) {
Expand Down Expand Up @@ -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
Expand Down
11 changes: 4 additions & 7 deletions packages/1-tapi/src/client/create-fetch-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -40,15 +41,11 @@ async function listenForInvalidations(url: string, cache: Cache) {
}
}

interface Hooks {
error?: (error: unknown) => void | Promise<void>;
}

interface Options {
fetch?: (url: string, init: RequestInit) => Promise<Response>;
minTTL?: number;
maxOverdueTTL?: number;
hooks?: Hooks;
logger?: Logger;
invalidationsUrl?: string | false;
}

Expand All @@ -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 =
Expand Down Expand Up @@ -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;
}),
{
Expand Down
1 change: 1 addition & 0 deletions packages/1-tapi/src/client/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
9 changes: 3 additions & 6 deletions packages/1-tapi/src/server/create-request-handler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -79,7 +79,6 @@ describe("createRequestHandler", () => {
},
),
}),
{ hooks: { error: errorHook } },
);
const response = await sut(new Request("http://localhost:3000"));
expect(response.status).toBe(500);
Expand All @@ -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,
Expand All @@ -103,7 +102,6 @@ describe("createRequestHandler", () => {
},
),
}),
{ hooks: { error: errorHook } },
);
const response = await sut(
new Request("http://localhost:3000", {
Expand Down Expand Up @@ -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",
Expand All @@ -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" });
Expand Down
28 changes: 12 additions & 16 deletions packages/1-tapi/src/server/create-request-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>;
};
/** the default maximum time-to-live (TTL) for cached responses */
defaultTTL?: number;
}
Expand All @@ -37,11 +34,10 @@ export function createRequestHandler(
api: ApiDefinition<Record<BasePath, MaybePromise<BaseRoute>>>,
options: Options = {},
) {
const errorHook =
options.hooks?.error ??
const errorLog =
api.logger?.error ??
((error) => {
console.error(error);
return error;
});

const basePath = options.basePath ?? "";
Expand Down Expand Up @@ -93,7 +89,7 @@ export function createRequestHandler(
}
} catch (error) {
// caches errors while retrieving from cache
errorHook(error);
errorLog(error);
}

const handler = route[req.method];
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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);
}
}
Expand Down Expand Up @@ -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);
}
}
Expand Down
5 changes: 4 additions & 1 deletion packages/1-tapi/src/server/define-api.ts
Original file line number Diff line number Diff line change
@@ -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<Routes extends Record<BasePath, unknown>> {
constructor(
public routes: Routes,
public cache: Cache,
public logger?: Logger,
) {}

async invalidate(tags: string[]) {
Expand Down
1 change: 1 addition & 0 deletions packages/1-tapi/src/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
5 changes: 5 additions & 0 deletions packages/1-tapi/src/shared/logger.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import type { MaybePromise } from "./maybe-promise.js";

export interface Logger {
error?: (error: unknown) => MaybePromise<void>;
}
8 changes: 6 additions & 2 deletions packages/1-tapi/src/worker/index.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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;
}
}
Expand Down
7 changes: 2 additions & 5 deletions packages/3-bunny/src/cli/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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"),
Expand Down
7 changes: 7 additions & 0 deletions packages/3-bunny/src/server/bunny-logger.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import type { Logger } from "@farbenmeer/tapi/server";

export const bunnyLogger: Logger = {
error: (error) => {
console.error(error);
},
};
15 changes: 6 additions & 9 deletions packages/3-bunny/src/server/create-bunny-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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) => {
Expand Down
10 changes: 6 additions & 4 deletions packages/3-vite-plugin-tapi/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
};

Expand Down
Loading