Route API errors through translatable codes, with a status fallback - #142
Merged
Conversation
Implements the "API error codes: envelope, status fallback and guardrails"
Quackback ticket. Raw NestJS exception messages (English, dev-facing) no
longer reach the user directly.
- packages/shared/src/error-codes.ts: shared ErrorCode registry (as const,
same convention as enums.ts) and the ApiErrorBody envelope shape.
- apps/api: AppException (HttpException + code + params) and
ValidationException (structured per-field details from the global
ValidationPipe's exceptionFactory). AllExceptionsFilter now builds the
envelope and sends it via HttpAdapterHost.httpAdapter.reply() —
host.switchToHttp().getResponse() returns the raw Node ServerResponse for
a global filter, not the FastifyReply, so calling .code()/.send() on it
directly crashed every request; this is the platform-agnostic send Nest's
own BaseExceptionFilter uses. 5xx bodies never carry more than a constant
message; the real detail stays in the logs, keyed by requestId.
- apps/web: ApiError carries code/params/details/requestId/retryAfterSeconds;
a rejected fetch (offline, VPS down) is now itself an ApiError
(network.offline) instead of falling through uncaught. resolveApiError()
is the single translation point — known code, unknown/null code falls
back to a per-HTTP-status message (keeps every not-yet-migrated throw
rendering cleanly), 429 interpolates Retry-After when present. All ~50
`err instanceof ApiError ? err.message : fallback` call sites now go
through it; the per-call-site fallback argument was dead weight once the
status/network fallbacks existed, so it — and everything that only existed
to feed it (statsResource's fallbackError param, TrendPeriodCard's
errorMessage prop, LibraryEntryConfig.addErrorMessage, ~19 orphaned
paraglide keys) — was removed rather than kept.
- Retires the ForbiddenException("MFA_REQUIRED") string convention in
AdminGuard in favor of ErrorCode.AuthMfaRequired.
- Migrates a first batch of throws as proof the mechanism works: the 5
hardcoded-French messages that were the original leak this closes,
auth's registration/anti-bot/duplicate-email/account-not-found checks,
and the avatar upload validation.
- Guardrails: a `no-restricted-syntax` ESLint rule (warn, not error — ~185
throw sites are still legitimately unmigrated pending the follow-up
ticket) flags bare NestJS exceptions in apps/api/src; a vitest suite
(new to apps/web — wired into CI's web job) asserts every ErrorCode has a
fr/en translation with no orphans, and unit-tests resolveApiError's
branches directly.
Follow-up Quackback tickets already filed: migrating the remaining ~185
throw sites domain by domain, translating form validation errors,
translating transactional emails, and centralizing API calls
(mandatory error handling / TanStack Query — nothing decided yet).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…mber
Fastify's own request.id is a string ("req-1"), but pino-http's default
genReqId (used when nestjs-pino's LoggerModule wires it up) returns a raw
number instead, and that's what ends up on request.id at runtime. The
envelope's requestId is typed (and documented) as a string, so AllExceptionsFilter
now coerces explicitly rather than trusting the type — caught by e2e in a
real environment (Turnstile-free), where request.id showed up as 26 instead
of "26".
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the "API error codes: envelope, status fallback and guardrails" Quackback ticket (post_01m119ra3qecw9tbrpc34tn0dv). Raw NestJS exception messages (English, dev-facing) no longer reach the user directly.
packages/shared/src/error-codes.ts: sharedErrorCoderegistry (as const, same convention asenums.ts) and theApiErrorBodyenvelope shape.apps/api:AppException(HttpException+ code + params) andValidationException(structured per-field details from the globalValidationPipe'sexceptionFactory).AllExceptionsFilterbuilds the envelope and sends it viaHttpAdapterHost.httpAdapter.reply()—host.switchToHttp().getResponse()returns the raw NodeServerResponsefor a global filter, not theFastifyReply, so calling.code()/.send()directly crashed every request; this is the platform-agnostic sendBaseExceptionFilteritself uses. 5xx bodies never carry more than a constant message; the real detail stays in the logs, keyed byrequestId.apps/web:ApiErrorcarriescode/params/details/requestId/retryAfterSeconds; a rejectedfetch(offline, VPS down) is now itself anApiError(network.offline) instead of falling through uncaught.resolveApiError()is the single translation point — known code, unknown/null code falls back to a per-HTTP-status message (keeps every not-yet-migrated throw rendering cleanly), 429 interpolatesRetry-Afterwhen present. All ~50err instanceof ApiError ? err.message : fallbackcall sites now go through it; the per-call-site fallback argument was dead weight once the status/network fallbacks existed, so it — and everything that only existed to feed it (statsResource'sfallbackErrorparam,TrendPeriodCard'serrorMessageprop,LibraryEntryConfig.addErrorMessage, ~19 orphaned paraglide keys) — was removed.ForbiddenException("MFA_REQUIRED")string convention inAdminGuardin favor ofErrorCode.AuthMfaRequired.no-restricted-syntaxESLint rule (warn, not error — ~185 throw sites are still legitimately unmigrated pending the follow-up ticket) flags bare NestJS exceptions inapps/api/src; a new vitest suite inapps/web(wired into CI's web job) asserts everyErrorCodehas a fr/en translation with no orphans, and unit-testsresolveApiError's branches directly.Follow-up Quackback tickets already filed: migrating the remaining ~185 throw sites domain by domain, translating form validation errors, translating transactional emails, and centralizing API calls (mandatory error handling / TanStack Query — nothing decided yet).
Test plan
pnpm --filter @loomkeep/api exec jest— 820/820 (one pre-existing flaky bcrypt-timing test, unrelated)pnpm --filter @loomkeep/webvitest — 13/13 (newerrors.spec.ts+error-codes-i18n.spec.ts)pnpm --filter @loomkeep/web check— 0 errorspnpm lint— 0 errors (182 expectedno-restricted-syntaxwarnings on not-yet-migrated throws).envmaking registration fail closed with no network path to Cloudflare; confirmed viagit stashthat this predates this branch. Needs verification in an environment where e2e can actually run (CI, or Logan's normal dev setup).🤖 Generated with Claude Code