Torben Sorensen
1bd27d7112
All checks were successful
Deploy to Test Environment / deploy-to-test (push) Successful in 14m37s
43 lines
3.9 KiB
Markdown
43 lines
3.9 KiB
Markdown
# ADR-001: Standardized Error Handling for Service and Repository Layers
|
|
|
|
**Date**: 2025-12-12
|
|
|
|
**Status**: Accepted
|
|
|
|
## Context
|
|
|
|
Our application has experienced a recurring pattern of bugs and brittle tests related to error handling, specifically for "resource not found" scenarios. The root causes identified are:
|
|
|
|
1. **Inconsistent Return Types**: Database repository methods that fetch a single entity (e.g., `getUserById`, `getRecipeById`) had inconsistent behavior when an entity was not found. Some returned `undefined`, some returned `null`, and others threw a generic `Error`.
|
|
2. **Burden on Callers**: This inconsistency forced route handlers (the callers) to implement defensive checks for `undefined` or `null` before sending a response. These checks were often forgotten or implemented incorrectly.
|
|
3. **Incorrect HTTP Status Codes**: When a route handler forgot to check for an `undefined` result and passed it to `res.json()`, the Express framework would interpret this as a server-side failure, resulting in an incorrect `500 Internal Server Error` instead of the correct `404 Not Found`.
|
|
4. **Brittle Tests**: Unit and integration tests for routes were unreliable. Mocks often threw a generic `new Error()` when the actual implementation returned `undefined` or a specific custom error, leading to unexpected `500` status codes in test environments.
|
|
|
|
This pattern led to increased development friction, difficult-to-diagnose bugs, and a fragile test suite.
|
|
|
|
## Decision
|
|
|
|
We will adopt a strict, consistent error-handling contract for the service and repository layers.
|
|
|
|
1. **Always Throw on Not Found**: Any function or method responsible for fetching a single, specific resource (e.g., by ID, checksum, or other unique identifier) **MUST** throw a `NotFoundError` if that resource does not exist. It **MUST NOT** return `null` or `undefined` to signify absence.
|
|
|
|
2. **Use Specific, Custom Errors**: For other known, predictable failure modes (e.g., unique constraint violations, foreign key violations), the repository layer **MUST** throw the corresponding custom `DatabaseError` subclass (e.g., `UniqueConstraintError`, `ForeignKeyConstraintError`).
|
|
|
|
3. **Centralize HTTP Status Mapping**: The `errorHandler` middleware is the **single source of truth** for mapping these specific error types to their corresponding HTTP status codes (e.g., `NotFoundError` -> 404, `UniqueConstraintError` -> 409).
|
|
|
|
4. **Simplify Route Handlers**: Route handlers should be simplified to use a standard `try...catch` block. All errors caught from the service/repository layer should be passed directly to `next(error)`, relying on the `errorHandler` middleware to format the final response. No special `if (result === undefined)` checks are needed.
|
|
|
|
## Consequences
|
|
|
|
### Positive
|
|
|
|
**Robustness**: Eliminates an entire class of bugs where `undefined` is passed to `res.json()`, preventing incorrect `500` errors.
|
|
**Consistency & Predictability**: All data-fetching methods now have a predictable contract. They either return the expected data or throw a specific, typed error.
|
|
**Developer Experience**: Route handlers become simpler, cleaner, and easier to write correctly. The cognitive load on developers is reduced as they no longer need to remember to check for `undefined`.
|
|
**Improved Testability**: Tests become more reliable and realistic. Mocks can now throw the *exact* error type (`new NotFoundError()`) that the real implementation would, ensuring tests accurately reflect the application's behavior.
|
|
**Centralized Control**: Error-to-HTTP-status logic is centralized in the `errorHandler` middleware, making it easy to manage and modify error responses globally.
|
|
|
|
### Negative
|
|
|
|
**Initial Refactoring**: Requires a one-time effort to audit and refactor all existing repository methods to conform to this new standard.
|
|
**Convention Adherence**: Developers must be aware of and adhere to this convention. This ADR serves as the primary documentation for this pattern. |