testing ADR - architectural decisions

src/services/db/0001-standardized-error-handling.md

@@ -0,0 +1,43 @@
+# 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.

src/services/db/recipe.db.ts

@@ -211,7 +211,9 @@ export class RecipeRepository {
       `;
       const res = await this.db.query<Recipe>(query, [recipeId]);
       if (res.rowCount === 0) {
-        throw new NotFoundError(`Recipe with ID ${recipeId} not found.`);
+        // A recipe not being found is an expected case, so we throw a specific
+        // error that our error handler middleware can convert to a 404 status.
+        throw new NotFoundError(`Recipe with ID ${recipeId} not found`);
       }
       return res.rows[0];
     } catch (error) {