Skip to content

Two of the seven AppError subclasses (ValidationError, InternalError) are defined but never instantiated anywhere in the codebase #134

Description

@dev-fani
  • Labels: technical-debt, refactor, backend
  • Area / component: shared/errors/app-error.ts, shared/errors/error-handler.ts

Problem
shared/errors/app-error.ts defines seven AppError subclasses as "the single domain error hierarchy" every module's errors extend. Two of the seven — ValidationError and InternalError — are never constructed or subclassed anywhere in the application.

Current behavior
Confirmed via direct grep for new ValidationError/extends ValidationError and new InternalError/extends InternalError across src/ (excluding the definition file itself and specs): zero results for both. The 400 VALIDATION_ERROR behavior these classes' names imply is instead produced entirely through separate code paths — Zod's own ZodError, and Fastify's FST_ERR_VALIDATION-coded validation errors — both handled by dedicated branches in error-handler.ts that never touch the ValidationError class. Similarly, every genuine 500 INTERNAL_ERROR response is produced by error-handler.ts's final generic fallback branch, which hardcodes code: 'INTERNAL_ERROR' as a string literal rather than constructing (or type-checking against) an InternalError instance — the class the hierarchy already defines for exactly this purpose sits unused one branch away from where it would apply.

Evidence / code location

  • src/shared/errors/app-error.ts:21-24 (ValidationError) and :58-61 (InternalError) — both defined, neither ever instantiated per the grep above.
  • src/shared/errors/error-handler.ts:55-84 — the ZodError/FST_ERR_VALIDATION branches that produce VALIDATION_ERROR without going through the ValidationError class.
  • src/shared/errors/error-handler.ts:113-117 — the generic fallback that hardcodes code: 'INTERNAL_ERROR' as a literal rather than via InternalError.
  • docs/SECURITY.md, ARCHITECTURE.md § 9 — both describe "the single domain error hierarchy" as the uniform mechanism every error maps through, which is not quite accurate for these two members.

Impact
Minor, but a real inconsistency in a hierarchy the project's own documentation presents as complete and uniform: two of its seven members are decorative, and the code paths that produce their exact same HTTP status/code do so without ever touching them, undermining the "one hierarchy, one mapping" design goal for those two cases specifically.

Expected behavior
Either both classes are put to actual use (or removed, if genuinely superfluous given Zod/the generic fallback already cover their cases), and error-handler.ts's generic fallback and validation branches are internally consistent with whichever choice is made.

Proposed scope / implementation direction

  1. For InternalError: have error-handler.ts's final fallback branch construct one (or at least assert its .code/.statusCode match the hardcoded literal via a shared constant), so the class and the fallback can't drift independently.
  2. For ValidationError: either find a genuine call site where a domain-level (not framework-level) validation failure should throw it — e.g., a cross-field business-rule check that Zod's schema-only validation can't express — or remove it from the hierarchy if no such call site is warranted, documenting that domain-level input shape validation is intentionally handled entirely by Zod/Fastify rather than this class.
  3. Update docs/SECURITY.md/ARCHITECTURE.md if the resolution changes what "the single error hierarchy" actually covers.

Acceptance criteria

  • Every remaining class in the hierarchy has at least one real call site, or the hierarchy's documentation is corrected to state which classes are reserved for future use and why.
  • error-handler.ts's fallback and validation branches are consistent with the resolution chosen.

Verification / testing requirements

  • If a new call site is added: a unit test exercising it.
  • If a class is removed: confirm no remaining reference anywhere in src/.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    backendBackend application coderefactorStructural cleanup without behaviour changetechnical-debtDead code, duplication, or unfinished scaffolding

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions