diff --git a/.changeset/error-text-handle-error.md b/.changeset/error-text-handle-error.md new file mode 100644 index 00000000..d6f2b330 --- /dev/null +++ b/.changeset/error-text-handle-error.md @@ -0,0 +1,13 @@ +--- +"@bounda-dev/core": minor +"@bounda-dev/cli": minor +"@bounda-dev/cloudflare": minor +"@bounda-dev/react-router": minor +"create-bounda": minor +--- + +React Router apps no longer log the text of what a request throws unless the rule of `runtime.telemetry.errorText` says so. `createBounda`, from `@bounda-dev/react-router` and from `@bounda-dev/react-router/cloudflare`, and `@bounda-dev/react-router/app` return a `handleError` to export from `app/entry.server.tsx`, which React Router calls for what a loader, an action or the render throws, and which the render's `onError` calls too: it logs the method and the error's class and `code`, and its message, its stack and the request's path only with `"include"`, where React Router's default logs every message and stack, in production too. On Node it follows the booted app's configuration, the dev server's `"include"` before the app has booted, and writes to the console as `boot()` does (`logger` and `errorText` options of `createBounda`); on Cloudflare it follows `BOUNDA_ERROR_TEXT` in the Worker's `env`, or else the configuration, and writes JSON lines (`logger` option). It skips a request the client aborted and unwraps what React Router throws itself. The `bounda()` Vite plugin warns when the dev server or the build starts if `app/entry.server.*` is missing or exports no `handleError`. + +The environment variable `BOUNDA_ERROR_TEXT` (`ERROR_TEXT_VARIABLE`), `"omit"` or `"include"`, overrides `runtime.telemetry.errorText`: `boot()` reads it once `.env` is loaded, the CLI reads it, below `--error-text`, and the Durable Object and the Worker of `@bounda-dev/cloudflare` read it from their `env`, so `.dev.vars` turns the text of errors on in development while a deployment keeps it out. `boot({ errorText })` and `bounda --error-text` still win; any other value fails with `ConfigurationError`. Every host resolves that one precedence, its own override, then `BOUNDA_ERROR_TEXT`, then the configuration, then its own default, then `"omit"`, through `resolveErrorText`, and `withErrorText` returns the configuration with it applied, for a host of your own. `bounda status` opens the text of the subscribers' failures under that resolved rule, `--error-text` or `BOUNDA_ERROR_TEXT=include` alike, through the call the access log records. `@bounda-dev/cloudflare` exports `workersLogger`, the logger its object and Worker default to. + +Projects from `create-bounda` with React Router get `app/entry.server.tsx`, React Router's default entry for their runtime wired to Bounda's `handleError`; projects on Cloudflare get `BOUNDA_ERROR_TEXT=include` in `.dev.vars`, and a note in `.dev.vars.example` that keeps it out of what Deploy to Cloudflare asks for. diff --git a/docs/src/content/docs/adapters/cloudflare.md b/docs/src/content/docs/adapters/cloudflare.md index 5ca1e83c..371b2e6a 100644 --- a/docs/src/content/docs/adapters/cloudflare.md +++ b/docs/src/content/docs/adapters/cloudflare.md @@ -90,6 +90,15 @@ Object has no loop running between requests: `createBoundaObject` accepts `passesPerAlarm` (50 by default): how many rounds of work one alarm does before it yields and wakes itself again. +The object and the Worker log one JSON line per entry, which Workers Logs parses into fields; +`workersLogger` is that logger, and a `logger` option takes another. A failure is logged by its +class and its `code`, without its message, as [error text](/reference/observability/#error-text) +says by default. The configuration is the same bundle wherever it is deployed, so +`BOUNDA_ERROR_TEXT` in the Worker's `env` decides instead: `include` there, in `.dev.vars`, puts +the message and the stack in the logs under `wrangler dev`, for the object and the Worker alike, +and a deployment, which never reads `.dev.vars`, keeps them out. `create-bounda` writes that line; +`.dev.vars.example` only mentions it, so Deploy to Cloudflare never asks for it. + ## Ports and `env` A port implementation that exports `create` receives the object's `env`: the Worker's @@ -180,7 +189,7 @@ global stream. Refusals come back as `{ "error": { "code", "message" } }` with the statuses in [errors over HTTP](/reference/errors/#over-http); anything else is a 500 whose message goes to the -logs, not to the caller. A body over `maxPayloadBytes`, `DEFAULT_MAX_PAYLOAD_BYTES` unless +logs, not to the caller, under the rule of [error text](/reference/observability/#error-text). A body over `maxPayloadBytes`, `DEFAULT_MAX_PAYLOAD_BYTES` unless `createWorker({ config, tenantOf, maxPayloadBytes })` says otherwise, is refused with a 413 before it is read. diff --git a/docs/src/content/docs/guides/personal-data.md b/docs/src/content/docs/guides/personal-data.md index 091d5972..bfaa41f9 100644 --- a/docs/src/content/docs/guides/personal-data.md +++ b/docs/src/content/docs/guides/personal-data.md @@ -255,8 +255,12 @@ and never its text, and so does the last error the inbox and the scheduler keep The runtime's own log lines and spans carry no error's text: an error is logged and traced by its class, its code and the ids of what failed, and the line that files a dead letter names the letter, whose text `app.deadLetters` opens for as long as the person is not forgotten. Only -[`runtime.telemetry.errorText: "include"`](/reference/observability/#error-text) puts the message -and the stack there, in the clear, out of `forget`'s reach: keep it to development. +[`runtime.telemetry.errorText: "include"`](/reference/observability/#error-text), or +`BOUNDA_ERROR_TEXT=include`, puts the message and the stack there, in the clear, out of `forget`'s +reach: keep it to development. A React Router app logs what its requests throw by the same rule +once `app/entry.server.tsx` exports Bounda's `handleError` +([logging errors](/guides/react-router/#logging-errors)); without it, React Router logs every +message and stack. What `forget` does not reach is text your code writes elsewhere and data left unmarked: your logs keep whatever you wrote to them, an event's payload written in the clear, with the data in a field diff --git a/docs/src/content/docs/guides/react-router.md b/docs/src/content/docs/guides/react-router.md index 33b01394..caf00fd1 100644 --- a/docs/src/content/docs/guides/react-router.md +++ b/docs/src/content/docs/guides/react-router.md @@ -36,8 +36,8 @@ export default defineConfig({ plugins: [bounda(), reactRouter()] }); The plugin does two things. It runs `bounda generate` when the dev server or the build starts and after every change under `app/domain` and `app/read`, so there is no generator to keep running on the side. And it serves `@bounda-dev/react-router/app`: the `bounda` context, the -`boundaMiddleware`, a `dispose()` and `failure`, wired to the generated registry and typed for -your project through `.bounda/register.d.ts`. +`boundaMiddleware`, a `dispose()`, `failure` and `handleError`, wired to the generated registry and +typed for your project through `.bounda/register.d.ts`. Bounda's modules live in the same `app/` directory as your routes: `app/domain` and `app/read` next to `app/routes`, `app/root.tsx` and `app/routes.ts`. The generator only looks at those two @@ -114,6 +114,42 @@ Cloudflare, which arrive as plain errors. Anything else is rethrown and reaches the route's `ErrorBoundary`. `Failure`, the shape of that data, is exported from `@bounda-dev/react-router`. +## Logging errors + +React Router logs what a loader, an action or the render throws, and its default `handleError` +logs the message and the stack, in production too. A message may quote what the code read, a +person's email included, and a log keeps it in the clear, where `forget` never reaches. So export +Bounda's `handleError` from `app/entry.server.tsx`, and call it from the render's `onError`: + +```tsx +// app/entry.server.tsx +import { handleError } from "@bounda-dev/react-router/app"; + +export { handleError }; + +export default function handleRequest(/* … */) { + // React Router's default entry, with `onError` calling + // `void handleError(error, { request })` where the default calls `console.error(error)`. +} +``` + +It logs one line per error, `request failed` with the method, the error's class and, for one of +Bounda's, its `code`, under the rule of +[error text](/reference/observability/#error-text): the message, the stack and the request's path, +which may quote what a user typed, only with `runtime.telemetry.errorText: "include"` or +`BOUNDA_ERROR_TEXT=include`. Like the default, it skips a request the client aborted, and it +unwraps what React Router throws itself, such as no route matching, to the error inside. On Node +it writes to the console as `boot()` does, and follows the app's configuration once it has booted: +the dev server's `"include"` before that, `"omit"` in a build. On Cloudflare it writes JSON lines, +as the Durable Object does, and follows `BOUNDA_ERROR_TEXT` in the Worker's `env`, or else the +configuration once a request has imported it: it never waits for an import, which the Worker could +cancel once the response is sent. + +`create-bounda` writes the whole file, React Router's default entry for the runtime with these two +changes: in an existing project, `react-router reveal entry.server` writes the default into `app/` +to start from. Without the file, or when it does not export `handleError`, the `bounda()` plugin warns +when the dev server or the build starts. + ## When the client goes away A command keeps going when the browser closes the tab: the request has already arrived, and @@ -230,8 +266,8 @@ right after. the built app reads it from the directory it runs in. - The dev server logs the text of errors, message and stack, unless `bounda.config.ts` sets [`runtime.telemetry.errorText`](/reference/observability/#error-text); the built app does not, - since an error's message may quote personal data. On Cloudflare the app runs in its Durable - Object, which follows its own configuration. + since an error's message may quote personal data. `BOUNDA_ERROR_TEXT` in the environment or in + `.env` overrides both. On Cloudflare the app runs in its Durable Object: see below. - `react-router build` bundles `bounda.config.ts` and the generated registry into the server build, so the build runs from wherever it is deployed, with no need to ship either file. - A component that touches `@bounda-dev/react-router/app` gets a clear error: the client build @@ -325,6 +361,10 @@ processes and scheduled commands run in the object's alarm, as - `react-router build` writes the Worker and its `wrangler.json`; `wrangler deploy` deploys it. - Bounda loads no `.env` here: the Worker's variables and secrets come from Wrangler, locally from `.dev.vars`, and port implementations receive them as `env` in `create`. +- The configuration is the same bundle in development and in production, so the text of errors + comes from `.dev.vars`: `BOUNDA_ERROR_TEXT=include` there makes the Durable Object's logs and + `handleError` in the Worker carry it under `react-router dev` and `wrangler dev`, and a + deployment, which never reads `.dev.vars`, logs without it. `create-bounda` writes that line. - With `storage: cloudflare()` but without `@cloudflare/vite-plugin`, the server runs in Node, which cannot import the Workers runtime, and the first request fails saying to add the plugin. @@ -348,7 +388,7 @@ import { boot } from "@bounda-dev/core/node"; import { createBounda } from "@bounda-dev/react-router"; import { registry } from "../.bounda/registry.ts"; -export const { bounda, boundaMiddleware, dispose } = createBounda({ +export const { bounda, boundaMiddleware, dispose, handleError } = createBounda({ boot: () => boot({ registry, importConfig: () => import("../bounda.config.ts") }), }); ``` @@ -358,8 +398,11 @@ build bundles both and a change to either re-evaluates this file in development; then stops the app booted before, and the next request boots a new one once that stop has finished, so the two never hold the storage at the same time. Its options are `boot` (how to create the app, `boot()` by default), `consistency`, `actor` (the function `app/actor.ts` would -export) and `key` (where the running app is kept on `globalThis`, one app per key). `dispose()` stops the running app and forgets it, and resolves -once every app booted under that key has stopped. Import `failure` from +export), `key` (where the running app is kept on `globalThis`, one app per key), and for +`handleError` a `logger` (`createConsoleLogger()` by default) and the `errorText` it follows before +an app has booted (`"omit"` by default). `dispose()` stops the running app and forgets it, and resolves +once every app booted under that key has stopped. Export `handleError` from +`app/entry.server.tsx` as [above](#logging-errors). Import `failure` from `@bounda-dev/react-router`. On Cloudflare, `createBounda` from `@bounda-dev/react-router/cloudflare` takes the configuration @@ -370,12 +413,13 @@ and the tenant instead, and reads the binding from the Worker's `env`: import { createBounda } from "@bounda-dev/react-router/cloudflare"; import config from "../bounda.config.ts"; -export const { bounda, boundaMiddleware } = createBounda({ +export const { bounda, boundaMiddleware, handleError } = createBounda({ config, tenant: ({ params }) => params.workspace ?? "default", }); ``` -It takes `consistency` and `actor` too, and throws `ConfigurationError` when `storage` is not `cloudflare()`, +It takes `consistency`, `actor` and a `logger` for `handleError` (`workersLogger` from +`@bounda-dev/cloudflare` by default) too, and throws `ConfigurationError` when `storage` is not `cloudflare()`, when the Worker has no such binding, without a `tenant` function, or for a `consistency` it does not know. diff --git a/docs/src/content/docs/reference/cli.md b/docs/src/content/docs/reference/cli.md index 821d7aab..9592875c 100644 --- a/docs/src/content/docs/reference/cli.md +++ b/docs/src/content/docs/reference/cli.md @@ -41,8 +41,9 @@ error: ValidationError (VALIDATION_FAILED); its message may quote personal data: ``` `--error-text`, on every command that loads the project, prints the message and lets the log lines -carry the text of errors for that run; a configuration that sets `errorText: "include"` does the -same without it. What fails before any handler runs is printed in full: a configuration or a +carry the text of errors for that run; `BOUNDA_ERROR_TEXT=include` in the environment or in `.env`, +or a configuration that sets `errorText: "include"`, does the same without it, and +`BOUNDA_ERROR_TEXT=omit` keeps the text out whatever the configuration says. What fails before any handler runs is printed in full: a configuration or a registry that cannot be loaded, a read model that is not in the registry, a list `forget` cannot read, and errors whose message Bounda writes from names and ids alone, such as a dead letter that is not found or was already settled. To read what a reaction failed on, read its letter: @@ -287,8 +288,9 @@ dead letter retention: 30d, next purge 2026-11-20T08:00:00.000Z [retention](/guides/personal-data/#inactivity) and the [purge of resolved dead letters](/guides/dead-letters/#keeping-resolved-letters), each `not configured` when it is off. -`--error-text` also prints, under each failure, the text it keeps, encrypted under the person the -event names: it is read through `app.subscriberFailures.list()`, which records it in the +Under the rule of [error text](#error-text) set to `"include"`, by `--error-text`, +`BOUNDA_ERROR_TEXT=include` or the configuration, it also prints, under each failure, the text it +keeps, encrypted under the person the event names: it is read through `app.subscriberFailures.list()`, which records it in the [access log](/guides/personal-data/#who-opened-what) with the persons and `--actor`. A failure whose person was forgotten says its text went with them. diff --git a/docs/src/content/docs/reference/configuration.md b/docs/src/content/docs/reference/configuration.md index 92f5f1e6..3961acaf 100644 --- a/docs/src/content/docs/reference/configuration.md +++ b/docs/src/content/docs/reference/configuration.md @@ -165,7 +165,8 @@ a stopped one left: see [delivery guarantees](/concepts/delivery-guarantees/#the as `ConcurrencyError` or a dead letter not found, keep it. `"include"` adds the message and the stack of every error, for development; `createTestApp` and the dev server of `@bounda-dev/react-router` default to it, and `bounda --error-text` turns it on for one command. - See [error text](/reference/observability/#error-text). + The environment variable `BOUNDA_ERROR_TEXT` overrides it, as `.dev.vars` does in development on + Cloudflare. See [error text](/reference/observability/#error-text). ### Per aggregate diff --git a/docs/src/content/docs/reference/observability.md b/docs/src/content/docs/reference/observability.md index 2a06b7e3..d5b2bd71 100644 --- a/docs/src/content/docs/reference/observability.md +++ b/docs/src/content/docs/reference/observability.md @@ -96,6 +96,10 @@ import { errorFields } from "@bounda-dev/core"; logger.error("request failed", { path, ...errorFields({ error }) }); ``` +A React Router app exports Bounda's `handleError` from `app/entry.server.tsx`, which logs what a +loader, an action or the render throws by the same rule: React Router's default logs every message +and stack, in production too (see [logging errors](/guides/react-router/#logging-errors)). + [`runtime.telemetry.errorText: "include"`](/reference/configuration/#telemetry) adds the message and the stack everywhere: `errorMessage` and `errorStack` in log lines, `exception.message` and `exception.stacktrace` on spans, the message as a failed span's status and `errorMessage` in the @@ -103,6 +107,25 @@ lag. It is for development: `createTestApp` and the dev server of `@bounda-dev/r default to it unless the configuration says otherwise, `bounda --error-text` turns it on for one command, and the logger handed to port implementations follows it too. +The environment variable `BOUNDA_ERROR_TEXT`, `"omit"` or `"include"`, sets it for a deployment +without touching the configuration, which on Cloudflare is the same bundle everywhere: `boot()`, +the CLI, and the Durable Object and the Worker of `@bounda-dev/cloudflare` read it, over what the +configuration says. `bounda --error-text` and `boot({ errorText })` still win over it, and any +other value fails the boot with `ConfigurationError`. Projects from `create-bounda` on Cloudflare +set it in `.dev.vars`, which `wrangler dev` and `vite dev` read and a deployment never sees; the +constant `ERROR_TEXT_VARIABLE` from `@bounda-dev/core` names it. + +Every host resolves the rule with the same precedence: its own override, then `BOUNDA_ERROR_TEXT`, +then the configuration, then the host's own default (the dev server's `"include"`), then +`"omit"`. A host of your own resolves it the same way with `resolveErrorText` from +`@bounda-dev/core`, and `withErrorText` returns the configuration with it applied: + +```ts +import { withErrorText } from "@bounda-dev/core"; + +const app = await createApp({ registry, config: withErrorText({ config, env: process.env }) }); +``` + ## How a command loaded its aggregate A `bounda.command` span that loaded its aggregate says how, once it ends, whatever the outcome: diff --git a/examples/onboarding/README.md b/examples/onboarding/README.md index a358170e..fc0ef7b8 100644 --- a/examples/onboarding/README.md +++ b/examples/onboarding/README.md @@ -35,7 +35,9 @@ export default defineConfig({ plugins: [bounda(), reactRouter()] }); The plugin runs `bounda generate` when the dev server or the build starts and after every change under `app/domain` and `app/read`, and serves `@bounda-dev/react-router/app`: the `bounda` -context, the `boundaMiddleware` that `root.tsx` mounts, and `dispose`. Loaders and actions read +context, the `boundaMiddleware` that `root.tsx` mounts, `dispose`, and the `handleError` that +`entry.server.tsx` exports, so what a request throws is logged without its message, which may +quote personal data, except under the dev server. Loaders and actions read the app with `context.get(bounda)`. The served module imports the generated registry by value, so editing a domain module re-evaluates it and the next request boots an app from the new code. `register.d.ts`, also generated, types `bounda` for this project. diff --git a/examples/onboarding/app/entry.server.tsx b/examples/onboarding/app/entry.server.tsx new file mode 100644 index 00000000..370d15f6 --- /dev/null +++ b/examples/onboarding/app/entry.server.tsx @@ -0,0 +1,69 @@ +import { PassThrough } from "node:stream"; +import { handleError } from "@bounda-dev/react-router/app"; +import { createReadableStreamFromReadable } from "@react-router/node"; +import { isbot } from "isbot"; +import { type RenderToPipeableStreamOptions, renderToPipeableStream } from "react-dom/server"; +import { type EntryContext, ServerRouter } from "react-router"; + +// React Router's default entry for Node, with Bounda's handleError: it logs what a loader, an +// action or the render throws as runtime.telemetry.errorText says, where the default logs every +// message and stack, and a message may quote personal data. +export { handleError }; + +export const streamTimeout = 5_000; + +export default function handleRequest( + request: Request, + responseStatusCode: number, + responseHeaders: Headers, + routerContext: EntryContext, +) { + // https://httpwg.org/specs/rfc9110.html#HEAD + if (request.method.toUpperCase() === "HEAD") { + return new Response(null, { status: responseStatusCode, headers: responseHeaders }); + } + + return new Promise((resolve, reject) => { + let status = responseStatusCode; + let shellRendered = false; + const userAgent = request.headers.get("user-agent"); + + // Bots and SPA mode wait for all content before the response starts. + const readyOption: keyof RenderToPipeableStreamOptions = + (userAgent && isbot(userAgent)) || routerContext.isSpaMode ? "onAllReady" : "onShellReady"; + + // Aborts the render after streamTimeout, leaving it time to flush the rejected boundaries. + let timeoutId: ReturnType | undefined = setTimeout( + () => abort(), + streamTimeout + 1000, + ); + + const { pipe, abort } = renderToPipeableStream( + , + { + [readyOption]() { + shellRendered = true; + const body = new PassThrough({ + final(callback) { + clearTimeout(timeoutId); + timeoutId = undefined; + callback(); + }, + }); + const stream = createReadableStreamFromReadable(body); + responseHeaders.set("Content-Type", "text/html"); + pipe(body); + resolve(new Response(stream, { headers: responseHeaders, status })); + }, + onShellError(error: unknown) { + reject(error); + }, + onError(error: unknown) { + status = 500; + // An error before the shell rejects, and React Router hands it to handleError itself. + if (shellRendered) void handleError(error, { request }); + }, + }, + ); + }); +} diff --git a/packages/cli/src/run.test.ts b/packages/cli/src/run.test.ts index 9370f794..065292e2 100644 --- a/packages/cli/src/run.test.ts +++ b/packages/cli/src/run.test.ts @@ -403,6 +403,51 @@ describe("bounda rebuild", () => { expect(told.stderr).toContain('"errorMessage":"cannot total ana@example.com"'); }); + it("prints and logs the text of errors as BOUNDA_ERROR_TEXT says, under --error-text", async () => { + const args = ["dead-letters", "list", "--registry", ".bounda/failing-registry.ts"]; + try { + process.env.BOUNDA_ERROR_TEXT = "include"; + const told = await cli(args, fixture); + expect(told.stderr).toContain('"errorMessage":"probed ana@example.com"'); + process.env.BOUNDA_ERROR_TEXT = "omit"; + const quiet = await cli(args, fixture); + expect(quiet.stderr).not.toContain("ana@example.com"); + const flagged = await cli([...args, "--error-text"], fixture); + expect(flagged.stderr).toContain('"errorMessage":"probed ana@example.com"'); + process.env.BOUNDA_ERROR_TEXT = "loud"; + const refused = await cli(args, fixture); + expect(refused.code).toBe(EXIT_FAILURE); + expect(refused.stderr).toBe( + 'error: BOUNDA_ERROR_TEXT must be "omit" or "include", not "loud"\n', + ); + } finally { + delete process.env.BOUNDA_ERROR_TEXT; + } + }); + + it("prints the text of a rebuild's error when BOUNDA_ERROR_TEXT includes it", async () => { + const project = await loadProject({ + root: fixture, + registryPath: ".bounda/failing-registry.ts", + logger: silentLogger, + }); + const app = await createApp({ ...project, logger: silentLogger }); + await app.commands.increment({ counterId: `c-${Date.now()}` }); + await app.stop(); + try { + process.env.BOUNDA_ERROR_TEXT = "include"; + const told = await cli( + ["rebuild", "failingTotals", "--registry", ".bounda/failing-registry.ts"], + fixture, + ); + expect(told.code).toBe(EXIT_FAILURE); + expect(told.stderr).toContain("error: cannot total ana@example.com\n"); + expect(told.stderr).toContain('"errorMessage":"cannot total ana@example.com"'); + } finally { + delete process.env.BOUNDA_ERROR_TEXT; + } + }); + it("logs the text of errors while the app runs only with --error-text", async () => { const args = ["dead-letters", "list", "--registry", ".bounda/failing-registry.ts"]; const quiet = await cli(args, fixture); @@ -821,6 +866,28 @@ describe("bounda status", () => { ]); }); + it.each([ + ["include", [], true], + ["omit", [], false], + ["omit", ["--error-text"], true], + ] as const)( + "opens the texts of the failures as BOUNDA_ERROR_TEXT %s says, under %j", + async (variable, flags, opened) => { + await fail(); + try { + process.env.BOUNDA_ERROR_TEXT = variable; + const result = await cli(["status", ...failing, ...flags], fixture); + expect(result.code).toBe(EXIT_OK); + expect(result.stdout.includes("cannot total ana@example.com")).toBe(opened); + expect(logged(result.stderr, "info", "personal data accessed")).toHaveLength( + opened ? 1 : 0, + ); + } finally { + delete process.env.BOUNDA_ERROR_TEXT; + } + }, + ); + it("prints the status as JSON with --json, with the texts only when asked for", async () => { await fail(); const quiet = await cli(["status", ...failing, "--json"], fixture); @@ -905,6 +972,28 @@ export default { storage: adapter }; } }); + it("prints the text a migration fails on under BOUNDA_ERROR_TEXT, and refuses a value it does not know before migrating", async () => { + const root = await migrating(); + process.env.REFUSE_MIGRATION = "1"; + try { + process.env.BOUNDA_ERROR_TEXT = "include"; + const told = await cli(["migrate", "--root", root], repoRoot); + expect(told.code).toBe(EXIT_FAILURE); + expect(told.stderr).toContain("ana@example.com"); + Reflect.deleteProperty(globalThis, "migratedScopes"); + process.env.BOUNDA_ERROR_TEXT = "loud"; + const refused = await cli(["migrate", "--root", root], repoRoot); + expect(refused.code).toBe(EXIT_FAILURE); + expect(refused.stderr).toBe( + 'error: BOUNDA_ERROR_TEXT must be "omit" or "include", not "loud"\n', + ); + expect(Reflect.get(globalThis, "migratedScopes")).toBeUndefined(); + } finally { + delete process.env.REFUSE_MIGRATION; + delete process.env.BOUNDA_ERROR_TEXT; + } + }); + it("exits 2 with the reason when the configuration is not there", async () => { const root = await mkdtemp(join(tmpdir(), "bounda-cli-migrate-")); temporary.push(root); diff --git a/packages/cli/src/run.ts b/packages/cli/src/run.ts index 0bd68158..39414e74 100644 --- a/packages/cli/src/run.ts +++ b/packages/cli/src/run.ts @@ -12,7 +12,9 @@ import { migrateTables, type Registry, rebuildReadModel, + resolveErrorText, type SubscriberFailure, + withErrorText, } from "@bounda-dev/core"; import type { Config, ErrorText } from "@bounda-dev/core/config"; import { boot, createConsoleLogger, loadProject } from "@bounda-dev/core/node"; @@ -96,25 +98,17 @@ const runGenerate = async ( const logTo = (stderr: Output): Logger => createConsoleLogger({ stream: stderr }); -// The flag wins; without it, what the configuration sets. -const errorTextOf = (options: ProjectOptions, config: Config): ErrorText | undefined => - options.errorText ? "include" : config.runtime?.telemetry?.errorText; - -const withErrorText = (config: Config, options: ProjectOptions): Config => - options.errorText - ? { - ...config, - runtime: { - ...config.runtime, - telemetry: { ...config.runtime?.telemetry, errorText: "include" }, - }, - } - : config; +// `process.env` is read once `load` has loaded `.env` and checked `BOUNDA_ERROR_TEXT`. +const errorTextArgs = (options: ProjectOptions, config: Config) => ({ + forced: options.errorText ? ("include" as const) : undefined, + env: process.env, + config, +}); const rootOf = (options: ProjectOptions, cwd: string): string => resolve(cwd, options.root ?? "."); -// What fails while the project's modules load tells of files and exports, never of data a handler -// read, so it is printed in full. +// What fails while the project's modules load, or a `BOUNDA_ERROR_TEXT` it does not know, tells of +// files, exports and settings, never of data a handler read, so it is printed in full. const load = async ( options: ProjectOptions, cwd: string, @@ -122,12 +116,14 @@ const load = async ( stderr: Output, ): Promise> | undefined> => { try { - return await loadProject({ + const project = await loadProject({ root: rootOf(options, cwd), configPath: options.config, registryPath: options.registry, logger, }); + resolveErrorText({ env: process.env }); + return project; } catch (error) { line(stderr, `error: ${error instanceof Error ? error.message : String(error)}`); return undefined; @@ -139,11 +135,7 @@ const ERROR_TEXT_HINT = // A handler's error may quote personal data, and stderr often ends in a log: its message is // printed only as the runtime would log it. -const failed = ( - stderr: Output, - error: unknown, - errorText: ErrorText | undefined, -): typeof EXIT_FAILURE => { +const failed = (stderr: Output, error: unknown, errorText: ErrorText): typeof EXIT_FAILURE => { const { errorName, errorCode, errorMessage } = errorFields({ error, errorText }); line( stderr, @@ -167,7 +159,7 @@ const runRebuild = async ( try { const result = await rebuildReadModel({ ...project, - config: withErrorText(project.config, options), + config: withErrorText(errorTextArgs(options, project.config)), env: process.env, name: readModel, logger, @@ -179,7 +171,7 @@ const runRebuild = async ( ); return EXIT_OK; } catch (error) { - return failed(stderr, error, errorTextOf(options, project.config)); + return failed(stderr, error, resolveErrorText(errorTextArgs(options, project.config))); } }; @@ -226,7 +218,7 @@ const withApp = async ( }); return await work(app); } catch (error) { - return failed(stderr, error, errorTextOf(options, project.config)); + return failed(stderr, error, resolveErrorText(errorTextArgs(options, project.config))); } finally { await app?.stop(); } @@ -364,7 +356,8 @@ interface StatusOptions extends ActingOptions { readonly json: boolean; } -// The text of each failure is opened only when asked for, through the call the access log records. +// The text of each failure is opened only under the app's rule for error text, `--error-text` +// included, through the call the access log records. const runStatus = ( options: StatusOptions, cwd: string, @@ -373,9 +366,10 @@ const runStatus = ( ): Promise => withApp(options, cwd, stderr, async (app) => { const status = await app.status(); - const opened = options.errorText - ? await actingAs(app, options).subscriberFailures.list() - : undefined; + const opened = + app.config.runtime.telemetry.errorText === "include" + ? await actingAs(app, options).subscriberFailures.list() + : undefined; if (options.json) { line( stdout, @@ -432,6 +426,7 @@ const runMigrate = async ( registry: NO_REGISTRY, logger, })); + resolveErrorText({ env: process.env }); } catch (error) { line(stderr, `error: ${error instanceof Error ? error.message : String(error)}`); return EXIT_FAILURE; @@ -440,7 +435,7 @@ const runMigrate = async ( line(stdout, `migrated\n${formatSchema(await migrateTables({ config }))}`); return EXIT_OK; } catch (error) { - return failed(stderr, error, errorTextOf(options, config)); + return failed(stderr, error, resolveErrorText(errorTextArgs(options, config))); } }; @@ -641,6 +636,6 @@ export const runCli: RunCliFunction = async ({ argv, cwd, stdout, stderr, signal if (error instanceof CommanderError) { return error.exitCode === 0 ? EXIT_OK : EXIT_CONVENTION; } - return failed(stderr, error, undefined); + return failed(stderr, error, resolveErrorText({ env: process.env, ignoreInvalidEnv: true })); } }; diff --git a/packages/cloudflare/src/bounda-object.ts b/packages/cloudflare/src/bounda-object.ts index 066941f6..078cb9b5 100644 --- a/packages/cloudflare/src/bounda-object.ts +++ b/packages/cloudflare/src/bounda-object.ts @@ -25,6 +25,7 @@ import { type StoredEvent, type SubscriberFailure, systemClock, + withErrorText, } from "@bounda-dev/core"; import type { Config } from "@bounda-dev/core/config"; import { durableObjectAdapter } from "./adapter.ts"; @@ -201,7 +202,9 @@ const ownFunction = unknown>( * scheduled commands and retries run in the object's alarm, which it arms itself for whatever * comes next. One object is one store: give each tenant its own with `idFromName(tenant)`. Port * implementations receive the object's `env` in `create`, once per object, and that name as - * `tenant`, which an object reached through an id without a name does not have. + * `tenant`, which an object reached through an id without a name does not have. `BOUNDA_ERROR_TEXT` + * in that `env`, such as `.dev.vars` sets in development, overrides `runtime.telemetry.errorText`; + * any value but `"omit"` or `"include"` fails the object's start with `ConfigurationError`. */ export const createBoundaObject: CreateBoundaObjectFunction = ({ registry, @@ -212,7 +215,6 @@ export const createBoundaObject: CreateBoundaObjectFunction = ): BoundaObjectClass => { - const errorText = config.runtime?.telemetry?.errorText; return class BoundaObject extends DurableObject implements BoundaObjectMethods { #app: BoundaApp | undefined; @@ -221,7 +223,9 @@ export const createBoundaObject: CreateBoundaObjectFunction = { this.#app = await createApp({ registry, - config: configForObject(config, ctx.storage), + // The configuration is one bundle for every environment, so `.dev.vars` decides in + // development. + config: configForObject(withErrorText({ config, env }), ctx.storage), env, tenant: ctx.id.name, logger, @@ -236,6 +240,10 @@ export const createBoundaObject: CreateBoundaObjectFunction = { const app = this.#ready(); @@ -287,7 +295,7 @@ export const createBoundaObject: CreateBoundaObjectFunction = console.debug(JSON.stringify({ level: "debug", message, ...fields })), info: (message, fields) => console.info(JSON.stringify({ level: "info", message, ...fields })), diff --git a/packages/cloudflare/src/worker.ts b/packages/cloudflare/src/worker.ts index ab6475cc..cedee569 100644 --- a/packages/cloudflare/src/worker.ts +++ b/packages/cloudflare/src/worker.ts @@ -5,6 +5,7 @@ import { type DispatchOptions, errorFields, type Logger, + resolveErrorText, } from "@bounda-dev/core"; import type { Config } from "@bounda-dev/core/config"; import type { BoundaStub } from "./client.ts"; @@ -162,7 +163,8 @@ const route = (pathname: string): { readonly kind: string; readonly name: string * with `issues`, 404 for an unknown command, query or row, 409 for a command's rejection, with its * code in `rejected`, for one refused because the person it touches is restricted, or for a * conflict, 413 for a body over `maxPayloadBytes`. Anything else is a - * 500 without its message, which is logged instead, as `runtime.telemetry.errorText` says. There is no authentication and no operator + * 500 without its message, which is logged instead, as `BOUNDA_ERROR_TEXT` in the Worker's `env` + * says, or else `runtime.telemetry.errorText`. There is no authentication and no operator * endpoint: it is a starting point, and an app with users writes its own `fetch` over `connect`. Throws `ConfigurationError` when * `storage` is not `cloudflare()` or for a `consistency` it does not know. */ @@ -227,7 +229,12 @@ export const createWorker: CreateWorkerFunction = ({ } logger.error("bounda worker request failed", { path: url.pathname, - ...errorFields({ error, errorText: config.runtime?.telemetry?.errorText }), + // The object refuses a `BOUNDA_ERROR_TEXT` it does not know, and the request fails with + // that: logging the failure must not throw instead. + ...errorFields({ + error, + errorText: resolveErrorText({ env, config, ignoreInvalidEnv: true }), + }), }); return failure("INTERNAL", "Internal error", 500); } diff --git a/packages/cloudflare/test/bounda-object.test.ts b/packages/cloudflare/test/bounda-object.test.ts index 27b87b46..32c7925f 100644 --- a/packages/cloudflare/test/bounda-object.test.ts +++ b/packages/cloudflare/test/bounda-object.test.ts @@ -4,18 +4,18 @@ import { createSqliteAdapter } from "@bounda-dev/core/adapter/sqlite"; import { expectForgotten } from "@bounda-dev/core/testing"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { durableObjectAdapter } from "../src/adapter.ts"; -import { configForObject } from "../src/bounda-object.ts"; +import { configForObject, createBoundaObject } from "../src/bounda-object.ts"; import { connect } from "../src/client.ts"; import { cloudflare } from "../src/definition.ts"; import { outage, type processRegistry, - type quietRegistry, + quietRegistry, type registry, type slicedRegistry, } from "./app.ts"; import { clock } from "./clock.ts"; -import { logs } from "./logs.ts"; +import { logs, recordingLogger } from "./logs.ts"; import { QuietStore } from "./test-worker.ts"; const fresh = () => env.STORE.get(env.STORE.newUniqueId()); @@ -839,3 +839,59 @@ describe("configForObject", () => { }); }); }); + +describe("BOUNDA_ERROR_TEXT", () => { + // An object built in the context of an empty one, so it reads the environment the test hands it. + const failingAlarmWith = async ( + variables: Readonly>, + runtime: { readonly telemetry?: { readonly errorText: "omit" | "include" } } = { + telemetry: { errorText: "omit" }, + }, + ) => + runInDurableObject(env.BARE.get(env.BARE.newUniqueId()), async (_bare, state) => { + const Store = createBoundaObject({ + registry: quietRegistry, + config: { storage: cloudflare(), runtime }, + clock, + logger: recordingLogger, + }); + const store = new Store(state, { ...env, ...variables } as Cloudflare.Env); + await vi.waitFor(async () => expect((await store.lag()).ok).toBe(true)); + const tables = state.storage.sql + .exec(`SELECT "name" FROM sqlite_master WHERE "type" = 'table' AND "name" LIKE 'bounda_%'`) + .toArray(); + for (const { name } of tables) state.storage.sql.exec(`DROP TABLE "${String(name)}"`); + logs.error.length = 0; + await store.alarm(); + return objectLogs(logs.error)[0]; + }); + + it("logs the text of the object's errors when its environment includes it, over the configuration", async () => { + expect(await failingAlarmWith({ BOUNDA_ERROR_TEXT: "include" })).toEqual([ + "bounda rebuilds could not be listed", + { + errorName: "Error", + errorMessage: expect.stringContaining("no such table"), + errorStack: expect.any(String), + }, + ]); + expect(await failingAlarmWith({})).toEqual([ + "bounda rebuilds could not be listed", + { errorName: "Error" }, + ]); + expect( + await failingAlarmWith( + { BOUNDA_ERROR_TEXT: "omit" }, + { telemetry: { errorText: "include" } }, + ), + ).toEqual(["bounda rebuilds could not be listed", { errorName: "Error" }]); + expect(await failingAlarmWith({}, { telemetry: { errorText: "include" } })).toMatchObject([ + "bounda rebuilds could not be listed", + { errorMessage: expect.stringContaining("no such table") }, + ]); + expect(await failingAlarmWith({ BOUNDA_ERROR_TEXT: "include" }, {})).toMatchObject([ + "bounda rebuilds could not be listed", + { errorMessage: expect.stringContaining("no such table") }, + ]); + }); +}); diff --git a/packages/cloudflare/test/worker.test.ts b/packages/cloudflare/test/worker.test.ts index 57ae692a..2e6e721b 100644 --- a/packages/cloudflare/test/worker.test.ts +++ b/packages/cloudflare/test/worker.test.ts @@ -208,6 +208,99 @@ describe("createWorker", () => { }, ); + it.each([ + ["include", "omit", { errorMessage: "no session for ana@example.com" }], + ["omit", "include", {}], + ] as const)( + "logs the error of a 500 as BOUNDA_ERROR_TEXT %s in the Worker's env says, over the configuration's %s", + async (variable, configured, text) => { + const errors: unknown[][] = []; + const worker = createWorker({ + config: { + storage: cloudflare({ binding: "SPY" }), + runtime: { telemetry: { errorText: configured } }, + }, + tenantOf: () => { + throw new Error("no session for ana@example.com"); + }, + logger: { + debug: () => undefined, + info: () => undefined, + warn: () => undefined, + error: (...entry) => void errors.push(entry), + }, + }); + const response = await worker.fetch?.( + new Request("https://bounda.test/queries/getOrder", { + method: "POST", + body: JSON.stringify({ orderId: "o-1" }), + }) as Request, + { + BOUNDA_ERROR_TEXT: variable, + SPY: { idFromName: () => "id", get: () => recordingStub().stub }, + } as unknown as Cloudflare.Env, + createExecutionContext(), + ); + expect(response?.status).toBe(500); + expect(errors).toEqual([ + [ + "bounda worker request failed", + { + path: "/queries/getOrder", + errorName: "Error", + ...text, + ...("errorMessage" in text ? { errorStack: expect.any(String) } : {}), + }, + ], + ]); + }, + ); + + it("still answers its 500 and logs it as the configuration says under a BOUNDA_ERROR_TEXT it does not know", async () => { + const errors: unknown[][] = []; + const worker = createWorker({ + config: { + storage: cloudflare({ binding: "SPY" }), + runtime: { telemetry: { errorText: "include" } }, + }, + tenantOf: () => { + throw new Error("no session for ana@example.com"); + }, + logger: { + debug: () => undefined, + info: () => undefined, + warn: () => undefined, + error: (...entry) => void errors.push(entry), + }, + }); + const response = await worker.fetch?.( + new Request("https://bounda.test/queries/getOrder", { + method: "POST", + body: JSON.stringify({ orderId: "o-1" }), + }) as Request, + { + BOUNDA_ERROR_TEXT: "loud", + SPY: { idFromName: () => "id", get: () => recordingStub().stub }, + } as unknown as Cloudflare.Env, + createExecutionContext(), + ); + expect(response?.status).toBe(500); + expect(await response?.json()).toEqual({ + error: { code: "INTERNAL", message: "Internal error" }, + }); + expect(errors).toEqual([ + [ + "bounda worker request failed", + { + path: "/queries/getOrder", + errorName: "Error", + errorMessage: "no session for ana@example.com", + errorStack: expect.any(String), + }, + ], + ]); + }); + it("answers a 500 and logs it, without the value, when actorOf names no valid actor", async () => { const errors: unknown[][] = []; const logger: Logger = { diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index d73feb0b..b15a1f32 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -83,6 +83,17 @@ export type { ErrorFieldsFunction, } from "./kernel/shared/error-fields.ts"; export { errorFields } from "./kernel/shared/error-fields.ts"; +export type { + ResolveErrorTextArgs, + ResolveErrorTextFunction, + WithErrorTextArgs, + WithErrorTextFunction, +} from "./kernel/shared/error-text.ts"; +export { + ERROR_TEXT_VARIABLE, + resolveErrorText, + withErrorText, +} from "./kernel/shared/error-text.ts"; export type { IdempotencyKeyForFunction } from "./kernel/shared/idempotency-key.ts"; export { idempotencyKeyFor } from "./kernel/shared/idempotency-key.ts"; export type { AppStatus, SchemaStatus, SubscriberFailureStatus } from "./kernel/status.ts"; diff --git a/packages/core/src/kernel/shared/error-text.test.ts b/packages/core/src/kernel/shared/error-text.test.ts new file mode 100644 index 00000000..30912908 --- /dev/null +++ b/packages/core/src/kernel/shared/error-text.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, it } from "vitest"; +import type { Config, ErrorText } from "../../config/types.ts"; +import { ConfigurationError } from "../../contracts/errors.ts"; +import { memory } from "../../memory/index.ts"; +import { ERROR_TEXT_VARIABLE, resolveErrorText, withErrorText } from "./error-text.ts"; + +const configured = (errorText?: ErrorText): Config => ({ + storage: memory(), + ...(errorText === undefined ? {} : { runtime: { telemetry: { errorText } } }), +}); + +describe("resolveErrorText", () => { + it("names BOUNDA_ERROR_TEXT", () => { + expect(ERROR_TEXT_VARIABLE).toBe("BOUNDA_ERROR_TEXT"); + }); + + // forced, BOUNDA_ERROR_TEXT, configuration, fallback → rule + it.each([ + [undefined, undefined, undefined, undefined, "omit"], + [undefined, undefined, undefined, "include", "include"], + [undefined, undefined, "include", "omit", "include"], + [undefined, undefined, "omit", "include", "omit"], + [undefined, "include", "omit", "omit", "include"], + [undefined, "omit", "include", "include", "omit"], + [undefined, "", "include", undefined, "include"], + ["omit", "include", "include", "include", "omit"], + ["include", "omit", "omit", "omit", "include"], + ] as const)( + "forced %s over BOUNDA_ERROR_TEXT %j over the configuration's %s over the fallback %s: %s", + (forced, variable, errorText, fallback, rule) => { + const args = { + forced, + env: variable === undefined ? {} : { BOUNDA_ERROR_TEXT: variable }, + config: configured(errorText), + fallback, + }; + expect(resolveErrorText(args)).toBe(rule); + expect(withErrorText(args).runtime?.telemetry?.errorText).toBe(rule); + }, + ); + + it("reads no environment and no configuration it is not given", () => { + expect(resolveErrorText({})).toBe("omit"); + expect(resolveErrorText({ config: { storage: memory(), runtime: { role: "web" } } })).toBe( + "omit", + ); + expect(resolveErrorText({ fallback: "include" })).toBe("include"); + }); + + it("refuses a BOUNDA_ERROR_TEXT it does not know, unless told to ignore it", () => { + const env = { BOUNDA_ERROR_TEXT: "yes" }; + expect(() => resolveErrorText({ env })).toThrow( + new ConfigurationError('BOUNDA_ERROR_TEXT must be "omit" or "include", not "yes"'), + ); + expect(() => resolveErrorText({ env: { BOUNDA_ERROR_TEXT: true } })).toThrow( + ConfigurationError, + ); + expect(resolveErrorText({ env, config: configured("include"), ignoreInvalidEnv: true })).toBe( + "include", + ); + expect(resolveErrorText({ env, forced: "include" })).toBe("include"); + }); +}); + +describe("withErrorText", () => { + it("keeps the rest of the configuration", () => { + const config: Config = { + storage: memory(), + runtime: { role: "web", telemetry: { errorText: "omit" } }, + }; + expect(withErrorText({ config, forced: "include" })).toEqual({ + storage: config.storage, + runtime: { role: "web", telemetry: { errorText: "include" } }, + }); + expect(config.runtime?.telemetry?.errorText).toBe("omit"); + }); +}); diff --git a/packages/core/src/kernel/shared/error-text.ts b/packages/core/src/kernel/shared/error-text.ts new file mode 100644 index 00000000..b3fb238b --- /dev/null +++ b/packages/core/src/kernel/shared/error-text.ts @@ -0,0 +1,84 @@ +import type { Config, ErrorText } from "../../config/types.ts"; +import { ConfigurationError } from "../../contracts/errors.ts"; + +/** + * The environment variable that sets `runtime.telemetry.errorText` for a running app, over what + * the configuration says: `"omit"` or `"include"`. + */ +export const ERROR_TEXT_VARIABLE: "BOUNDA_ERROR_TEXT" = "BOUNDA_ERROR_TEXT"; + +export interface ResolveErrorTextArgs { + /** + * A host's own override, such as `bounda --error-text` or `boot({ errorText })`. + */ + readonly forced?: ErrorText | undefined; + /** + * Where `BOUNDA_ERROR_TEXT` is read: `process.env`, or a Worker's `env`. + */ + readonly env?: object | undefined; + readonly config?: Config | undefined; + /** + * What a host applies when nothing else says, such as the dev server's `"include"`. + */ + readonly fallback?: ErrorText | undefined; + /** + * Passes over a `BOUNDA_ERROR_TEXT` other than `"omit"` or `"include"` instead of throwing, for + * code that logs the failure of an app, which is likely that refusal; whatever starts the app + * should throw. + */ + readonly ignoreInvalidEnv?: boolean; +} + +export interface ResolveErrorTextFunction { + (args: ResolveErrorTextArgs): ErrorText; +} + +const fromEnv = (env: object, ignoreInvalid: boolean | undefined): ErrorText | undefined => { + const value: unknown = Reflect.get(env, ERROR_TEXT_VARIABLE); + if (value === "omit" || value === "include") return value; + if (value === undefined || value === "" || ignoreInvalid) return undefined; + throw new ConfigurationError( + `${ERROR_TEXT_VARIABLE} must be "omit" or "include", not ${JSON.stringify(value)}`, + ); +}; + +/** + * The rule of `runtime.telemetry.errorText` a host applies, the one precedence every host shares: + * its own override (`forced`), then `BOUNDA_ERROR_TEXT` in `env`, then the configuration, then + * the host's `fallback`, then `"omit"`. `boot()`, the CLI, the Durable Object and the Worker of + * `@bounda-dev/cloudflare` and the `handleError` of `@bounda-dev/react-router` all resolve it + * here. Throws `ConfigurationError` for a `BOUNDA_ERROR_TEXT` other than `"omit"` or `"include"` + * unless `ignoreInvalidEnv` is set. + */ +export const resolveErrorText: ResolveErrorTextFunction = ({ + forced, + env, + config, + fallback, + ignoreInvalidEnv, +}) => + forced ?? + (env === undefined ? undefined : fromEnv(env, ignoreInvalidEnv)) ?? + config?.runtime?.telemetry?.errorText ?? + fallback ?? + "omit"; + +export interface WithErrorTextArgs extends ResolveErrorTextArgs { + readonly config: Config; +} + +export interface WithErrorTextFunction { + (args: WithErrorTextArgs): Config; +} + +/** + * The configuration with `runtime.telemetry.errorText` set as `resolveErrorText` resolves it, for + * a host to create the app with. + */ +export const withErrorText: WithErrorTextFunction = (args) => ({ + ...args.config, + runtime: { + ...args.config.runtime, + telemetry: { ...args.config.runtime?.telemetry, errorText: resolveErrorText(args) }, + }, +}); diff --git a/packages/core/src/node/boot.test.ts b/packages/core/src/node/boot.test.ts index 525f0778..aacdb30c 100644 --- a/packages/core/src/node/boot.test.ts +++ b/packages/core/src/node/boot.test.ts @@ -1,4 +1,4 @@ -import { mkdir, mkdtemp, rm } from "node:fs/promises"; +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; @@ -20,6 +20,7 @@ const defaultRoot = join(dirname(fileURLToPath(import.meta.url)), "fixtures", "p afterEach(() => { delete process.env.BOUNDA_ROLE; delete process.env.BOUNDA_TEST_MARKER; + delete process.env.BOUNDA_ERROR_TEXT; }); describe("loadProject", () => { @@ -95,14 +96,22 @@ describe("boot", () => { await app.stop(); }); + const omitted = { errorName: "Error" } as const; + const included = { errorName: "Error", errorMessage: "ana@example.com" } as const; + it.each([ - [undefined, undefined, { errorName: "Error" }], - ["include", "omit", { errorName: "Error", errorMessage: "ana@example.com" }], - ["include", undefined, { errorName: "Error", errorMessage: "ana@example.com" }], - [undefined, "include", { errorName: "Error", errorMessage: "ana@example.com" }], + [undefined, undefined, undefined, omitted], + ["include", undefined, "omit", included], + ["include", undefined, undefined, included], + [undefined, undefined, "include", included], + [undefined, "include", "omit", included], + [undefined, "omit", "include", omitted], + [undefined, "", "include", included], + ["omit", "include", undefined, omitted], ] as const)( - "logs error text as errorText %s says over the configuration's %s", - async (errorText, configured, logged) => { + "logs error text as errorText %s, then BOUNDA_ERROR_TEXT %j, says over the configuration's %s", + async (errorText, variable, configured, logged) => { + if (variable !== undefined) process.env.BOUNDA_ERROR_TEXT = variable; const fields: unknown[] = []; const withPort = { ...registry, @@ -146,6 +155,37 @@ describe("boot", () => { }, ); + it("reads BOUNDA_ERROR_TEXT once .env is loaded", async () => { + const project = await mkdtemp(join(tmpdir(), "bounda-boot-")); + try { + await writeFile(join(project, ".env"), "BOUNDA_ERROR_TEXT=include\n"); + const app = await boot({ + root: project, + signals: false, + registry, + config: { storage: memory() }, + logger: silentLogger, + }); + expect(app.config.runtime.telemetry.errorText).toBe("include"); + await app.stop(); + } finally { + await rm(project, { recursive: true, force: true }); + } + }); + + it("refuses a BOUNDA_ERROR_TEXT it does not know", async () => { + process.env.BOUNDA_ERROR_TEXT = "verbose"; + await expect( + boot({ + root, + signals: false, + registry, + config: { storage: memory() }, + logger: silentLogger, + }), + ).rejects.toThrow(ConfigurationError); + }); + it("hands process.env, with .env loaded, to the implementations' create", async () => { let marker: string | undefined; const withPort = { diff --git a/packages/core/src/node/boot.ts b/packages/core/src/node/boot.ts index 2e5b0b48..22be3da2 100644 --- a/packages/core/src/node/boot.ts +++ b/packages/core/src/node/boot.ts @@ -8,6 +8,7 @@ import { ConfigurationError } from "../contracts/errors.ts"; import type { IdGenerator } from "../contracts/ids.ts"; import type { Logger } from "../contracts/logger.ts"; import { type BoundaApp, createApp } from "../kernel/app.ts"; +import { withErrorText } from "../kernel/shared/error-text.ts"; import { guardedLogger } from "../kernel/shared/guarded-logger.ts"; import type { Registry } from "../modules/registry.ts"; import type { AppRegistry } from "../register/index.ts"; @@ -59,7 +60,8 @@ export interface BootArgs { */ readonly logger?: Logger; /** - * Overrides `runtime.telemetry.errorText` for this app, as `bounda --error-text` does. + * Overrides `runtime.telemetry.errorText` for this app, as `bounda --error-text` does. Without + * it, `BOUNDA_ERROR_TEXT` does, when set. */ readonly errorText?: ErrorText; /** @@ -190,7 +192,8 @@ export const loadProject: LoadProjectFunction = async ({ root = process.cwd(), @@ -221,16 +224,8 @@ export const boot: BootFunction = async ({ const app = await createApp({ registry: project.registry, - config: - errorText === undefined - ? project.config - : { - ...project.config, - runtime: { - ...project.config.runtime, - telemetry: { ...project.config.runtime?.telemetry, errorText }, - }, - }, + // `process.env` is read once `.env` is loaded, which may set `BOUNDA_ERROR_TEXT`. + config: withErrorText({ config: project.config, forced: errorText, env: process.env }), logger: rawLogger, env: process.env, ...(ids === undefined ? {} : { ids }), diff --git a/packages/core/test-types/config.test-d.ts b/packages/core/test-types/config.test-d.ts index fca7c628..90315e74 100644 --- a/packages/core/test-types/config.test-d.ts +++ b/packages/core/test-types/config.test-d.ts @@ -1,8 +1,11 @@ import { type DurationInput, + ERROR_TEXT_VARIABLE, type ErrorFields, errorFields, + resolveErrorText, type SubscriberFailing, + withErrorText, } from "@bounda-dev/core"; import type { Adapter, @@ -53,6 +56,30 @@ describe("errorFields", () => { expectTypeOf().toEqualTypeOf(); expectTypeOf().not.toHaveProperty("message"); }); + + it("resolves one rule from a host's override, any environment, the configuration and a fallback", () => { + expectTypeOf(ERROR_TEXT_VARIABLE).toEqualTypeOf<"BOUNDA_ERROR_TEXT">(); + const processEnv: Readonly> = {}; + expectTypeOf(resolveErrorText({})).toEqualTypeOf(); + expectTypeOf( + resolveErrorText({ + forced: undefined, + env: processEnv, + config: { storage: sqlite, ports }, + fallback: "include", + ignoreInvalidEnv: true, + }), + ).toEqualTypeOf(); + expectTypeOf( + withErrorText({ config: { storage: sqlite, ports }, env: processEnv }), + ).toEqualTypeOf(); + // @ts-expect-error an environment is an object + resolveErrorText({ env: "include" }); + // @ts-expect-error a fallback is "omit" or "include" + resolveErrorText({ fallback: "all" }); + // @ts-expect-error withErrorText needs the configuration it sets the rule in + withErrorText({ forced: "include" }); + }); }); describe("defineConfig", () => { diff --git a/packages/create-bounda/src/run.e2e.test.ts b/packages/create-bounda/src/run.e2e.test.ts index e07d36f7..4cb830b8 100644 --- a/packages/create-bounda/src/run.e2e.test.ts +++ b/packages/create-bounda/src/run.e2e.test.ts @@ -375,6 +375,18 @@ describe("a project created by create-bounda", () => { const grace = await fetch(`${server.url}/?customer=grace`); expect(await rendered(grace)).toContain("grace: 1 order(s), 99 in total"); + // React Router hands what it throws to app/entry.server.tsx's handleError, which logs its + // text in development. + expect((await fetch(`${server.url}/nope/ana`)).status).toBe(404); + await until( + () => + /request failed \{"method":"GET","path":"\/nope\/ana","errorName":"Error","errorMessage":"No route matches URL/.test( + server.output(), + ), + () => `the dev server did not log the error's text:\n${server.output()}`, + 10_000, + ); + // An edited configuration reboots the app: here, on a database of its own. expect(original).toContain("./data/app.db"); await writeFile(config, original.replace("./data/app.db", "./data/edited.db")); @@ -405,6 +417,18 @@ describe("a project created by create-bounda", () => { const grace = await fetch(`${production.url}/?customer=grace`); expect(grace.status).toBe(200); expect(await rendered(grace)).toContain("grace: 1 order(s), 99 in total"); + + // The build logs no text of an error: an error's message may quote personal data. + expect((await fetch(`${production.url}/nope/ana`)).status).toBe(404); + await until( + () => + production + .output() + .includes('[bounda] error request failed {"method":"GET","errorName":"Error"}'), + () => `the build did not log the error:\n${production.output()}`, + 10_000, + ); + expect(production.output()).not.toContain("No route matches"); } finally { await production.stop(); } @@ -475,8 +499,10 @@ describe("a project created by create-bounda", () => { expect(await portTaken(server.port)).toBe(false); // Without the secret the object refuses to start, saying which one, rather than store keys in - // the clear; the Worker answers 500 without the reason, which it logs. + // the clear; the Worker answers 500 without the reason, which it logs, with its text as + // BOUNDA_ERROR_TEXT in .dev.vars says. await rename(join(project, ".dev.vars"), join(project, ".dev.vars.kept")); + await writeFile(join(project, ".dev.vars"), "BOUNDA_ERROR_TEXT=include\n"); const keyless = await startServer(project, { args: (port) => [wrangler, "dev", "--port", String(port), "--ip", "127.0.0.1"], }); @@ -502,6 +528,14 @@ describe("a project created by create-bounda", () => { () => `the object's failure was not logged:\n${keyless.output()}`, 10_000, ); + await until( + () => + /"message":"bounda worker request failed","path":"\/commands\/placeOrder","errorName":"[^"]+","errorMessage":"[^"]*BOUNDA_MASTER_KEY/.test( + keyless.output(), + ), + () => `the Worker did not log the failure's text:\n${keyless.output()}`, + 10_000, + ); } finally { await keyless.stop(); await rename(join(project, ".dev.vars.kept"), join(project, ".dev.vars")); @@ -573,21 +607,48 @@ describe("a project created by create-bounda", () => { "--strictPort", ], }); + // React Router hands what it throws to app/entry.server.tsx's handleError, which logs it as + // BOUNDA_ERROR_TEXT in .dev.vars says. + const notFound = async (url: string, output: () => string, line: RegExp): Promise => { + expect((await fetch(`${url}/nope/ana`)).status).toBe(404); + await until( + () => line.test(output()), + () => `the Worker did not log the error as expected:\n${output()}`, + 10_000, + ); + }; try { await placeAndList(server.url, "grace"); + await notFound( + server.url, + server.output, + /"message":"bounda request failed","method":"GET","path":"\/nope\/ana","errorName":"Error","errorMessage":"No route matches URL/, + ); } finally { await server.stop(); } expect(await portTaken(server.port)).toBe(false); - // What `wrangler deploy` would upload: the Worker the build wrote, with its own wrangler.json. + // What `wrangler deploy` would upload: the Worker the build wrote, with its own wrangler.json, + // here without BOUNDA_ERROR_TEXT, as a deployment has it. + const devVars = join(project, ".dev.vars"); + const vars = await readFile(devVars, "utf8"); + expect(vars).toMatch(/^BOUNDA_ERROR_TEXT=include$/m); + await writeFile(devVars, vars.replace(/^BOUNDA_ERROR_TEXT=.*$/m, "")); const production = await startServer(project, { args: (port) => [wrangler, "dev", "--port", String(port), "--ip", "127.0.0.1"], }); try { await placeAndList(production.url, "lin"); + await notFound( + production.url, + production.output, + /\{"level":"error","message":"bounda request failed","method":"GET","errorName":"Error"\}/, + ); + expect(production.output()).not.toContain("No route matches"); } finally { await production.stop(); + await writeFile(devVars, vars); } }, 300_000); }); diff --git a/packages/create-bounda/src/run.test.ts b/packages/create-bounda/src/run.test.ts index bcfa1767..baab13f6 100644 --- a/packages/create-bounda/src/run.test.ts +++ b/packages/create-bounda/src/run.test.ts @@ -218,7 +218,7 @@ describe("create-bounda", () => { cwd, ); expect(result.code).toBe(EXIT_OK); - expect(result.stdout).toContain("created web in web (23 files, node, react-router, sqlite)"); + expect(result.stdout).toContain("created web in web (24 files, node, react-router, sqlite)"); expect(result.stdout).toContain("next:\n cd web\n pnpm install\n pnpm test\n pnpm dev\n"); expect(await readdir(join(cwd, "web", "app", "routes"))).toEqual(["home.tsx"]); }); @@ -231,7 +231,7 @@ describe("create-bounda", () => { ); expect(result.code).toBe(EXIT_OK); expect(result.stdout).toContain( - "created web in web (28 files, cloudflare, react-router, durable-object)", + "created web in web (29 files, cloudflare, react-router, durable-object)", ); expect(result.stdout).toContain("next:\n cd web\n pnpm install\n pnpm test\n pnpm dev\n"); }); diff --git a/packages/create-bounda/src/scaffold.test.ts b/packages/create-bounda/src/scaffold.test.ts index 1776e7e7..67cd48f6 100644 --- a/packages/create-bounda/src/scaffold.test.ts +++ b/packages/create-bounda/src/scaffold.test.ts @@ -155,6 +155,7 @@ describe("scaffoldProject", () => { "app/app.css", "app/domain/order/commands/place-order.ts", "app/domain/order/order-placed.ts", + "app/entry.server.tsx", "app/read/orders/projections/bounda/personal-data-forgotten.ts", "app/read/orders/projections/order/order-placed.ts", "app/read/orders/queries/list-orders.ts", @@ -297,6 +298,7 @@ describe("scaffoldProject", () => { "app/app.css", "app/domain/order/commands/place-order.ts", "app/domain/order/order-placed.ts", + "app/entry.server.tsx", "app/read/orders/projections/bounda/personal-data-forgotten.ts", "app/read/orders/projections/order/order-placed.ts", "app/read/orders/queries/list-orders.ts", @@ -445,6 +447,9 @@ describe("scaffoldProject", () => { expect(await readFile(join(given, ".dev.vars"), "utf8")).toMatch( /^BOUNDA_MASTER_KEY=given-key$/m, ); + expect(await readFile(join(given, ".dev.vars"), "utf8")).toMatch( + /^BOUNDA_ERROR_TEXT=include$/m, + ); expect(await readFile(join(given, ".dev.vars.example"), "utf8")).toMatch( /^BOUNDA_MASTER_KEY=$/m, ); diff --git a/packages/create-bounda/template/cloudflare-react-router/README.md.tpl b/packages/create-bounda/template/cloudflare-react-router/README.md.tpl index e34d8deb..8fa5679a 100644 --- a/packages/create-bounda/template/cloudflare-react-router/README.md.tpl +++ b/packages/create-bounda/template/cloudflare-react-router/README.md.tpl @@ -23,8 +23,10 @@ app/domain/order/ the order aggregate app/read/orders/ a read model app/routes/home.tsx a loader that queries and an action that dispatches app/root.tsx mounts boundaMiddleware +app/entry.server.tsx React Router's entry, with Bounda's handleError app/tenant.ts names the Durable Object each request reaches bounda.config.ts storage: cloudflare() +.dev.vars the master key, and the text of errors in the logs, for development workers/app.ts the Worker: React Router, and the Durable Object class vite.config.ts plugins: [cloudflare(), bounda(), reactRouter()] wrangler.jsonc the binding and the SQLite migration for the object @@ -37,4 +39,9 @@ everyone is a choice; to keep each customer's data apart, name theirs there, fro signed-in user. The [React Router guide](https://docs.bounda.dev/guides/react-router/#on-cloudflare) has the details. +`app/entry.server.tsx` exports Bounda's `handleError`, so what a loader or an action throws is +logged without its message, which may quote personal data, unless `BOUNDA_ERROR_TEXT=include` +says otherwise, as `.dev.vars` does in development for the Worker and the Durable Object alike. +See [logging errors](https://docs.bounda.dev/guides/react-router/#logging-errors). + `.bounda/`, `+types/` and `.react-router/` are generated; they stay out of git. diff --git a/packages/create-bounda/template/cloudflare-react-router/app/entry.server.tsx b/packages/create-bounda/template/cloudflare-react-router/app/entry.server.tsx new file mode 100644 index 00000000..162ef768 --- /dev/null +++ b/packages/create-bounda/template/cloudflare-react-router/app/entry.server.tsx @@ -0,0 +1,49 @@ +import { handleError } from "@bounda-dev/react-router/app"; +import { isbot } from "isbot"; +import { renderToReadableStream } from "react-dom/server"; +import { type EntryContext, ServerRouter } from "react-router"; + +// React Router's default entry for web streams, as a Worker runs it, with Bounda's handleError: it +// logs what a loader, an action or the render throws as BOUNDA_ERROR_TEXT, or else +// runtime.telemetry.errorText, says, where the default logs every message and stack, and a +// message may quote personal data. +export { handleError }; + +export const streamTimeout = 5_000; + +export default async function handleRequest( + request: Request, + responseStatusCode: number, + responseHeaders: Headers, + routerContext: EntryContext, +) { + // https://httpwg.org/specs/rfc9110.html#HEAD + if (request.method.toUpperCase() === "HEAD") { + return new Response(null, { status: responseStatusCode, headers: responseHeaders }); + } + + let status = responseStatusCode; + let shellRendered = false; + const userAgent = request.headers.get("user-agent"); + + const body = await renderToReadableStream( + , + { + signal: AbortSignal.timeout(streamTimeout + 1000), + onError(error: unknown) { + status = 500; + // An error before the shell rejects, and React Router hands it to handleError itself. + if (shellRendered) void handleError(error, { request }); + }, + }, + ); + shellRendered = true; + + // Bots and SPA mode wait for all content before the response starts. + if ((userAgent && isbot(userAgent)) || routerContext.isSpaMode) { + await body.allReady; + } + + responseHeaders.set("Content-Type", "text/html"); + return new Response(body, { headers: responseHeaders, status }); +} diff --git a/packages/create-bounda/template/cloudflare/README.md.tpl b/packages/create-bounda/template/cloudflare/README.md.tpl index 17828775..2c8a37eb 100644 --- a/packages/create-bounda/template/cloudflare/README.md.tpl +++ b/packages/create-bounda/template/cloudflare/README.md.tpl @@ -36,6 +36,7 @@ bounda.config.ts storage: cloudflare() src/worker.ts the Durable Object class and the HTTP API public/index.html a page that uses the API, served as a static asset wrangler.jsonc the binding and the SQLite migration for the object +.dev.vars the master key, and the text of errors in the logs, for development tests/orders.test.ts the domain, on an in-memory store tests/api.test.ts the API, against the Durable Object in workerd ``` diff --git a/packages/create-bounda/template/durable-object/_dev.vars.example b/packages/create-bounda/template/durable-object/_dev.vars.example index 89c9e126..b56dd7b3 100644 --- a/packages/create-bounda/template/durable-object/_dev.vars.example +++ b/packages/create-bounda/template/durable-object/_dev.vars.example @@ -2,3 +2,6 @@ # `openssl rand -base64 32` prints. Set it as a secret when you deploy: # npx wrangler secret put BOUNDA_MASTER_KEY BOUNDA_MASTER_KEY= +# In .dev.vars only, so the logs carry the message and the stack of errors in development; never +# set it when you deploy, since a message may quote personal data: +# BOUNDA_ERROR_TEXT=include diff --git a/packages/create-bounda/template/durable-object/_dev.vars.tpl b/packages/create-bounda/template/durable-object/_dev.vars.tpl index 730ad327..eae8f420 100644 --- a/packages/create-bounda/template/durable-object/_dev.vars.tpl +++ b/packages/create-bounda/template/durable-object/_dev.vars.tpl @@ -2,3 +2,6 @@ # their data without it: 32 random bytes in base64, generated for this project. Keep it secret # and out of version control. https://docs.bounda.dev/guides/personal-data/#where-the-keys-live BOUNDA_MASTER_KEY={{masterKey}} +# Development only: the logs carry the message and the stack of errors, which may quote personal +# data. Leave it unset when you deploy. https://docs.bounda.dev/reference/observability/#error-text +BOUNDA_ERROR_TEXT=include diff --git a/packages/create-bounda/template/node-react-router/README.md.tpl b/packages/create-bounda/template/node-react-router/README.md.tpl index 14c1e2c3..f3b5b636 100644 --- a/packages/create-bounda/template/node-react-router/README.md.tpl +++ b/packages/create-bounda/template/node-react-router/README.md.tpl @@ -23,6 +23,7 @@ app/read/orders/ a read model queries/list-orders.ts app/routes/home.tsx a loader that queries and an action that dispatches app/root.tsx mounts boundaMiddleware +app/entry.server.tsx React Router's entry, with Bounda's handleError vite.config.ts plugins: [bounda(), reactRouter()] bounda.config.ts storage and ports tests/orders.test.ts the app on an in-memory adapter @@ -30,7 +31,9 @@ tests/orders.test.ts the app on an in-memory adapter The `bounda()` Vite plugin generates the types when the dev server starts and after every change under `app/domain` and `app/read`, and serves `@bounda-dev/react-router/app`: the `bounda` -context for loaders and actions and the `boundaMiddleware` that boots the app. The +context for loaders and actions, the `boundaMiddleware` that boots the app, and the +`handleError` that `app/entry.server.tsx` exports, which logs what a request throws without its +message, which may quote personal data, except under the dev server. The [React Router guide](https://docs.bounda.dev/guides/react-router/) has the details; the [project layout guide](https://docs.bounda.dev/guides/project-layout/) covers every kind of module. diff --git a/packages/create-bounda/template/node-react-router/app/entry.server.tsx b/packages/create-bounda/template/node-react-router/app/entry.server.tsx new file mode 100644 index 00000000..370d15f6 --- /dev/null +++ b/packages/create-bounda/template/node-react-router/app/entry.server.tsx @@ -0,0 +1,69 @@ +import { PassThrough } from "node:stream"; +import { handleError } from "@bounda-dev/react-router/app"; +import { createReadableStreamFromReadable } from "@react-router/node"; +import { isbot } from "isbot"; +import { type RenderToPipeableStreamOptions, renderToPipeableStream } from "react-dom/server"; +import { type EntryContext, ServerRouter } from "react-router"; + +// React Router's default entry for Node, with Bounda's handleError: it logs what a loader, an +// action or the render throws as runtime.telemetry.errorText says, where the default logs every +// message and stack, and a message may quote personal data. +export { handleError }; + +export const streamTimeout = 5_000; + +export default function handleRequest( + request: Request, + responseStatusCode: number, + responseHeaders: Headers, + routerContext: EntryContext, +) { + // https://httpwg.org/specs/rfc9110.html#HEAD + if (request.method.toUpperCase() === "HEAD") { + return new Response(null, { status: responseStatusCode, headers: responseHeaders }); + } + + return new Promise((resolve, reject) => { + let status = responseStatusCode; + let shellRendered = false; + const userAgent = request.headers.get("user-agent"); + + // Bots and SPA mode wait for all content before the response starts. + const readyOption: keyof RenderToPipeableStreamOptions = + (userAgent && isbot(userAgent)) || routerContext.isSpaMode ? "onAllReady" : "onShellReady"; + + // Aborts the render after streamTimeout, leaving it time to flush the rejected boundaries. + let timeoutId: ReturnType | undefined = setTimeout( + () => abort(), + streamTimeout + 1000, + ); + + const { pipe, abort } = renderToPipeableStream( + , + { + [readyOption]() { + shellRendered = true; + const body = new PassThrough({ + final(callback) { + clearTimeout(timeoutId); + timeoutId = undefined; + callback(); + }, + }); + const stream = createReadableStreamFromReadable(body); + responseHeaders.set("Content-Type", "text/html"); + pipe(body); + resolve(new Response(stream, { headers: responseHeaders, status })); + }, + onShellError(error: unknown) { + reject(error); + }, + onError(error: unknown) { + status = 500; + // An error before the shell rejects, and React Router hands it to handleError itself. + if (shellRendered) void handleError(error, { request }); + }, + }, + ); + }); +} diff --git a/packages/react-router/src/app.ts b/packages/react-router/src/app.ts index 860c3c48..26d75df8 100644 --- a/packages/react-router/src/app.ts +++ b/packages/react-router/src/app.ts @@ -27,3 +27,10 @@ export const boundaMiddleware: Bounda["boundaMiddleware"] = provided.boundaMiddl * Stops the running app and forgets it; the next request boots again. */ export const dispose: Bounda["dispose"] = provided.dispose; + +/** + * React Router's `handleError`: `export { handleError } from "@bounda-dev/react-router/app"` in + * `app/entry.server.tsx`, and call it from the render's `onError`. Logs what a request throws as + * `runtime.telemetry.errorText` says, where React Router's default logs every message and stack. + */ +export const handleError: Bounda["handleError"] = provided.handleError; diff --git a/packages/react-router/src/cloudflare/create-bounda.test.ts b/packages/react-router/src/cloudflare/create-bounda.test.ts index 7053882e..56c0faf2 100644 --- a/packages/react-router/src/cloudflare/create-bounda.test.ts +++ b/packages/react-router/src/cloudflare/create-bounda.test.ts @@ -7,7 +7,7 @@ import { idempotencyKeyFor, } from "@bounda-dev/core"; import { type RouterContext, RouterContextProvider } from "react-router"; -import { afterEach, describe, expect, expectTypeOf, it } from "vitest"; +import { afterEach, describe, expect, expectTypeOf, it, vi } from "vitest"; import { createBounda } from "./create-bounda.ts"; import { env, storeNamespace } from "./test-support.ts"; @@ -145,3 +145,86 @@ describe("createBounda on Cloudflare", () => { ).toThrow(ConfigurationError); }); }); + +describe("createBounda on Cloudflare: handleError", () => { + const recording = () => { + const lines: unknown[][] = []; + return { + lines, + logger: { + debug: () => undefined, + info: () => undefined, + warn: () => undefined, + error: (...entry: unknown[]) => void lines.push(entry), + }, + }; + }; + + afterEach(() => { + delete env.BOUNDA_ERROR_TEXT; + }); + + it.each([ + [undefined, undefined, false], + [undefined, "unset", false], + [undefined, "include", true], + ["include", "omit", true], + ["omit", "include", false], + ["loud", "include", true], + ] as const)( + "logs the text as BOUNDA_ERROR_TEXT %s says, over the configuration's %s", + async (variable, configured, included) => { + env.STORE = storeNamespace().namespace; + if (variable !== undefined) env.BOUNDA_ERROR_TEXT = variable; + const { lines, logger } = recording(); + const { handleError } = createBounda({ + config: { + storage: cloudflare(), + ...(configured === undefined + ? {} + : { + runtime: + configured === "unset" + ? { role: "web" as const } + : { telemetry: { errorText: configured } }, + }), + }, + tenant: () => "default", + logger, + }); + await handleError(new Error("ana@example.com"), { request: new Request("http://w/a") }); + expect(lines).toEqual([ + [ + "bounda request failed", + included + ? { + method: "GET", + path: "/a", + errorName: "Error", + errorMessage: "ana@example.com", + errorStack: expect.any(String), + } + : { method: "GET", errorName: "Error" }, + ], + ]); + }, + ); + + it("logs one JSON line per entry by default, as Workers Logs reads them", async () => { + env.STORE = storeNamespace().namespace; + const written: unknown[][] = []; + const spy = vi.spyOn(console, "error").mockImplementation((...args) => void written.push(args)); + try { + const { handleError } = createBounda({ + config: { storage: cloudflare() }, + tenant: () => "default", + }); + await handleError(new Error("ana@example.com"), { request: new Request("http://w/a") }); + } finally { + spy.mockRestore(); + } + expect(written).toEqual([ + ['{"level":"error","message":"bounda request failed","method":"GET","errorName":"Error"}'], + ]); + }); +}); diff --git a/packages/react-router/src/cloudflare/create-bounda.ts b/packages/react-router/src/cloudflare/create-bounda.ts index 08feefff..8274d2f7 100644 --- a/packages/react-router/src/cloudflare/create-bounda.ts +++ b/packages/react-router/src/cloudflare/create-bounda.ts @@ -3,12 +3,14 @@ import { type AppRegistry, type Consistency, checkConsistency, + type Logger, type Registry, } from "@bounda-dev/core"; import type { Config } from "@bounda-dev/core/config"; import type { ActorFunction } from "../actor.ts"; import type { Bounda } from "../create-bounda.ts"; import { cloudflareClients, type TenantFunction } from "./clients.ts"; +import { cloudflareHandleError } from "./handle-error.ts"; import { serve } from "./serve.ts"; export interface CreateBoundaArgs { @@ -28,6 +30,11 @@ export interface CreateBoundaArgs { * act as nobody. */ readonly actor?: ActorFunction; + /** + * Where `handleError` logs. Defaults to `workersLogger` from `@bounda-dev/cloudflare`, one JSON + * line per entry. + */ + readonly logger?: Logger; } export interface CreateBoundaFunction { @@ -37,15 +44,17 @@ export interface CreateBoundaFunction { /** * Wires a React Router app served by a Worker to the Bounda Durable Objects it binds: the * `bounda` context holds, in every loader and action, a client for the store `tenant` names, - * reached through `connect`. Declare it once in a server module and mount the middleware in - * `root.tsx`; `dispose` does nothing, since the app runs in the objects. A command's `signal` + * reached through `connect`. Declare it once in a server module, mount the middleware in + * `root.tsx` and export `handleError` from `app/entry.server.tsx`, which logs what a request + * throws as `BOUNDA_ERROR_TEXT` in the Worker's `env` says, or else the configuration's + * `runtime.telemetry.errorText`; `dispose` does nothing, since the app runs in the objects. A command's `signal` * only counts before the call leaves the Worker. Throws `ConfigurationError` when `storage` is not * `cloudflare()`, when the Worker has no such binding, without a `tenant`, or for a `consistency` * it does not know. * * @example * // app/bounda.server.ts - * export const { bounda, boundaMiddleware } = createBounda({ + * export const { bounda, boundaMiddleware, handleError } = createBounda({ * config, * tenant: ({ params }) => params.workspace ?? "default", * }); @@ -55,8 +64,10 @@ export const createBounda: CreateBoundaFunction = => { checkConsistency(consistency); const clientOf = cloudflareClients({ env, config, tenant, consistency }); - return serve(() => clientOf, actor); + const handleError = cloudflareHandleError({ config: () => config, logger }); + return serve(() => clientOf, handleError, actor); }; diff --git a/packages/react-router/src/cloudflare/handle-error.ts b/packages/react-router/src/cloudflare/handle-error.ts new file mode 100644 index 00000000..ed4da877 --- /dev/null +++ b/packages/react-router/src/cloudflare/handle-error.ts @@ -0,0 +1,29 @@ +import { env } from "cloudflare:workers"; +import { workersLogger } from "@bounda-dev/cloudflare"; +import { type Logger, resolveErrorText } from "@bounda-dev/core"; +import type { Config } from "@bounda-dev/core/config"; +import { createHandleError, type HandleBoundaErrorFunction } from "../handle-error.ts"; + +export interface CloudflareHandleErrorArgs { + /** + * The configuration the Worker holds, `undefined` while it has imported none. + */ + readonly config: () => Config | undefined; + readonly logger?: Logger | undefined; +} + +export interface CloudflareHandleErrorFunction { + (args: CloudflareHandleErrorArgs): HandleBoundaErrorFunction; +} + +// The configuration is one bundle for every environment, so the Worker's `env` decides first, as +// it does for the Durable Object. +export const cloudflareHandleError: CloudflareHandleErrorFunction = ({ + config, + logger = workersLogger, +}) => + createHandleError({ + message: "bounda request failed", + errorText: () => resolveErrorText({ env, config: config(), ignoreInvalidEnv: true }), + logger: () => logger, + }); diff --git a/packages/react-router/src/cloudflare/serve.ts b/packages/react-router/src/cloudflare/serve.ts index 0e58a6c7..a14b830b 100644 --- a/packages/react-router/src/cloudflare/serve.ts +++ b/packages/react-router/src/cloudflare/serve.ts @@ -2,18 +2,21 @@ import type { BoundaClient, Registry } from "@bounda-dev/core"; import { createContext } from "react-router"; import { type ActorFunction, actAs } from "../actor.ts"; import type { Bounda, BoundaMiddleware } from "../create-bounda.ts"; +import type { HandleBoundaErrorFunction } from "../handle-error.ts"; import { withIdempotencyKey } from "../idempotency-key.ts"; import type { ClientOfFunction } from "./clients.ts"; export interface ServeFunction { ( load: () => ClientOfFunction | Promise>, + handleError: HandleBoundaErrorFunction, actor?: ActorFunction, ): Bounda; } export const serve: ServeFunction = ( load: () => ClientOfFunction | Promise>, + handleError: HandleBoundaErrorFunction, actor?: ActorFunction, ): Bounda => { const bounda = createContext>(); @@ -35,5 +38,5 @@ export const serve: ServeFunction = ( }; // The app lives in its Durable Objects: nothing runs in the Worker to stop. - return { bounda, boundaMiddleware, dispose: async () => {} }; + return { bounda, boundaMiddleware, dispose: async () => {}, handleError }; }; diff --git a/packages/react-router/src/create-bounda.default.test.ts b/packages/react-router/src/create-bounda.default.test.ts index a2d98166..41a1ae31 100644 --- a/packages/react-router/src/create-bounda.default.test.ts +++ b/packages/react-router/src/create-bounda.default.test.ts @@ -12,6 +12,7 @@ const app = { stop: async () => { started.push("stop"); }, + config: { runtime: { telemetry: { errorText: "omit" } } }, } as unknown as BoundaApp; vi.mock("@bounda-dev/core/node", () => ({ boot: vi.fn(async () => app) })); diff --git a/packages/react-router/src/create-bounda.test.ts b/packages/react-router/src/create-bounda.test.ts index 40cbc7df..59026db8 100644 --- a/packages/react-router/src/create-bounda.test.ts +++ b/packages/react-router/src/create-bounda.test.ts @@ -5,7 +5,7 @@ import { idempotencyKeyFor, } from "@bounda-dev/core"; import { RouterContextProvider } from "react-router"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { createBounda } from "./create-bounda.ts"; interface FakeApp { @@ -34,6 +34,7 @@ const fakeApp = (name: string): FakeApp => { stop: async () => { calls.push("stop"); }, + config: { runtime: { telemetry: { errorText: "omit" } } }, } as unknown as BoundaApp; return { app, calls }; }; @@ -71,6 +72,7 @@ const actingApp = (): { readonly app: BoundaApp; readonly calls: unknown[][] } = }, start: () => undefined, stop: async () => undefined, + config: { runtime: { telemetry: { errorText: "omit" } } }, } as unknown as BoundaApp; return { app, calls }; }; @@ -190,6 +192,7 @@ describe("createBounda", () => { }, start: () => undefined, stop: async () => undefined, + config: { runtime: { telemetry: { errorText: "omit" } } }, } as unknown as BoundaApp; const { bounda, boundaMiddleware } = createBounda({ consistency: "eventual", @@ -227,6 +230,7 @@ describe("createBounda", () => { }, start: () => undefined, stop: async () => undefined, + config: { runtime: { telemetry: { errorText: "omit" } } }, } as unknown as BoundaApp; const { bounda, boundaMiddleware } = createBounda({ consistency: "eventual", @@ -607,3 +611,142 @@ describe("createBounda", () => { await expect(disposing).resolves.toBeUndefined(); }); }); + +describe("createBounda's handleError", () => { + const thrown = () => new Error("no account for ana@example.com"); + const request = () => new Request("http://localhost/accounts"); + const recording = () => { + const lines: unknown[][] = []; + return { + lines, + logger: { + debug: () => undefined, + info: () => undefined, + warn: () => undefined, + error: (...entry: unknown[]) => void lines.push(entry), + }, + }; + }; + const withErrorText = (errorText: "omit" | "include"): BoundaApp => + ({ + ...fakeApp("a").app, + config: { runtime: { telemetry: { errorText } } }, + }) as unknown as BoundaApp; + + it("follows the booted app's errorText, and errorText before any app booted", async () => { + const { lines, logger } = recording(); + let booting: BoundaApp = withErrorText("include"); + const { boundaMiddleware, dispose, handleError } = createBounda({ + boot: async () => booting, + key: uniqueKey(), + logger, + errorText: "include", + }); + await handleError(thrown(), { request: request() }); + expect(lines.at(-1)?.[1]).toMatchObject({ errorMessage: "no account for ana@example.com" }); + + booting = withErrorText("omit"); + await boundaMiddleware(middlewareArgs().args, async () => undefined); + await handleError(thrown(), { request: request() }); + expect(lines.at(-1)).toEqual(["request failed", { method: "GET", errorName: "Error" }]); + await dispose(); + }); + + it("omits the text before any app booted by default, also when the boot failed", async () => { + const { lines, logger } = recording(); + const { boundaMiddleware, handleError } = createBounda({ + boot: async () => { + throw thrown(); + }, + key: uniqueKey(), + logger, + }); + const failed = await boundaMiddleware(middlewareArgs().args, async () => undefined).catch( + (error: unknown) => error, + ); + await handleError(failed, { request: request() }); + expect(lines).toEqual([["request failed", { method: "GET", errorName: "Error" }]]); + }); + + it("follows BOUNDA_ERROR_TEXT before any app booted, as boot() would, over errorText", async () => { + const { lines, logger } = recording(); + const { handleError } = createBounda({ + boot: async () => fakeApp("a").app, + key: uniqueKey(), + logger, + errorText: "omit", + }); + try { + process.env.BOUNDA_ERROR_TEXT = "include"; + await handleError(thrown(), { request: request() }); + process.env.BOUNDA_ERROR_TEXT = "loud"; + await handleError(thrown(), { request: request() }); + } finally { + delete process.env.BOUNDA_ERROR_TEXT; + } + expect(lines.map(([, fields]) => "errorMessage" in (fields as object))).toEqual([true, false]); + }); + + it("logs without waiting for a boot in flight, and forgets the rule of an app it disposed", async () => { + const { lines, logger } = recording(); + let finish: (app: BoundaApp) => void = () => undefined; + const pending = new Promise((resolve) => { + finish = resolve; + }); + const { boundaMiddleware, dispose, handleError } = createBounda({ + boot: () => pending, + key: uniqueKey(), + logger, + }); + const booting = boundaMiddleware(middlewareArgs().args, async () => undefined); + await handleError(thrown(), { request: request() }); + expect(lines).toHaveLength(1); + finish(withErrorText("include")); + await booting; + await handleError(thrown(), { request: request() }); + expect(lines.at(-1)?.[1]).toMatchObject({ errorMessage: "no account for ana@example.com" }); + await dispose(); + await handleError(thrown(), { request: request() }); + expect(lines.at(-1)).toEqual(["request failed", { method: "GET", errorName: "Error" }]); + }); + + it("keeps no rule of a boot that finishes after it was disposed", async () => { + const { lines, logger } = recording(); + let finish: (app: BoundaApp) => void = () => undefined; + const { boundaMiddleware, dispose, handleError } = createBounda({ + boot: () => + new Promise((resolve) => { + finish = resolve; + }), + key: uniqueKey(), + logger, + }); + const booting = boundaMiddleware(middlewareArgs().args, async () => undefined); + await Promise.resolve(); + const disposed = dispose(); + finish(withErrorText("include")); + await disposed; + await booting; + await handleError(thrown(), { request: request() }); + expect(lines).toEqual([["request failed", { method: "GET", errorName: "Error" }]]); + }); + + it("logs a line to the console by default, as boot() does", async () => { + const written: unknown[][] = []; + const spy = vi.spyOn(console, "error").mockImplementation((...args) => void written.push(args)); + try { + const { handleError } = createBounda({ + boot: async () => fakeApp("a").app, + key: uniqueKey(), + }); + await handleError(thrown(), { request: request() }); + await handleError(thrown(), { request: request() }); + } finally { + spy.mockRestore(); + } + expect(written).toEqual([ + ['[bounda] error request failed {"method":"GET","errorName":"Error"}'], + ['[bounda] error request failed {"method":"GET","errorName":"Error"}'], + ]); + }); +}); diff --git a/packages/react-router/src/create-bounda.ts b/packages/react-router/src/create-bounda.ts index ed7ee985..516682ed 100644 --- a/packages/react-router/src/create-bounda.ts +++ b/packages/react-router/src/create-bounda.ts @@ -4,11 +4,15 @@ import { type BoundaClient, type Consistency, checkConsistency, + type Logger, type Registry, readYourWrites, + resolveErrorText, } from "@bounda-dev/core"; +import type { ErrorText } from "@bounda-dev/core/config"; import { createContext, type MiddlewareFunction, type RouterContext } from "react-router"; import { type ActorFunction, actAs } from "./actor.ts"; +import { createHandleError, type HandleBoundaErrorFunction } from "./handle-error.ts"; import { withIdempotencyKey } from "./idempotency-key.ts"; /** @@ -42,6 +46,17 @@ export interface CreateBoundaArgs { * nobody. */ readonly actor?: ActorFunction; + /** + * Where `handleError` logs. Defaults to `createConsoleLogger()` from `@bounda-dev/core/node`, + * the logger `boot()` defaults to. + */ + readonly logger?: Logger; + /** + * How `handleError` logs an error thrown while no app has booted, such as the boot's own + * failure, unless `BOUNDA_ERROR_TEXT` says otherwise. Defaults to `"omit"`; once an app has + * booted, `handleError` follows its `runtime.telemetry.errorText`. + */ + readonly errorText?: ErrorText | undefined; } /** @@ -70,6 +85,7 @@ export interface Bounda { readonly bounda: RouterContext>; readonly boundaMiddleware: BoundaMiddleware; readonly dispose: DisposeBoundaFunction; + readonly handleError: HandleBoundaErrorFunction; } export interface CreateBoundaFunction { @@ -79,6 +95,10 @@ export interface CreateBoundaFunction { interface Slot { app?: Promise> | undefined; retired?: Promise | undefined; + /** + * The rule of the app `app` resolved to, read without waiting for a boot in flight. + */ + errorText?: ErrorText | undefined; } const DEFAULT_KEY = "bounda.app"; @@ -101,6 +121,7 @@ const started = (app: BoundaApp): BoundaApp => { const retire = (slot: Slot): Promise => { const running = slot.app; slot.app = undefined; + slot.errorText = undefined; if (running === undefined && slot.retired === undefined) return Promise.resolve(); const stopping = (slot.retired ?? Promise.resolve()).then(async () => { const app = await running?.catch(() => undefined); @@ -123,7 +144,10 @@ const load = ( if (slot.app !== undefined) return slot.app; const starting = (slot.retired === undefined ? bootApp() : slot.retired.then(() => bootApp())) .then(started) - .then((app) => (consistency === "read-your-writes" ? readYourWrites(app) : app)); + .then((app) => { + if (slot.app === starting) slot.errorText = app.config.runtime.telemetry.errorText; + return consistency === "read-your-writes" ? readYourWrites(app) : app; + }); slot.app = starting; starting.catch(() => { if (slot.app === starting) slot.app = undefined; @@ -133,19 +157,25 @@ const load = ( /** * Wires Bounda into a React Router app: a context for the running app and the middleware that - * boots it once and provides it to every loader and action. Declare it once in a server module - * and mount the middleware in `root.tsx`. By default a command resolves once the read models + * boots it once and provides it to every loader and action. Declare it once in a server module, + * mount the middleware in `root.tsx` and export `handleError` from `app/entry.server.tsx`, which + * logs what a request throws as the app's `runtime.telemetry.errorText` says. By default a command resolves once the read models * reflect it (see `consistency`). When the server module is re-evaluated in development, the next * request boots from the new modules only once the previous app has stopped, so the two never * hold the storage at once. Throws `ConfigurationError` for a `consistency` it does not know. * * @example * // app/bounda.server.ts - * export const { bounda, boundaMiddleware } = createBounda({ boot: () => boot({ registry }) }); + * export const { bounda, boundaMiddleware, handleError } = createBounda({ + * boot: () => boot({ registry }), + * }); * * // app/root.tsx * export const middleware = [boundaMiddleware]; * + * // app/entry.server.tsx + * export { handleError } from "./bounda.server.ts"; + * * // app/routes/user.tsx * export const action = ({ params, context }: Route.ActionArgs) => * context.get(bounda).commands.activateUser({ userId: params.userId }); @@ -156,11 +186,14 @@ export const createBounda: CreateBoundaFunction = = {}): Bounda => { checkConsistency(consistency); const bounda = createContext>(); const slot = slotFor(key); void retire(slot); + let consoleLogger: Promise | undefined; const boundaMiddleware: BoundaMiddleware = async (args, next) => { const client = await actAs(await load(slot, bootApp, consistency), actor, args); @@ -170,5 +203,18 @@ export const createBounda: CreateBoundaFunction = retire(slot); - return { bounda, boundaMiddleware, dispose }; + const handleError = createHandleError({ + message: "request failed", + // Before any app has booted, what `boot()` would resolve without the configuration. + errorText: () => + slot.errorText ?? + resolveErrorText({ env: process.env, fallback: errorText, ignoreInvalidEnv: true }), + logger: () => { + if (logger !== undefined) return logger; + consoleLogger ??= import("@bounda-dev/core/node").then((node) => node.createConsoleLogger()); + return consoleLogger; + }, + }); + + return { bounda, boundaMiddleware, dispose, handleError }; }; diff --git a/packages/react-router/src/handle-error.test.ts b/packages/react-router/src/handle-error.test.ts new file mode 100644 index 00000000..99864797 --- /dev/null +++ b/packages/react-router/src/handle-error.test.ts @@ -0,0 +1,243 @@ +import { inspect } from "node:util"; +import type { BoundaApp, Logger } from "@bounda-dev/core"; +import type { ErrorText } from "@bounda-dev/core/config"; +import { + createRequestHandler, + type HandleErrorFunction, + type ServerBuild, + UNSAFE_ErrorResponseImpl, +} from "react-router"; +import { afterEach, describe, expect, expectTypeOf, it, vi } from "vitest"; +import { createBounda } from "./create-bounda.ts"; +import { createHandleError, type HandleBoundaErrorFunction } from "./handle-error.ts"; + +const recording = () => { + const lines: unknown[][] = []; + const logger: Logger = { + debug: () => undefined, + info: () => undefined, + warn: () => undefined, + error: (...entry) => void lines.push(entry), + }; + return { lines, logger }; +}; + +const request = () => new Request("http://localhost/accounts/ana@example.com?step=2"); + +const quoting = () => new Error("no account for ana@example.com"); + +let keys = 0; +const uniqueKey = (): string => `bounda.handle-error.${process.pid}.${++keys}`; + +const appWith = (errorText: ErrorText): BoundaApp => + ({ + config: { runtime: { telemetry: { errorText } } }, + start: () => undefined, + stop: async () => undefined, + catchUpReadModels: async () => undefined, + commands: {}, + }) as unknown as BoundaApp; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe("createHandleError", () => { + it("logs an error's class and code alone with omit, and its text and the path with include", async () => { + const omitted = recording(); + await createHandleError({ + message: "request failed", + errorText: () => "omit", + logger: () => omitted.logger, + })(quoting(), { request: request() }); + expect(omitted.lines).toEqual([["request failed", { method: "GET", errorName: "Error" }]]); + + const included = recording(); + await createHandleError({ + message: "request failed", + errorText: () => "include", + logger: async () => included.logger, + })(quoting(), { request: request() }); + expect(included.lines).toEqual([ + [ + "request failed", + { + method: "GET", + path: "/accounts/ana@example.com", + errorName: "Error", + errorMessage: "no account for ana@example.com", + errorStack: expect.stringContaining("no account for ana@example.com"), + }, + ], + ]); + }); + + it("logs the error React Router wraps in what it throws itself, and the wrapper without one", async () => { + const { lines, logger } = recording(); + const handleError = createHandleError({ + message: "request failed", + errorText: () => "omit", + logger: () => logger, + }); + await handleError( + new UNSAFE_ErrorResponseImpl( + 404, + "Not Found", + new TypeError('No route matches "/ana"'), + true, + ), + { request: request() }, + ); + await handleError(new UNSAFE_ErrorResponseImpl(404, "Not Found", "ana"), { + request: request(), + }); + expect(lines).toEqual([ + ["request failed", { method: "GET", errorName: "TypeError" }], + ["request failed", { method: "GET", errorName: "object" }], + ]); + }); + + it("logs nothing for a request the client aborted", async () => { + const { lines, logger } = recording(); + const controller = new AbortController(); + controller.abort(); + await createHandleError({ + message: "request failed", + errorText: () => "include", + logger: () => logger, + })(quoting(), { request: new Request("http://localhost/", { signal: controller.signal }) }); + expect(lines).toEqual([]); + }); + + it("never rejects, whatever fails on the way", async () => { + const { lines, logger } = recording(); + await expect( + createHandleError({ + message: "request failed", + errorText: () => { + throw new Error("no configuration"); + }, + logger: () => logger, + })(quoting(), { request: request() }), + ).resolves.toBeUndefined(); + expect(lines).toEqual([]); + await expect( + createHandleError({ + message: "request failed", + errorText: () => "include", + logger: () => ({ + ...logger, + error: () => { + throw new Error("full disk"); + }, + }), + })(quoting(), { request: request() }), + ).resolves.toBeUndefined(); + await expect( + createHandleError({ + message: "request failed", + errorText: () => "include", + logger: () => Promise.reject(new Error("no logger")), + })(quoting(), { request: request() }), + ).resolves.toBeUndefined(); + }); + + it("is a handleError React Router accepts", () => { + expectTypeOf().toExtend(); + }); +}); + +// A server build whose root route mounts the middleware and whose loader rethrows what it caught, +// served by React Router's own request handler, as `react-router-serve` and a Worker do. +const serverBuild = ( + bounda: ReturnType, + withHandleError: boolean, +): ServerBuild => + ({ + entry: { + module: { + default: (_request: Request, status: number) => new Response("document", { status }), + ...(withHandleError ? { handleError: bounda.handleError } : {}), + }, + }, + routes: { + root: { + id: "root", + path: "", + module: { default: () => null, middleware: [bounda.boundaMiddleware] }, + }, + account: { + id: "account", + parentId: "root", + path: "accounts/:email", + module: { + default: () => null, + loader: () => { + try { + throw quoting(); + } catch (error) { + throw new Error(`could not load: ${(error as Error).message}`, { cause: error }); + } + }, + }, + }, + }, + assets: { entry: { imports: [], module: "" }, routes: {}, url: "", version: "1" }, + publicPath: "/", + assetsBuildDirectory: "", + future: {}, + ssr: true, + isSpaMode: false, + prerender: [], + routeDiscovery: { mode: "lazy", manifestPath: "/__manifest" }, + }) as unknown as ServerBuild; + +// What the console would print: an error with its message, stack and cause, not `{}`. +const shown = (value: unknown): string => (typeof value === "string" ? value : inspect(value)); + +const served = async (errorText: ErrorText, withHandleError = true) => { + const console_: unknown[][] = []; + for (const level of ["log", "info", "warn", "error", "debug"] as const) { + vi.spyOn(console, level).mockImplementation((...args) => void console_.push(args.map(shown))); + } + const { lines, logger } = recording(); + const bounda = createBounda({ boot: async () => appWith(errorText), key: uniqueKey(), logger }); + const handler = createRequestHandler(serverBuild(bounda, withHandleError), "production"); + const response = await handler(request()); + if (withHandleError) await vi.waitFor(() => expect(lines).toHaveLength(1)); + await bounda.dispose(); + return { response, logged: JSON.stringify([lines, console_]), lines }; +}; + +describe("handleError behind React Router's request handler", () => { + it("is needed: React Router's default logs the message of what a loader rethrows", async () => { + const { logged } = await served("omit", false); + expect(logged).toContain("could not load: no account for ana@example.com"); + }); + + it("keeps the message and the stack of what a loader rethrows out of every log with omit", async () => { + const { response, logged, lines } = await served("omit"); + expect(response.status).toBe(500); + expect(logged).not.toContain("ana@example.com"); + expect(logged).not.toContain("could not load"); + expect(lines).toEqual([["request failed", { method: "GET", errorName: "Error" }]]); + }); + + it("logs them with include", async () => { + const { response, logged, lines } = await served("include"); + expect(response.status).toBe(500); + expect(logged).toContain("could not load: no account for ana@example.com"); + expect(lines).toEqual([ + [ + "request failed", + { + method: "GET", + path: "/accounts/ana@example.com", + errorName: "Error", + errorMessage: "could not load: no account for ana@example.com", + errorStack: expect.stringContaining("at "), + }, + ], + ]); + }); +}); diff --git a/packages/react-router/src/handle-error.ts b/packages/react-router/src/handle-error.ts new file mode 100644 index 00000000..8846641d --- /dev/null +++ b/packages/react-router/src/handle-error.ts @@ -0,0 +1,51 @@ +import { errorFields, type Logger } from "@bounda-dev/core"; +import type { ErrorText } from "@bounda-dev/core/config"; +import { isRouteErrorResponse } from "react-router"; + +/** + * React Router's `handleError`, to export from `app/entry.server.tsx`, and to call from its + * streaming render's `onError`: logs what a loader, an action or the render throws as + * `runtime.telemetry.errorText` says, its class and its `code`, plus its message, its stack and + * the request's path only with `"include"`, where React Router's default logs the message and the + * stack always, and an error's message may quote personal data. It skips a request the client + * aborted, as the default does. Resolves once the line is logged, and never rejects. + */ +export interface HandleBoundaErrorFunction { + (error: unknown, args: { readonly request: Request }): Promise; +} + +export interface CreateHandleErrorArgs { + readonly message: string; + /** + * Synchronous: a Worker may cancel what is still pending once its response is sent, and React + * Router never waits for `handleError`. + */ + readonly errorText: () => ErrorText; + readonly logger: () => Logger | Promise; +} + +export interface CreateHandleErrorFunction { + (args: CreateHandleErrorArgs): HandleBoundaErrorFunction; +} + +// React Router wraps what it throws itself, such as no route matching, with the error inside. +const unwrapped = (error: unknown): unknown => + isRouteErrorResponse(error) && Reflect.get(error, "error") instanceof Error + ? Reflect.get(error, "error") + : error; + +export const createHandleError: CreateHandleErrorFunction = + ({ message, errorText, logger }) => + async (error, { request }) => { + if (request.signal.aborted) return; + try { + const text = errorText(); + (await logger()).error(message, { + method: request.method, + ...(text === "include" ? { path: new URL(request.url).pathname } : {}), + ...errorFields({ error: unwrapped(error), errorText: text }), + }); + } catch { + // A failure here has nowhere left to be reported, and React Router must not see it throw. + } + }; diff --git a/packages/react-router/src/host/args.ts b/packages/react-router/src/host/args.ts index 105fcef5..6cf05fcf 100644 --- a/packages/react-router/src/host/args.ts +++ b/packages/react-router/src/host/args.ts @@ -26,9 +26,11 @@ export interface CreateHostArgs { readonly importActor?: ImportModuleFunction | undefined; readonly consistency: Consistency; /** - * What `runtime.telemetry.errorText` is when the configuration sets none: the dev server passes - * `"include"`. Only the Node host applies it: on Cloudflare the app runs in its Durable Object, - * under the configuration `createBoundaObject` takes. + * What `runtime.telemetry.errorText` is when the configuration sets none, and what + * `handleError` follows before the app has booted: the dev server passes `"include"`. Only the + * Node host applies it: on Cloudflare the app runs in its Durable Object, under the + * configuration `createBoundaObject` takes and `BOUNDA_ERROR_TEXT` in its `env`, which + * `.dev.vars` sets in development. */ readonly errorText?: ErrorText; } diff --git a/packages/react-router/src/host/cloudflare.test.ts b/packages/react-router/src/host/cloudflare.test.ts index e6a302b4..0b207710 100644 --- a/packages/react-router/src/host/cloudflare.test.ts +++ b/packages/react-router/src/host/cloudflare.test.ts @@ -2,7 +2,7 @@ import { cloudflare } from "@bounda-dev/cloudflare"; import { ConfigurationError, type Consistency, type Registry } from "@bounda-dev/core"; import type { ImportModuleFunction } from "@bounda-dev/core/node"; import { RouterContextProvider } from "react-router"; -import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { env, type FakeStoreNamespace, storeNamespace } from "../cloudflare/test-support.ts"; import { failure } from "../failure.ts"; import * as cloudflareHost from "./cloudflare.ts"; @@ -150,3 +150,90 @@ describe("createHost under the workerd condition", () => { expect(config.imported.count).toBe(0); }); }); + +describe("createHost under the workerd condition: handleError", () => { + afterEach(() => { + delete env.BOUNDA_ERROR_TEXT; + }); + + const logged = async (work: () => Promise): Promise => { + const written: unknown[][] = []; + const spy = vi.spyOn(console, "error").mockImplementation((...args) => void written.push(args)); + try { + await work(); + } finally { + spy.mockRestore(); + } + return written.map(([line]) => JSON.parse(String(line))); + }; + + const omitted = { + level: "error", + message: "bounda request failed", + method: "GET", + errorName: "Error", + }; + const included = { + ...omitted, + path: "/a", + errorMessage: "ana@example.com", + errorStack: expect.any(String), + }; + + it.each([ + [undefined, undefined, omitted], + [undefined, "unset", omitted], + [undefined, "include", included], + ["include", "omit", included], + ["omit", "include", omitted], + ["loud", "include", included], + ] as const)( + "logs the text as BOUNDA_ERROR_TEXT %s says, over the configuration %s a request imported, as one JSON line", + async (variable, configured, line) => { + if (variable !== undefined) env.BOUNDA_ERROR_TEXT = variable; + const host = createHost({ + registry, + importConfig: async () => ({ + default: { + storage: cloudflare(), + ...(configured === undefined + ? {} + : { + runtime: + configured === "unset" + ? { role: "web" as const } + : { telemetry: { errorText: configured } }, + }), + }, + }), + importTenant: async () => ({ tenant: () => "acme" }), + consistency: "eventual", + }); + await host.boundaMiddleware(middlewareArgs().args, async () => undefined); + expect( + await logged(() => + host.handleError(new Error("ana@example.com"), { request: new Request("http://w/a") }), + ), + ).toEqual([line]); + }, + ); + + // A Worker may cancel an import still pending once its response is sent. + it("never imports the configuration itself: before the first request only the env decides", async () => { + const config = counted({ + default: { storage: cloudflare(), runtime: { telemetry: { errorText: "include" } } }, + }); + const { handleError } = createHost({ + registry, + importConfig: config.load, + importTenant: async () => ({ tenant: () => "acme" }), + consistency: "eventual", + }); + const error = new Error("ana@example.com"); + const request = () => new Request("http://w/a"); + expect(await logged(() => handleError(error, { request: request() }))).toEqual([omitted]); + env.BOUNDA_ERROR_TEXT = "include"; + expect(await logged(() => handleError(error, { request: request() }))).toEqual([included]); + expect(config.imported.count).toBe(0); + }); +}); diff --git a/packages/react-router/src/host/cloudflare.ts b/packages/react-router/src/host/cloudflare.ts index e0f7a15c..e3d9e75f 100644 --- a/packages/react-router/src/host/cloudflare.ts +++ b/packages/react-router/src/host/cloudflare.ts @@ -8,6 +8,7 @@ import { import type { Config } from "@bounda-dev/core/config"; import { actorOfModule } from "../actor.ts"; import { cloudflareClients, type TenantFunction } from "../cloudflare/clients.ts"; +import { cloudflareHandleError } from "../cloudflare/handle-error.ts"; import { serve } from "../cloudflare/serve.ts"; import type { Bounda } from "../create-bounda.ts"; import type { CreateHostArgs, CreateHostFunction } from "./args.ts"; @@ -19,7 +20,9 @@ export type { CreateHostArgs, CreateHostFunction } from "./args.ts"; * The host the `bounda()` Vite plugin serves the app from under the `workerd` condition: the * `createBounda` of `@bounda-dev/react-router/cloudflare`, with the project's configuration and * the `tenant` of its `app/tenant.ts`, both imported on the first request, acting as the `actor` - * of its `app/actor.ts`, when it has one. Not meant to be called + * of its `app/actor.ts`, when it has one; its `handleError` follows `BOUNDA_ERROR_TEXT` in the + * Worker's `env`, or else the configuration's `runtime.telemetry.errorText` once a request has + * imported it. Not meant to be called * by hand. Throws `ConfigurationError` for a `consistency` it does not know; the first request * rejects with one without `app/tenant.ts`, or as `createBounda` does. */ @@ -30,6 +33,9 @@ export const createHost: CreateHostFunction = consistency, }: CreateHostArgs): Bounda => { checkConsistency(consistency); + // What `handleError` reads: it cannot wait for an import, which the Worker may cancel once the + // response is sent, so until the first request has imported it only the `env` decides. + let imported: Config | undefined; return serve( async () => { if (importTenant === undefined) { @@ -48,6 +54,7 @@ export const createHost: CreateHostFunction = "app/tenant.ts does not export tenant, a function of the request that names its store", ); } + imported = config as Config; return cloudflareClients({ env, config: config as Config, @@ -55,6 +62,7 @@ export const createHost: CreateHostFunction = consistency, }); }, + cloudflareHandleError({ config: () => imported }), importActor === undefined ? undefined : actorOfModule(importActor), ); }; diff --git a/packages/react-router/src/host/node.test.ts b/packages/react-router/src/host/node.test.ts index ef221c53..fae4f0ce 100644 --- a/packages/react-router/src/host/node.test.ts +++ b/packages/react-router/src/host/node.test.ts @@ -11,14 +11,22 @@ const app = { start: () => undefined, stop: async () => undefined, as: (actor: string) => ({ actedAs: actor }), + config: { runtime: { telemetry: { errorText: "omit" } } }, } as unknown as BoundaApp; const booted: BootArgs[] = []; +const logged: unknown[][] = []; vi.mock("@bounda-dev/core/node", () => ({ boot: vi.fn(async (args: BootArgs) => { booted.push(args); return app; }), + createConsoleLogger: () => ({ + debug: () => undefined, + info: () => undefined, + warn: () => undefined, + error: (...entry: unknown[]) => void logged.push(entry), + }), })); const registry = { aggregates: {}, readModels: {} } as Registry; @@ -38,9 +46,39 @@ let host: ReturnType | undefined; afterEach(async () => { await host?.dispose(); booted.length = 0; + logged.length = 0; }); describe("createHost in Node", () => { + it.each([ + ["include", { errorMessage: "ana@example.com", errorStack: expect.any(String) }], + [undefined, {}], + ] as const)( + "logs what a request throws before the app boots as errorText %s says, on the console", + async (errorText, text) => { + host = createHost({ + registry, + importConfig: async () => ({ default: {} }), + consistency: "eventual", + ...(errorText === undefined ? {} : { errorText }), + }); + await host.handleError(new Error("ana@example.com"), { + request: new Request("http://localhost/a"), + }); + expect(logged).toEqual([ + [ + "request failed", + { + method: "GET", + ...(errorText === undefined ? {} : { path: "/a" }), + errorName: "Error", + ...text, + }, + ], + ]); + }, + ); + it("boots the project from the root, registry and configuration it is given, with its consistency", async () => { const config = { storage: { kind: "bounda-adapter", name: "sqlite", options: {} } }; host = createHost({ diff --git a/packages/react-router/src/host/node.ts b/packages/react-router/src/host/node.ts index f1b45fba..e0f00fe0 100644 --- a/packages/react-router/src/host/node.ts +++ b/packages/react-router/src/host/node.ts @@ -1,4 +1,9 @@ -import { type AppRegistry, ConfigurationError, type Registry } from "@bounda-dev/core"; +import { + type AppRegistry, + ConfigurationError, + type Registry, + withErrorText, +} from "@bounda-dev/core"; import type { Config, ErrorText } from "@bounda-dev/core/config"; import { boot, type ImportModuleFunction } from "@bounda-dev/core/node"; import { actorOfModule } from "../actor.ts"; @@ -14,21 +19,13 @@ const needsWorkers = (error: unknown): boolean => Reflect.get(error, "code") === "ERR_UNSUPPORTED_ESM_URL_SCHEME" && error.message.includes("'cloudflare:'"); -// The configuration's own `errorText` wins over the host's default. -const withErrorText = (module: Readonly>, errorText: ErrorText) => { +// The host's default applies under the configuration's own `errorText`; `boot()` then applies +// `BOUNDA_ERROR_TEXT` over both. +const withFallback = (module: Readonly>, fallback: ErrorText) => { const config = module.default as Config | undefined; return config === undefined ? module - : { - ...module, - default: { - ...config, - runtime: { - ...config.runtime, - telemetry: { errorText, ...config.runtime?.telemetry }, - }, - } satisfies Config, - }; + : { ...module, default: withErrorText({ config, fallback }) }; }; const explainWorkers = @@ -36,7 +33,7 @@ const explainWorkers = async () => { try { const module = await importConfig(); - return errorText === undefined ? module : withErrorText(module, errorText); + return errorText === undefined ? module : withFallback(module, errorText); } catch (error) { if (!needsWorkers(error)) throw error; throw new ConfigurationError( @@ -71,4 +68,5 @@ export const createHost: CreateHostFunction = }), consistency, ...(importActor === undefined ? {} : { actor: actorOfModule(importActor) }), + errorText, }); diff --git a/packages/react-router/src/index.ts b/packages/react-router/src/index.ts index fe214e5a..4922ec64 100644 --- a/packages/react-router/src/index.ts +++ b/packages/react-router/src/index.ts @@ -10,3 +10,4 @@ export type { export { createBounda } from "./create-bounda.ts"; export type { Failure, FailureFunction } from "./failure.ts"; export { failure } from "./failure.ts"; +export type { HandleBoundaErrorFunction } from "./handle-error.ts"; diff --git a/packages/react-router/src/vite-plugin.ts b/packages/react-router/src/vite-plugin.ts index 1d761b8e..0f1e9353 100644 --- a/packages/react-router/src/vite-plugin.ts +++ b/packages/react-router/src/vite-plugin.ts @@ -1,3 +1,4 @@ +import { readdir, readFile } from "node:fs/promises"; import { join } from "node:path"; import type { Clock, Consistency } from "@bounda-dev/core"; import { type Logger, normalizePath, type Plugin } from "vite"; @@ -27,6 +28,10 @@ const CONFIG_FILE = "bounda.config.ts"; const TENANT_FILE = "tenant.ts"; const ACTOR_FILE = "actor.ts"; const WATCHED = ["domain", "read"]; +const ENTRY_SERVER = /^entry\.server\.[cm]?[jt]sx?$/; +const EXPORTS_HANDLE_ERROR = + /export\s+(?:async\s+)?(?:function\*?|const|let|var)\s+handleError\b|export\s*\{[^}]*\bhandleError\b[^}]*\}/; +const LOGGING_ERRORS = "https://docs.bounda.dev/guides/react-router/#logging-errors"; const EVENTS = ["add", "change", "unlink", "addDir", "unlinkDir"] as const; /** @@ -43,7 +48,7 @@ const serverModule = ({ root, command }: Generation, consistency: Consistency): `import { createHost } from ${JSON.stringify(HOST_ID)};`, `import { registry } from ${JSON.stringify(normalizePath(join(root, ".bounda/registry.ts")))};`, "", - "export const { bounda, boundaMiddleware, dispose } = createHost({", + "export const { bounda, boundaMiddleware, dispose, handleError } = createHost({", ...(command === "serve" ? [` root: ${JSON.stringify(root)},`, ' errorText: "include",'] : []), " registry,", ` importConfig: () => import(${JSON.stringify(normalizePath(join(root, CONFIG_FILE)))}),`, @@ -63,6 +68,7 @@ const clientModule = (): string => "export const bounda = new Proxy({}, { get: serverOnly });", "export const boundaMiddleware = serverOnly;", "export const dispose = serverOnly;", + "export const handleError = serverOnly;", "export const failure = serverOnly;", "", ].join("\n"); @@ -92,6 +98,22 @@ const regenerate = async ({ root, logger, failOnConvention }: Regeneration): Pro } }; +// Without its own `handleError`, React Router logs every message and stack, in production too. +const checkEntryServer = async ({ root, logger }: Pick) => { + const directory = join(root, APP_DIRECTORY); + const files = await readdir(directory).catch(() => undefined); + const name = files?.find((file) => ENTRY_SERVER.test(file)); + if ( + name !== undefined && + EXPORTS_HANDLE_ERROR.test(String(await readFile(join(directory, name)))) + ) { + return; + } + logger.warn( + `[bounda] ${name === undefined ? `${APP_DIRECTORY}/entry.server.tsx is missing` : `${APP_DIRECTORY}/${name} does not export handleError`}, so React Router logs the message and the stack of whatever a loader, an action or the render throws, and a message may quote personal data. Export Bounda's from it: export { handleError } from "${APP_MODULE_ID}"; see ${LOGGING_ERRORS}`, + ); +}; + export const createBoundaPlugin: CreateBoundaPluginFunction = ({ consistency = "read-your-writes", debounceMs = 100, @@ -99,6 +121,7 @@ export const createBoundaPlugin: CreateBoundaPluginFunction = ({ }) => { let generation: Generation | undefined; let generated: Promise | undefined; + let entryChecked = false; let queue: Promise = Promise.resolve(); let cancelPending: (() => void) | undefined; // Both sides in Vite's form, so the watcher's separator, `\` on Windows, does not matter. @@ -137,6 +160,10 @@ export const createBoundaPlugin: CreateBoundaPluginFunction = ({ // A watching build starts again after every change, and each start has to see it. generated = this.meta.watchMode ? run() : (generated ?? run()); await generated; + if (!entryChecked) { + entryChecked = true; + await checkEntryServer(current); + } }, configureServer(server) { const schedule = (file: string): void => { diff --git a/packages/react-router/src/vite.test.ts b/packages/react-router/src/vite.test.ts index a2b32150..645bd5d0 100644 --- a/packages/react-router/src/vite.test.ts +++ b/packages/react-router/src/vite.test.ts @@ -23,6 +23,10 @@ const project = async (fixture = "order-app-inferred"): Promise => { recursive: true, filter: (source) => !source.includes("/+types"), }); + await writeFile( + join(root, "app/entry.server.tsx"), + 'export { handleError } from "@bounda-dev/react-router/app";\n', + ); await writeFile( join(root, "tsconfig.json"), JSON.stringify({ @@ -200,7 +204,7 @@ describe("bounda() Vite plugin", () => { 'import { createHost } from "@bounda-dev/react-router/host";', 'import { registry } from "/project/.bounda/registry.ts";', "", - "export const { bounda, boundaMiddleware, dispose } = createHost({", + "export const { bounda, boundaMiddleware, dispose, handleError } = createHost({", ' root: "/project",', ' errorText: "include",', " registry,", @@ -245,6 +249,7 @@ describe("bounda() Vite plugin", () => { "export const bounda = new Proxy({}, { get: serverOnly });", "export const boundaMiddleware = serverOnly;", "export const dispose = serverOnly;", + "export const handleError = serverOnly;", "export const failure = serverOnly;", "", ].join("\n"), @@ -253,10 +258,12 @@ describe("bounda() Vite plugin", () => { bounda: { defaultValue?: unknown }; boundaMiddleware: () => unknown; failure: () => unknown; + handleError: () => unknown; }; expect(() => module.bounda.defaultValue).toThrow("server-only"); expect(() => module.boundaMiddleware()).toThrow("server-only"); expect(() => module.failure()).toThrow("server-only"); + expect(() => module.handleError()).toThrow("server-only"); }); it("serves nothing before the config is resolved", () => { @@ -308,6 +315,48 @@ describe("bounda() Vite plugin", () => { expect(recorded.errors).toEqual([]); }); + it.each([ + ['export { handleError } from "@bounda-dev/react-router/app";', "entry.server.tsx", false], + ['export { default, handleError } from "./server.ts";', "entry.server.ts", false], + ["export function handleError() {}", "entry.server.jsx", false], + ["export async function handleError() {}", "entry.server.mts", false], + ["export const handleError = () => undefined;", "entry.server.js", false], + ["export default function handleRequest() {}", "entry.server.tsx", true], + ["const handleError = () => undefined;", "entry.server.tsx", true], + [undefined, undefined, true], + ] as const)( + "warns once, on serve and build, unless the entry server exports handleError: %s in %s", + async (source, file, warned) => { + const root = await project("order-app"); + await rm(join(root, "app/entry.server.tsx")); + if (source !== undefined && file !== undefined) { + await writeFile(join(root, "app", file), `${source}\n`); + } + for (const command of ["serve", "build"] as const) { + const { configure, start, recorded } = harness(root); + configure(command); + await start(true); + await start(true); + expect(recorded.warnings).toEqual( + warned + ? [ + `[bounda] ${file === undefined ? "app/entry.server.tsx is missing" : `app/${file} does not export handleError`}, so React Router logs the message and the stack of whatever a loader, an action or the render throws, and a message may quote personal data. Export Bounda's from it: export { handleError } from "@bounda-dev/react-router/app"; see https://docs.bounda.dev/guides/react-router/#logging-errors`, + ] + : [], + ); + } + }, + ); + + it("warns that the entry server is missing in a project without an app directory", async () => { + const root = await mkdtemp(join(tmpdir(), "bounda-vite-")); + temporary.push(root); + const { configure, start, recorded } = harness(root); + configure("serve"); + await start().catch(() => undefined); + expect(recorded.warnings[0]).toMatch(/^\[bounda\] app\/entry\.server\.tsx is missing, /); + }); + it("does nothing at build start before the config is resolved", async () => { const root = await project(); const { start } = harness(root);