diff --git a/CLAUDE.md b/CLAUDE.md index 169d121..a268c99 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -106,7 +106,7 @@ See `docs/guides/coverage.md` for the cookbook and ADR-020 for the full rational - **Tests inject mocks directly** — Construct `MockXRepository` and pass into the factory: `signInUseCase(mockUsers, mockAuth)(input)`. No container rebinding in unit tests - **Schemas in the use-case file** — Every use case exports `xInputSchema` (a `z.ZodObject` with `.strict()`; `z.object({}).strict()` for void inputs) and, for non-void use cases, `xOutputSchema`. Types: `XInput = z.infer` and `XOutput`. Use case body ends with `xOutputSchema.parse(result)` before returning (runtime guarantee against malformed repository data) - **Controllers receive `unknown` + presenter** — Controllers `safeParse(xInputSchema)` from the use-case file and throw `InputParseError` on failure. Non-void controllers define a top-level `function presenter(value: XOutput)` and return `Promise>` (identity is fine — `return value`); void controllers return `Promise` with no presenter -- **Feature-scoped tRPC error mapping** — Each feature has `integrations/api/procedures.ts` exporting `xProcedure = t.procedure.use(defineErrorMiddleware([[Ctor, "TRPC_CODE"], ...]))` from `@repo/core-shared/trpc/define-error-middleware`. Routers use `xProcedure.input(xInputSchema)` — schemas are imported from the use-case file, never redefined inline. `core-shared` never enumerates feature error classes +- **Feature-scoped tRPC error mapping** — Each feature has `integrations/api/procedures.ts` exporting `xProcedure = t.procedure.use(defineErrorMiddleware([[Ctor, "TRPC_CODE"], ...]))` from `@repo/core-shared/trpc/define-error-middleware`. Routers use `xProcedure.input(xInputSchema)` — schemas are imported from the use-case file, never redefined inline. `core-shared` never enumerates feature error classes. Matching is by **canonical error name**, not class identity (ADR-027): every domain error class MUST set `this.name = ""` from a string literal in its constructor (side-effect-free, callable with one message arg) — `instanceof` fails across module graphs (tsx DI vs webpack bundle) and would mask mapped errors as 500s - **Public surface split** — Feature root (`.`) exports contracts only: types, errors, schemas, IUseCase / IController aliases, router type, constants. UI artifacts (hooks, components, query builders) live behind `./ui` (`src/ui/index.ts`). Apps import hooks/components from `@repo//ui`, schemas/types from `@repo/` - **Feature UI owns its data fetching** — Each feature's `src/ui/hooks/` contains `"use client"` hooks that wrap `useTRPC` + `useSuspenseQuery`. Connected components in `src/ui/components/` call these hooks. App pages prefetch via `appRouter.createCaller({})` and hydrate via `HydrationBoundary` + `dehydrate` + `setQueryData`. See `docs/guides/building-feature-ui.md` - **Payload repositories via constructor** — Feature packages receive Payload config at constructor time, not as a direct dependency diff --git a/docs/decisions/adr-027-domain-error-matching-by-name.md b/docs/decisions/adr-027-domain-error-matching-by-name.md new file mode 100644 index 0000000..f23014d --- /dev/null +++ b/docs/decisions/adr-027-domain-error-matching-by-name.md @@ -0,0 +1,87 @@ +# ADR-027 — Domain errors match by canonical name, not class identity + +**Status:** Accepted · 2026-07-31 + +## Context + +`defineErrorMiddleware` (core-shared) translates feature domain errors to +tRPC codes from feature-owned `[Ctor, "CODE"]` maps. It originally matched +`result.error.cause instanceof Ctor` — class **identity**. + +A downstream app built from this template hit the failure in production: +a use case threw `UnauthenticatedError`, whose `[UnauthenticatedError, +"UNAUTHORIZED"]` mapping exists, yet the client received a raw 500. Root +cause: web-next starts through the template's custom server (`node --import +tsx server.ts` — the Socket.IO-ready entry). The DI container binds feature +modules loaded by **tsx**, while the tRPC route handler executes inside the +**Next/webpack-compiled** bundle. Two module graphs, two copies of every +error class. The thrown instance and the registered constructor are +different objects, `instanceof` is false, and every mapped error falls +through to tRPC's default `INTERNAL_SERVER_ERROR`. + +The failure is doubly deceptive: the HTTP status lies (500 for an expected +domain condition), and a routine domain signal presents as a server fault — +in the downstream incident an expected `UNAUTHORIZED` was debugged as an +infrastructure crash. + +Constraints on any fix: + +- **Feature-owned maps stay.** core-shared must not enumerate feature error + classes (existing rule), and the `[Ctor, "CODE"]` call-site API is used + across features/cores — churn there multiplies the diff. +- **Server minification.** Next minifies server bundles; class _identifiers_ + (`Ctor.name`) may be mangled, so a fix keyed on the static class name is + not reliable. String _literals_ survive minification. +- **Existing convention.** Every domain error class in the template already + sets `this.name = ""` from a string literal in its constructor. + +## Decision + +1. **Match by canonical error name, with identity as a fast path.** At + middleware-definition time, probe one instance per registered constructor + (`new Ctor("__probe__")`) and record its `.name` — the value assigned + from the string literal, which survives both module-graph duplication and + minification. At request time an error matches an entry when `cause +instanceof Ctor` **or** `cause.name === canonicalName`. +2. **Nameless classes match by identity only.** A class that never sets a + custom name probes as the inherited `"Error"`; name-matching is disabled + for that entry, otherwise every unmapped plain `Error` would translate. +3. **`this.name` from a string literal becomes a load-bearing convention.** + Every domain error class MUST assign `this.name = ""` in its + constructor (string literal, matching the class name), and error + constructors MUST be side-effect-free and callable with a single message + argument (the probe relies on it). This was already universal practice; + it is now the matching key. +4. **Call sites unchanged.** All existing `defineErrorMiddleware([[Ctor, +"CODE"], ...])` usages keep working verbatim. + +## Alternatives considered + +- **`Ctor.name` comparison (no probe)** — rejected: minified server bundles + can mangle class identifiers while leaving the constructor-body string + literal intact; the probe reads the literal. +- **Shared `DomainError` base class with a `code` discriminator** — + rejected (for now): touches every feature's error classes and public + contracts to achieve the same runtime behavior name-matching gets with a + one-file change. Revisit if error metadata beyond a code is ever needed. +- **Unify the module graphs** (make the webpack bundle resolve the same + module instances the tsx DI graph holds) — rejected: depends on Next + bundling internals and externals configuration, fragile across upgrades, + and would silently regress the day either toolchain changes resolution. +- **Serialize errors at the feature boundary** (throw plain objects with a + code) — rejected: reshapes the whole error architecture (capture, + `__sentryReported` flag, `.cause` chains) for a problem the name check + solves locally. + +## Consequences + +- Domain errors now surface with their mapped codes regardless of which + module graph constructed them; 500 again means "actual server fault". +- Within one feature's map, two classes sharing a name would both match the + first entry. Maps are feature-scoped and small, and names conventionally + equal the class name, so collisions indicate a naming bug — acceptable. +- The regression test encodes the failure mode (structurally identical + class, same name, different identity) so the cross-graph scenario stays + covered even where local test runs use a single module graph. +- Any generator or new feature emitting error classes must keep the + `this.name` string-literal assignment — it is no longer cosmetic. diff --git a/packages/core-shared/src/trpc/define-error-middleware.test.ts b/packages/core-shared/src/trpc/define-error-middleware.test.ts index d3a370e..bb8ce9f 100644 --- a/packages/core-shared/src/trpc/define-error-middleware.test.ts +++ b/packages/core-shared/src/trpc/define-error-middleware.test.ts @@ -17,6 +17,21 @@ class FooBadRequestError extends Error { } } +// Structurally identical to FooNotFoundError but a distinct class identity — +// simulates the production failure where the DI-bound feature modules and the +// route-handler bundle each load their own copy of an error module, so the +// thrown instance fails `instanceof` against the registered constructor. +class CrossRealmFooNotFoundError extends Error { + constructor(message: string) { + super(message); + this.name = "FooNotFoundError"; + } +} + +// Never sets `this.name` — probes as the inherited "Error". Registering it +// must not cause every plain Error to match by name. +class AnonymousDomainError extends Error {} + const errorRouter = t.router({ notFound: t.procedure .use( @@ -43,6 +58,21 @@ const errorRouter = t.router({ .query(() => { throw new Error("plain"); }), + crossRealm: t.procedure + .use(defineErrorMiddleware([[FooNotFoundError, "NOT_FOUND"]])) + .query(() => { + throw new CrossRealmFooNotFoundError("other copy"); + }), + namelessMapped: t.procedure + .use(defineErrorMiddleware([[AnonymousDomainError, "CONFLICT"]])) + .query(() => { + throw new Error("plain"); + }), + namelessInstance: t.procedure + .use(defineErrorMiddleware([[AnonymousDomainError, "CONFLICT"]])) + .query(() => { + throw new AnonymousDomainError("actual instance"); + }), }); describe("defineErrorMiddleware", () => { @@ -75,6 +105,31 @@ describe("defineErrorMiddleware", () => { } }); + it("matches by error name when the instance comes from a different module graph", async () => { + // Production regression: tsx-bound DI modules and the webpack-bundled + // route handler hold separate copies of the error classes, so identity + // (`instanceof`) fails and mapped errors surfaced as 500s. + const caller = errorRouter.createCaller({}); + await expect(caller.crossRealm()).rejects.toMatchObject({ + code: "NOT_FOUND", + message: "other copy", + }); + }); + + it("does not name-match plain Errors against a class that never set a custom name", async () => { + const caller = errorRouter.createCaller({}); + await expect(caller.namelessMapped()).rejects.toMatchObject({ + code: "INTERNAL_SERVER_ERROR", + }); + }); + + it("still matches a nameless class by identity", async () => { + const caller = errorRouter.createCaller({}); + await expect(caller.namelessInstance()).rejects.toMatchObject({ + code: "CONFLICT", + }); + }); + it("preserves the original error as the cause", async () => { const caller = errorRouter.createCaller({}); try { diff --git a/packages/core-shared/src/trpc/define-error-middleware.ts b/packages/core-shared/src/trpc/define-error-middleware.ts index 681b596..fdfae9c 100644 --- a/packages/core-shared/src/trpc/define-error-middleware.ts +++ b/packages/core-shared/src/trpc/define-error-middleware.ts @@ -20,17 +20,37 @@ type ErrorCtor = new (...args: never[]) => Error; * * Owned by features: each feature passes its own constructors in. * core-shared never enumerates feature-specific error classes. + * + * Matching is by `instanceof` OR by canonical error name: the DI-bound + * feature modules and the app's route-handler bundle can each load their + * own copy of an error class (separate module graphs), so identity checks + * fail across them and mapped errors would surface as 500s. Every domain + * error sets `this.name` from a string literal in its constructor — that + * string survives duplication and minification, so the middleware probes + * one instance per constructor to learn its canonical name and matches + * `cause.name` against it. A class that never sets a custom name probes + * as the inherited "Error"; name-matching is disabled for it (identity + * only), otherwise every plain Error would match. */ export function defineErrorMiddleware( map: ReadonlyArray, ) { + const entries = map.map(([Ctor, code]) => { + const probe = new (Ctor as new (...args: unknown[]) => Error)("__probe__"); + return { + Ctor, + name: probe.name === "Error" ? undefined : probe.name, + code, + }; + }); return t.middleware(async ({ next }) => { const result = await next(); if (!result.ok) { const cause = result.error.cause; if (cause instanceof Error) { - for (const [Ctor, code] of map) { - if (cause instanceof Ctor) { + for (const { Ctor, name, code } of entries) { + const matchesName = name !== undefined && cause.name === name; + if (matchesName || cause instanceof Ctor) { throw new TRPCError({ code, message: cause.message, cause }); } }