Files
flyer-crawler.projectium.com/docs/development/ERROR-LOGGING-PATHS.md
Torben Sorensen 4f06698dfd
Some checks failed
Deploy to Test Environment / deploy-to-test (push) Failing after 2m50s
test fixes and doc work
2026-01-28 15:33:48 -08:00

154 lines
5.5 KiB
Markdown

# Error Logging Path Patterns
## Overview
This document describes the correct pattern for logging request paths in error handlers within Express route files. Following this pattern ensures that error logs accurately reflect the actual request URL, including any API version prefixes.
## The Problem
When ADR-008 (API Versioning Strategy) was implemented, all routes were moved from `/api/*` to `/api/v1/*`. However, some error log messages contained hardcoded paths that did not update automatically:
```typescript
// INCORRECT - hardcoded path
req.log.error({ error }, 'Error in /api/flyers/:id:');
```
This caused 16 unit test failures because tests expected the error log message to contain `/api/v1/flyers/:id` but received `/api/flyers/:id`.
## The Solution
Always use `req.originalUrl` to dynamically capture the actual request path in error logs:
```typescript
// CORRECT - dynamic path from request
req.log.error({ error }, `Error in ${req.originalUrl.split('?')[0]}:`);
```
### Why `req.originalUrl`?
| Property | Value for `/api/v1/flyers/123?active=true` | Use Case |
| ----------------- | ------------------------------------------ | ----------------------------------- |
| `req.url` | `/123?active=true` | Path relative to router mount point |
| `req.path` | `/123` | Path without query string |
| `req.originalUrl` | `/api/v1/flyers/123?active=true` | Full original request URL |
| `req.baseUrl` | `/api/v1/flyers` | Router mount path |
`req.originalUrl` is the correct choice because:
1. It contains the full path including version prefix (`/api/v1/`)
2. It reflects what the client actually requested
3. It makes log messages searchable by the actual endpoint path
4. It automatically adapts when routes are mounted at different paths
### Stripping Query Parameters
Use `.split('?')[0]` to remove query parameters from log messages:
```typescript
// Request: /api/v1/flyers?page=1&limit=20
req.originalUrl.split('?')[0]; // Returns: /api/v1/flyers
```
This keeps log messages clean and prevents sensitive query parameters from appearing in logs.
## Standard Error Logging Pattern
### Basic Pattern
```typescript
router.get('/:id', async (req, res) => {
try {
const result = await someService.getData(req.params.id);
return sendSuccess(res, result);
} catch (error) {
req.log.error({ error }, `Error in ${req.originalUrl.split('?')[0]}:`);
return sendError(res, error);
}
});
```
### With Additional Context
```typescript
router.post('/', async (req, res) => {
try {
const result = await someService.createItem(req.body);
return sendSuccess(res, result, 'Item created', 201);
} catch (error) {
req.log.error(
{ error, userId: req.user?.id, body: req.body },
`Error creating item in ${req.originalUrl.split('?')[0]}:`,
);
return sendError(res, error);
}
});
```
### Descriptive Messages
For clarity, include a brief description of the operation:
```typescript
// Good - describes the operation
req.log.error({ error }, `Error fetching recipes in ${req.originalUrl.split('?')[0]}:`);
req.log.error({ error }, `Error updating user profile in ${req.originalUrl.split('?')[0]}:`);
// Acceptable - just the path
req.log.error({ error }, `Error in ${req.originalUrl.split('?')[0]}:`);
// Bad - hardcoded path
req.log.error({ error }, 'Error in /api/recipes:');
```
## Files Updated in Initial Fix (2026-01-27)
The following files were updated to use this pattern:
| File | Error Log Statements Fixed |
| -------------------------------------- | -------------------------- |
| `src/routes/recipe.routes.ts` | 3 |
| `src/routes/stats.routes.ts` | 1 |
| `src/routes/flyer.routes.ts` | 2 |
| `src/routes/personalization.routes.ts` | 3 |
## Testing Error Log Messages
When writing tests that verify error log messages, use flexible matchers that account for versioned paths:
```typescript
// Good - matches any version prefix
expect(logSpy).toHaveBeenCalledWith(
expect.objectContaining({ error: expect.any(Error) }),
expect.stringContaining('/flyers'),
);
// Good - explicit version match
expect(logSpy).toHaveBeenCalledWith(
expect.objectContaining({ error: expect.any(Error) }),
expect.stringContaining('/api/v1/flyers'),
);
// Bad - hardcoded unversioned path (will fail)
expect(logSpy).toHaveBeenCalledWith(
expect.objectContaining({ error: expect.any(Error) }),
'Error in /api/flyers:',
);
```
## Checklist for New Routes
When creating new route handlers:
- [ ] Use `req.originalUrl.split('?')[0]` in all error log messages
- [ ] Include descriptive text about the operation being performed
- [ ] Add structured context (userId, relevant IDs) to the log object
- [ ] Write tests that verify error logs contain the versioned path
## Related Documentation
- [ADR-008: API Versioning Strategy](../adr/0008-api-versioning-strategy.md) - Versioning implementation details
- [ADR-057: Test Remediation Post-API Versioning](../adr/0057-test-remediation-post-api-versioning.md) - Comprehensive remediation guide
- [ADR-004: Structured Logging](../adr/0004-standardized-application-wide-structured-logging.md) - Logging standards
- [CODE-PATTERNS.md](CODE-PATTERNS.md) - General code patterns
- [TESTING.md](TESTING.md) - Testing guidelines