Skip to content

Route API errors through translatable codes, with a status fallback - #142

Merged
Logan2234 merged 2 commits into
mainfrom
feat/api-error-codes
Aug 27, 2026
Merged

Route API errors through translatable codes, with a status fallback#142
Logan2234 merged 2 commits into
mainfrom
feat/api-error-codes

Conversation

@Logan2234

Copy link
Copy Markdown
Owner

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: 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 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() directly crashed every request; this is the platform-agnostic send BaseExceptionFilter itself 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.
  • 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 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 new vitest suite in 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).

Test plan

  • pnpm --filter @loomkeep/api exec jest — 820/820 (one pre-existing flaky bcrypt-timing test, unrelated)
  • pnpm --filter @loomkeep/web vitest — 13/13 (new errors.spec.ts + error-codes-i18n.spec.ts)
  • pnpm --filter @loomkeep/web check — 0 errors
  • pnpm lint — 0 errors (182 expected no-restricted-syntax warnings on not-yet-migrated throws)
  • e2e — blocked in the dev sandbox by a live Turnstile secret in .env making registration fail closed with no network path to Cloudflare; confirmed via git stash that 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

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>
@github-actions github-actions Bot added ci CI/CD, GitHub Actions workflows api apps/api (NestJS) web apps/web (SvelteKit) shared packages/shared labels Aug 27, 2026
…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>
@Logan2234
Logan2234 merged commit bc5e744 into main Aug 27, 2026
15 of 16 checks passed
@Logan2234
Logan2234 deleted the feat/api-error-codes branch August 27, 2026 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api apps/api (NestJS) ci CI/CD, GitHub Actions workflows shared packages/shared web apps/web (SvelteKit)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant