Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ jobs:
- name: Lint
run: npm run lint

# TODO: duplicate build step; consider removing from here and only keeping in build-and-push
# once we figure a better way to prevent build errors from reaching build-and-push job.
- name: Build
run: npm run build
Comment on lines +51 to +54
Copy link

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment indicates this is a duplicate build step and acknowledges it's redundant. Having duplicate build steps increases CI time and resource usage unnecessarily. Consider implementing the suggested solution of ensuring the build-and-push job only runs after successful test/lint steps, or add a dedicated build job that both test and build-and-push depend on. The TODO should be resolved rather than leaving duplicate steps in production.

Copilot uses AI. Check for mistakes.

- name: Test
run: npm run test

Expand Down
21 changes: 16 additions & 5 deletions src/api/middleware/errorHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,26 +31,37 @@ export function errorHandler(
): void {
const errorId = generateErrorId();

// Extract error properties with safe type handling.
// We accept Error | unknown and need to safely access properties that may exist
// on Error objects (message), HTTP error objects (statusCode, status), or custom
// error objects (code). Type casting to any is necessary to access these
// arbitrary properties while maintaining runtime safety through optional chaining.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const errorObj = err as any; // Allow accessing arbitrary properties
const errorMessage = errorObj?.message ?? String(err);
const errorType = errorObj?.constructor?.name ?? 'Unknown';
const statusCode = errorObj?.statusCode ?? errorObj?.status ?? 500;
const errorCode = errorObj?.code;

// Log the full error internally with correlation ID (safe location)
log.error({
message: 'API error',
data: {
errorId,
type: err?.constructor?.name || 'Unknown',
message: err?.message || String(err)
type: errorType,
message: errorMessage
}
});

// Return safe error response to client with error ID for tracing
const statusCode = err?.statusCode || err?.status || 500;
const errorResponse: ErrorResponse = {
error: 'An error occurred processing your request',
errorId
};

// Add code if it's a validation error or known error type
if (err?.code) {
errorResponse.code = err.code;
if (errorCode) {
errorResponse.code = errorCode;
}

res.status(statusCode).json(errorResponse);
Comment on lines 56 to 67
Copy link

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error response always returns a generic message "An error occurred processing your request" regardless of the error type. While this prevents information leakage, it makes the API difficult to use because clients cannot distinguish between different error types (e.g., validation errors vs. server errors vs. not found). Consider categorizing common error types and returning more specific error messages for 400-level errors while keeping 500-level errors generic. For example, validation errors could return "Invalid request parameters" while keeping internal server errors generic.

Copilot uses AI. Check for mistakes.
Expand Down
19 changes: 14 additions & 5 deletions src/api/middleware/logging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,18 @@ export function requestLogging(req: Request, res: Response, next: NextFunction):
const startTime = Date.now();
const clientIp = getClientIP(req);

// Override res.end to capture response
const originalEnd = res.end;
res.end = function (chunk?: Buffer | string, encoding?: string): Response {
// Override res.end to capture response timings and metadata.
// Express Response.end has multiple overloaded signatures:
// end(): Response
// end(callback: Function): Response
// end(data: Buffer | string): Response
// end(data: Buffer | string, callback: Function): Response
// end(data: Buffer | string, encoding: string, callback: Function): Response
// We accept variadic args to match all overloads while preserving the original
// function's ability to handle any combination of parameters.
const originalEnd = res.end.bind(res);
// eslint-disable-next-line @typescript-eslint/no-explicit-any
res.end = function (...args: any[]): Response {
const duration = Date.now() - startTime;

log.debug({
Expand All @@ -27,8 +36,8 @@ export function requestLogging(req: Request, res: Response, next: NextFunction):
}
});

return originalEnd.call(this, chunk, encoding);
};
return originalEnd(...args);
} as typeof res.end;

next();
}
Expand Down