last test fixes for upcoming V0.1 + pretty
Some checks failed
Deploy to Test Environment / deploy-to-test (push) Failing after 2m40s
Some checks failed
Deploy to Test Environment / deploy-to-test (push) Failing after 2m40s
This commit is contained in:
@@ -34,7 +34,7 @@ We will adopt a strict, consistent error-handling contract for the service and r
|
||||
**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.
|
||||
**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
|
||||
|
||||
@@ -10,21 +10,19 @@ Following the standardization of error handling in ADR-001, the next most common
|
||||
|
||||
This manual approach has several drawbacks:
|
||||
**Repetitive Boilerplate**: The `try/catch/finally` block for transaction management is duplicated across multiple files.
|
||||
**Error-Prone**: It is easy to forget to `client.release()` in all code paths, which can lead to connection pool exhaustion and bring down the application.
|
||||
3. **Poor Composability**: It is difficult to compose multiple repository methods into a single, atomic "Unit of Work". For example, a service function that needs to update a user's points and create a budget in a single transaction cannot easily do so if both underlying repository methods create their own transactions.
|
||||
**Error-Prone**: It is easy to forget to `client.release()` in all code paths, which can lead to connection pool exhaustion and bring down the application. 3. **Poor Composability**: It is difficult to compose multiple repository methods into a single, atomic "Unit of Work". For example, a service function that needs to update a user's points and create a budget in a single transaction cannot easily do so if both underlying repository methods create their own transactions.
|
||||
|
||||
## Decision
|
||||
|
||||
We will implement a standardized "Unit of Work" pattern through a high-level `withTransaction` helper function. This function will abstract away the complexity of transaction management.
|
||||
|
||||
1. **`withTransaction` Helper**: A new helper function, `withTransaction<T>(callback: (client: PoolClient) => Promise<T>): Promise<T>`, will be created. This function will be responsible for:
|
||||
|
||||
* Acquiring a client from the database pool.
|
||||
* Starting a transaction (`BEGIN`).
|
||||
* Executing the `callback` function, passing the transactional client to it.
|
||||
* If the callback succeeds, it will `COMMIT` the transaction.
|
||||
* If the callback throws an error, it will `ROLLBACK` the transaction and re-throw the error.
|
||||
* In all cases, it will `RELEASE` the client back to the pool.
|
||||
- Acquiring a client from the database pool.
|
||||
- Starting a transaction (`BEGIN`).
|
||||
- Executing the `callback` function, passing the transactional client to it.
|
||||
- If the callback succeeds, it will `COMMIT` the transaction.
|
||||
- If the callback throws an error, it will `ROLLBACK` the transaction and re-throw the error.
|
||||
- In all cases, it will `RELEASE` the client back to the pool.
|
||||
|
||||
2. **Repository Method Signature**: Repository methods that need to be part of a transaction will be updated to optionally accept a `PoolClient` in their constructor or as a method parameter. By default, they will use the global pool. When called from within a `withTransaction` block, they will be passed the transactional client.
|
||||
3. **Service Layer Orchestration**: Service-layer functions that orchestrate multi-step operations will use `withTransaction` to ensure atomicity. They will instantiate or call repository methods, providing them with the transactional client from the callback.
|
||||
@@ -40,7 +38,7 @@ async function registerUserAndCreateDefaultList(userData) {
|
||||
const shoppingRepo = new ShoppingRepository(client);
|
||||
|
||||
const newUser = await userRepo.createUser(userData);
|
||||
await shoppingRepo.createShoppingList(newUser.user_id, "My First List");
|
||||
await shoppingRepo.createShoppingList(newUser.user_id, 'My First List');
|
||||
|
||||
return newUser;
|
||||
});
|
||||
|
||||
@@ -20,8 +20,8 @@ We will adopt a schema-based approach for input validation using the `zod` libra
|
||||
1. **Adopt `zod` for Schema Definition**: We will use `zod` to define clear, type-safe schemas for the `params`, `query`, and `body` of each API request. `zod` provides powerful and declarative validation rules and automatically infers TypeScript types.
|
||||
|
||||
2. **Create a Reusable Validation Middleware**: A generic `validateRequest(schema)` middleware will be created. This middleware will take a `zod` schema, parse the incoming request against it, and handle success and error cases.
|
||||
* On successful validation, the parsed and typed data will be attached to the `req` object (e.g., `req.body` will be replaced with the parsed body), and `next()` will be called.
|
||||
* On validation failure, the middleware will call `next()` with a custom `ValidationError` containing a structured list of issues, which `ADR-001`'s `errorHandler` can then format into a user-friendly `400 Bad Request` response.
|
||||
- On successful validation, the parsed and typed data will be attached to the `req` object (e.g., `req.body` will be replaced with the parsed body), and `next()` will be called.
|
||||
- On validation failure, the middleware will call `next()` with a custom `ValidationError` containing a structured list of issues, which `ADR-001`'s `errorHandler` can then format into a user-friendly `400 Bad Request` response.
|
||||
|
||||
3. **Refactor Routes**: All route handlers will be refactored to use this new middleware, removing all manual validation logic.
|
||||
|
||||
@@ -46,18 +46,18 @@ const getFlyerSchema = z.object({
|
||||
type GetFlyerRequest = z.infer<typeof getFlyerSchema>;
|
||||
|
||||
// 3. Apply the middleware and use an inline cast for the request
|
||||
router.get('/:id', validateRequest(getFlyerSchema), (async (req, res, next) => {
|
||||
// Cast 'req' to the inferred type.
|
||||
// This provides full type safety for params, query, and body.
|
||||
const { params } = req as unknown as GetFlyerRequest;
|
||||
router.get('/:id', validateRequest(getFlyerSchema), async (req, res, next) => {
|
||||
// Cast 'req' to the inferred type.
|
||||
// This provides full type safety for params, query, and body.
|
||||
const { params } = req as unknown as GetFlyerRequest;
|
||||
|
||||
try {
|
||||
const flyer = await db.flyerRepo.getFlyerById(params.id); // params.id is 'number'
|
||||
res.json(flyer);
|
||||
} catch (error) {
|
||||
next(error);
|
||||
}
|
||||
}));
|
||||
try {
|
||||
const flyer = await db.flyerRepo.getFlyerById(params.id); // params.id is 'number'
|
||||
res.json(flyer);
|
||||
} catch (error) {
|
||||
next(error);
|
||||
}
|
||||
});
|
||||
```
|
||||
|
||||
## Consequences
|
||||
|
||||
@@ -20,9 +20,9 @@ We will adopt a standardized, application-wide structured logging policy. All lo
|
||||
|
||||
**Request-Scoped Logger with Context**: We will create a middleware that runs at the beginning of the request lifecycle. This middleware will:
|
||||
|
||||
* Generate a unique `request_id` for each incoming request.
|
||||
* Create a request-scoped logger instance (a "child logger") that automatically includes the `request_id`, `user_id` (if authenticated), and `ip_address` in every log message it generates.
|
||||
* Attach this child logger to the `req` object (e.g., `req.log`).
|
||||
- Generate a unique `request_id` for each incoming request.
|
||||
- Create a request-scoped logger instance (a "child logger") that automatically includes the `request_id`, `user_id` (if authenticated), and `ip_address` in every log message it generates.
|
||||
- Attach this child logger to the `req` object (e.g., `req.log`).
|
||||
|
||||
**Mandatory Use of Request-Scoped Logger**: All route handlers and any service functions called by them **MUST** use the request-scoped logger (`req.log`) instead of the global logger instance. This ensures all logs for a given request are automatically correlated.
|
||||
|
||||
@@ -32,9 +32,9 @@ We will adopt a standardized, application-wide structured logging policy. All lo
|
||||
|
||||
**Standardized Logging Practices**:
|
||||
**INFO**: Log key business events, such as `User logged in` or `Flyer processed`.
|
||||
**WARN**: Log recoverable errors or unusual situations that do not break the request, such as `Client Error: 404 on GET /api/non-existent-route` or `Retrying failed database connection`.
|
||||
**ERROR**: Log only unhandled or server-side errors that cause a request to fail (typically handled by the `errorHandler`). Avoid logging expected client errors (like 4xx) at this level.
|
||||
**DEBUG**: Log detailed diagnostic information useful during development, such as function entry/exit points or variable states.
|
||||
**WARN**: Log recoverable errors or unusual situations that do not break the request, such as `Client Error: 404 on GET /api/non-existent-route` or `Retrying failed database connection`.
|
||||
**ERROR**: Log only unhandled or server-side errors that cause a request to fail (typically handled by the `errorHandler`). Avoid logging expected client errors (like 4xx) at this level.
|
||||
**DEBUG**: Log detailed diagnostic information useful during development, such as function entry/exit points or variable states.
|
||||
|
||||
### Example Usage
|
||||
|
||||
@@ -59,15 +59,15 @@ export const requestLogger = (req, res, next) => {
|
||||
|
||||
// In a route handler:
|
||||
router.get('/:id', async (req, res, next) => {
|
||||
// Use the request-scoped logger
|
||||
req.log.info({ flyerId: req.params.id }, 'Fetching flyer by ID');
|
||||
try {
|
||||
// ... business logic ...
|
||||
res.json(flyer);
|
||||
} catch (error) {
|
||||
// The error itself will be logged with full context by the errorHandler
|
||||
next(error);
|
||||
}
|
||||
// Use the request-scoped logger
|
||||
req.log.info({ flyerId: req.params.id }, 'Fetching flyer by ID');
|
||||
try {
|
||||
// ... business logic ...
|
||||
res.json(flyer);
|
||||
} catch (error) {
|
||||
// The error itself will be logged with full context by the errorHandler
|
||||
next(error);
|
||||
}
|
||||
});
|
||||
```
|
||||
|
||||
|
||||
@@ -14,5 +14,5 @@ We will formalize a centralized Role-Based Access Control (RBAC) or Attribute-Ba
|
||||
|
||||
## Consequences
|
||||
|
||||
* **Positive**: Ensures authorization logic is consistent, easy to audit, and decoupled from business logic. Improves security by centralizing access control.
|
||||
* **Negative**: Requires a significant refactoring effort to integrate the new authorization system across all protected routes and features. Introduces a new dependency if an external library is chosen.
|
||||
- **Positive**: Ensures authorization logic is consistent, easy to audit, and decoupled from business logic. Improves security by centralizing access control.
|
||||
- **Negative**: Requires a significant refactoring effort to integrate the new authorization system across all protected routes and features. Introduces a new dependency if an external library is chosen.
|
||||
|
||||
@@ -14,5 +14,5 @@ We will establish a formal Design System and Component Library. This will involv
|
||||
|
||||
## Consequences
|
||||
|
||||
* **Positive**: Ensures a consistent and high-quality user interface. Accelerates frontend development by providing reusable, well-documented components. Improves maintainability and reduces technical debt.
|
||||
* **Negative**: Requires an initial investment in setting up Storybook and migrating existing components. Adds a new dependency and a new workflow for frontend development.
|
||||
- **Positive**: Ensures a consistent and high-quality user interface. Accelerates frontend development by providing reusable, well-documented components. Improves maintainability and reduces technical debt.
|
||||
- **Negative**: Requires an initial investment in setting up Storybook and migrating existing components. Adds a new dependency and a new workflow for frontend development.
|
||||
|
||||
@@ -14,5 +14,5 @@ We will adopt a dedicated database migration tool, such as **`node-pg-migrate`**
|
||||
|
||||
## Consequences
|
||||
|
||||
* **Positive**: Provides a safe, repeatable, and reversible way to evolve the database schema. Improves team collaboration on database changes. Reduces the risk of data loss or downtime during deployments.
|
||||
* **Negative**: Requires an initial setup and learning curve for the chosen migration tool. All future schema changes must adhere to the migration workflow.
|
||||
- **Positive**: Provides a safe, repeatable, and reversible way to evolve the database schema. Improves team collaboration on database changes. Reduces the risk of data loss or downtime during deployments.
|
||||
- **Negative**: Requires an initial setup and learning curve for the chosen migration tool. All future schema changes must adhere to the migration workflow.
|
||||
|
||||
@@ -14,5 +14,5 @@ We will standardize the deployment process by containerizing the application usi
|
||||
|
||||
## Consequences
|
||||
|
||||
* **Positive**: Ensures consistency between development and production environments. Simplifies the setup for new developers. Improves portability and scalability of the application.
|
||||
* **Negative**: Requires learning Docker and containerization concepts. Adds `Dockerfile` and `docker-compose.yml` to the project's configuration.
|
||||
- **Positive**: Ensures consistency between development and production environments. Simplifies the setup for new developers. Improves portability and scalability of the application.
|
||||
- **Negative**: Requires learning Docker and containerization concepts. Adds `Dockerfile` and `docker-compose.yml` to the project's configuration.
|
||||
|
||||
@@ -18,5 +18,5 @@ We will implement a multi-layered security approach for the API:
|
||||
|
||||
## Consequences
|
||||
|
||||
* **Positive**: Significantly improves the application's security posture against common web vulnerabilities like XSS, clickjacking, and brute-force attacks.
|
||||
* **Negative**: Requires careful configuration of CORS and rate limits to avoid blocking legitimate traffic. Content-Security-Policy can be complex to configure correctly.
|
||||
- **Positive**: Significantly improves the application's security posture against common web vulnerabilities like XSS, clickjacking, and brute-force attacks.
|
||||
- **Negative**: Requires careful configuration of CORS and rate limits to avoid blocking legitimate traffic. Content-Security-Policy can be complex to configure correctly.
|
||||
|
||||
@@ -14,5 +14,5 @@ We will formalize the end-to-end CI/CD process. This ADR will define the project
|
||||
|
||||
## Consequences
|
||||
|
||||
* **Positive**: Automates quality control and creates a safe, repeatable path to production. Increases development velocity and reduces deployment-related errors.
|
||||
* **Negative**: Initial setup effort for the CI/CD pipeline. May slightly increase the time to merge code due to mandatory checks.
|
||||
- **Positive**: Automates quality control and creates a safe, repeatable path to production. Increases development velocity and reduces deployment-related errors.
|
||||
- **Negative**: Initial setup effort for the CI/CD pipeline. May slightly increase the time to merge code due to mandatory checks.
|
||||
|
||||
@@ -14,5 +14,5 @@ We will adopt **OpenAPI (Swagger)** for API documentation. We will use tools (e.
|
||||
|
||||
## Consequences
|
||||
|
||||
* **Positive**: Creates a single source of truth for API documentation that stays in sync with the code. Enables auto-generation of client SDKs and simplifies testing.
|
||||
* **Negative**: Requires developers to maintain JSDoc annotations on all routes. Adds a build step and new dependencies to the project.
|
||||
- **Positive**: Creates a single source of truth for API documentation that stays in sync with the code. Enables auto-generation of client SDKs and simplifies testing.
|
||||
- **Negative**: Requires developers to maintain JSDoc annotations on all routes. Adds a build step and new dependencies to the project.
|
||||
|
||||
@@ -14,5 +14,5 @@ We will implement a formal data backup and recovery strategy. This will involve
|
||||
|
||||
## Consequences
|
||||
|
||||
* **Positive**: Protects against catastrophic data loss, ensuring business continuity. Provides a clear, tested plan for disaster recovery.
|
||||
* **Negative**: Requires setup and maintenance of backup scripts and secure storage. Incurs storage costs for backup files.
|
||||
- **Positive**: Protects against catastrophic data loss, ensuring business continuity. Provides a clear, tested plan for disaster recovery.
|
||||
- **Negative**: Requires setup and maintenance of backup scripts and secure storage. Incurs storage costs for backup files.
|
||||
|
||||
@@ -12,11 +12,11 @@ When the application is containerized (`ADR-014`), the container orchestrator (e
|
||||
|
||||
We will implement dedicated health check endpoints in the Express application.
|
||||
|
||||
* A **Liveness Probe** (`/api/health/live`) will return a `200 OK` to indicate the server is running. If it fails, the orchestrator should restart the container.
|
||||
- A **Liveness Probe** (`/api/health/live`) will return a `200 OK` to indicate the server is running. If it fails, the orchestrator should restart the container.
|
||||
|
||||
* A **Readiness Probe** (`/api/health/ready`) will return a `200 OK` only if the application is ready to accept traffic (e.g., database connection is established). If it fails, the orchestrator will temporarily remove the container from the load balancer.
|
||||
- A **Readiness Probe** (`/api/health/ready`) will return a `200 OK` only if the application is ready to accept traffic (e.g., database connection is established). If it fails, the orchestrator will temporarily remove the container from the load balancer.
|
||||
|
||||
## Consequences
|
||||
|
||||
* **Positive**: Enables robust, automated application lifecycle management in a containerized environment. Prevents traffic from being sent to unhealthy or uninitialized application instances.
|
||||
* **Negative**: Adds a small amount of code for the health check endpoints. Requires configuration in the container orchestration layer.
|
||||
- **Positive**: Enables robust, automated application lifecycle management in a containerized environment. Prevents traffic from being sent to unhealthy or uninitialized application instances.
|
||||
- **Negative**: Adds a small amount of code for the health check endpoints. Requires configuration in the container orchestration layer.
|
||||
|
||||
@@ -24,8 +24,8 @@ We will adopt a standardized, application-wide structured logging policy for all
|
||||
|
||||
**2. Pino-like API for Structured Logging**: The client logger mimics the `pino` API, which is the standard on the backend. It supports two primary call signatures:
|
||||
|
||||
* `logger.info('A simple message');`
|
||||
* `logger.info({ key: 'value' }, 'A message with a structured data payload');`
|
||||
- `logger.info('A simple message');`
|
||||
- `logger.info({ key: 'value' }, 'A message with a structured data payload');`
|
||||
|
||||
The second signature, which includes a data object as the first argument, is **strongly preferred**, especially for logging errors or complex state.
|
||||
|
||||
@@ -79,7 +79,7 @@ describe('MyComponent', () => {
|
||||
// Assert that the logger was called with the expected structure
|
||||
expect(logger.error).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ err: expect.any(Error) }), // Check for the error object
|
||||
'Failed to fetch component data' // Check for the message
|
||||
'Failed to fetch component data', // Check for the message
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user