Danijel Martinek
ed59b4348f
Some checks failed
CodeQL / Analyze (javascript-typescript) (push) Has been cancelled
CI / typecheck + lint + boundaries + test + build (push) Has been cancelled
Coverage snapshot / snapshot (push) Has been cancelled
Release Please / release-please (push) Has been cancelled
Sentry PII guard (R31) / pii-guard (push) Has been cancelled
CI / Storybook smoke tests + visual regression (push) Has been cancelled
CI / Playwright e2e (push) Has been cancelled
Mutation testing (nightly) / mutate (push) Has been cancelled
Library trace revalidation (weekly) / revalidate (push) Has been cancelled
defineErrorMiddleware matched by instanceof only; the custom server's tsx-bound DI graph and the Next-compiled route handler each load their own copy of every error class, so mapped domain errors (e.g. UnauthenticatedError -> UNAUTHORIZED) fell through and surfaced as raw 500s — hit in production by a downstream app built from this template. Match on the canonical error name (probed from the constructor's string-literal this.name) with instanceof as the fast path; nameless classes stay identity-only so plain Errors never translate. ADR-027 records the decision and the now-load-bearing this.name convention. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
88 lines
4.7 KiB
Markdown
88 lines
4.7 KiB
Markdown
# 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 = "<ClassName>"` 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 = "<ClassName>"` 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.
|