fix(server): map non-enum error codes to translatable ErrorCode members - #1843
Conversation
DAV token 404 responses hand-rolled a DAV_TOKEN_NOT_FOUND code that was never in the ErrorCode enum, so the client couldn't translate it. Reuse NOT_FOUND instead — the client never branched on the DAV-specific code (the /profile endpoint is only ever hit via a raw <a href download> link, never through the JS API client). The Fastify error handler also passed through raw Fastify/plugin internal codes (FST_ERR_CTP_BODY_TOO_LARGE, FST_REQ_FILE_TOO_LARGE, etc.) verbatim, or fell back to a made-up REQUEST_ERROR code — neither mappable by translateApiError(). Added a FASTIFY_ERROR_CODE_MAP for known codes, with a status-range fallback (VALIDATION_ERROR for <500, INTERNAL_ERROR for >=500) for anything unmapped. HTTP status codes are preserved exactly; only the `code` field changes. While researching the fix, found PAYLOAD_TOO_LARGE was already a live enum member thrown in production (photos.ts) with zero translations in either locale — the same class of bug. Added English and German keys since the new FST_* mapping routes more traffic through that code. Fixes #1811 Co-Authored-By: Claude dev-team-lead (Sonnet 4.6) <noreply@anthropic.com> Co-Authored-By: Claude backend-developer (Haiku 4.5) <noreply@anthropic.com> Co-Authored-By: Claude frontend-developer (Haiku 4.5) <noreply@anthropic.com> Co-Authored-By: Claude translator (Sonnet 4.5) <noreply@anthropic.com> Co-Authored-By: Claude qa-integration-tester (Sonnet 4.5) <noreply@anthropic.com>
|
[security-engineer] Security review of PR #1843 ( Scope reviewed: Verdict: APPROVED AnalysisA05 Security Misconfiguration / A09 Logging — error-code normalization (net positive)
DAV token endpoints — auth posture unchanged
Locale files
Tests
Non-blocking observations (informational, no action required for merge)
No injection, auth bypass, IDOR, or sensitive-data-exposure issues found. Approved. |
steilerDev
left a comment
There was a problem hiding this comment.
[product-architect] APPROVED
Reviewed against the API contract (error envelope) and the ErrorCode enum. This is a clean conformance fix — it brings previously non-conformant error paths back into line with the documented envelope + enum, and every response code now resolves to a translatable enum member.
Verified
- Error envelope unchanged. Still
{ error: { code, message, details? } }. HTTP status codes preserved on every path (413 stays 413, etc.). Only thecodevalue changes. - All emitted codes are enum members. The three fallback/mapping targets (
PAYLOAD_TOO_LARGE,VALIDATION_ERROR,INTERNAL_ERROR) all exist inshared/src/types/errors.ts, and bothenanddeerrors.json carry translations for all of them (plusNOT_FOUND). No untranslatable code can escape anymore. - NOT_FOUND reuse for DAV — agree.
DAV_TOKEN_NOT_FOUNDwas never in the enum, so it was a latent untranslatable-code bug, not a real contract surface. The/profileendpoint is reached only via a raw download link, never the JS API client, so nothing branches on the DAV-specific string. Minting a new enum member would add translation work and API surface for zero consumer benefit. ReusingNotFoundError(NOT_FOUND/404) while preserving the specific"No DAV token configured"message is the right call and consistent with the "structured codes must be enum members" principle. No lingeringDAV_TOKEN_NOT_FOUNDreferences remain in server/client/shared/e2e. - FST_ table + fallback.* The 13-entry table covers the realistic body/content-type/multipart failure codes. Crucially, completeness is not load-bearing: the status-range fallback guarantees any unmapped
FST_*code still resolves to a valid enum member, so there is no path back to a rawFST_*string or the old made-upREQUEST_ERROR. - Scope containment confirmed. Only the Fastify-internal branch changed. The AppError branch (uses
error.code, already enum), the AJVerror.validationbranch (VALIDATION_ERROR), the unknown-error branch (INTERNAL_ERROR), and the separateROUTE_NOT_FOUNDnot-found handler are all untouched. The new branch keeps its!error.validationguard, and the validation branch runs first, so there is no overlap or regression. - Tests. New coverage exercises the real oversized-body/invalid-JSON/empty-body paths via
app.inject()plus synthetic multipart and both fallback branches (<500 and >=500), and explicitly asserts the code is neitherundefinednorREQUEST_ERROR. DAV test updated to assertNOT_FOUND+ message.
Non-blocking notes (informational)
- Status-range fallback semantics. A hypothetical unmapped Fastify-internal 401/403/429 would collapse to
VALIDATION_ERROR. In practice those paths are all served byAppErrorsubclasses (UNAUTHORIZED,FORBIDDEN,RATE_LIMIT_EXCEEDED), so the fallback only ever sees generic client/server errors. Acceptable at this scale — the fallback's job is to guarantee a translatable code, not perfect semantic fidelity. Worth revisiting only if a plugin ever emits an internal auth/rate-limit error outside the AppError path. - Doc-comment typo ("see fallbackErrorCode()" should read
mapFastifyErrorCode()) — already known and deferred, not blocking.
No API-Contract or Schema wiki update required: this changes no contract surface, it restores conformance to the existing envelope and enum.
|
🎉 This PR is included in version 2.13.0-beta.11 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.13.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
DAV_TOKEN_NOT_FOUNDcode that was never in theErrorCodeenum — now reusesNOT_FOUND(the/profileendpoint is only ever reached via a raw<a href download>link, never the JS API client, so no code branches on the DAV-specific string).FST_ERR_CTP_BODY_TOO_LARGE,FST_REQ_FILE_TOO_LARGE, etc.) or a made-upREQUEST_ERRORfallback, neither translatable by the client. Added aFASTIFY_ERROR_CODE_MAPfor known codes plus a status-range fallback (VALIDATION_ERRORfor <500,INTERNAL_ERRORfor >=500). HTTP status codes are unchanged — onlycodechanges.PAYLOAD_TOO_LARGEwas already live in production (photos.ts) with zero client translations — same bug class, fixed alongside since the new mapping routes more traffic through it.Fixes #1811
Test plan
davTokens.test.ts+errorHandler.test.ts,errorTranslation.test.ts22/22app.inject()scenarios for oversized body, malformed/empty JSON, plus synthetic-error scenarios for multipart and fallback pathsCo-Authored-By: Claude dev-team-lead (Sonnet 4.6) noreply@anthropic.com