Refactor: Structured logging and type safety improvements#9
Conversation
- Replace console.* with NestJS Logger service in API - Add comprehensive error logging in HttpErrorFilter - Improve JSON parsing error handling in api-client - Add context (URL, method) to unhandled exception logs Co-authored-by: BENZOOgataga <50145143+BENZOOgataga@users.noreply.github.com>
- Extract test mock utility to reduce code duplication - Add proper type guard for Prisma errors - Document intentional type assertion in test utilities Co-authored-by: BENZOOgataga <50145143+BENZOOgataga@users.noreply.github.com>
Co-authored-by: BENZOOgataga <50145143+BENZOOgataga@users.noreply.github.com>
- Simplify logger.error() call to avoid duplicating exception message - Structure context object with separate message and stack fields Co-authored-by: BENZOOgataga <50145143+BENZOOgataga@users.noreply.github.com>
Greptile OverviewGreptile SummaryReplaced ad-hoc The changes align with AGENTS.md's code style requirements: TypeScript strict mode, avoiding unsafe Confidence Score: 5/5
Last reviewed commit: f75f2ab |
There was a problem hiding this comment.
Pull request overview
This PR refactors error handling and logging across the codebase, replacing ad-hoc console logging with NestJS Logger service for structured logging, and improves type safety in tests by consolidating duplicate mock utilities.
Changes:
- Replaced console logging with NestJS Logger in API bootstrap, Prisma service, and HTTP error filter
- Added
isPrismaErrorWithCode()type guard for safer Prisma error handling - Created shared
createPrismaTransactionMock()test utility to eliminate code duplication - Enhanced JSON parse error logging in api-client for better debugging in development mode
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/sim/tests/test-utils.ts | New shared test utility for creating Prisma transaction mocks with proper documentation |
| packages/sim/tests/workforce.test.ts | Removed duplicate createPrismaTransactionMock implementation, now imports from test-utils |
| packages/sim/tests/research.test.ts | Removed duplicate createPrismaTransactionMock implementation, now imports from test-utils |
| apps/web/src/lib/api-client.ts | Enhanced JSON parse error handling with development-mode logging instead of silent failure |
| apps/api/src/prisma/prisma.service.ts | Replaced console.warn with NestJS Logger and added type guard for Prisma errors |
| apps/api/src/main.ts | Replaced console.error with NestJS Logger for bootstrap failures |
| apps/api/src/common/filters/http-error.filter.ts | Added structured logging for unhandled exceptions with request context |
| .releases/unreleased/code-quality-improvements.md | Patch changeset documenting error handling and type safety improvements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| interface PrismaErrorWithCode extends Error { | ||
| errorCode?: string; | ||
| } | ||
|
|
||
| function isPrismaErrorWithCode(error: unknown): error is PrismaErrorWithCode { | ||
| return error instanceof Error && "errorCode" in error; | ||
| } |
There was a problem hiding this comment.
The Prisma error type guard only checks for 'errorCode', but other parts of the codebase show that Prisma errors can have either 'errorCode' or 'code' properties. The schema-readiness service checks both properties (schema-readiness.service.ts:76), and maintenance service uses 'code' (maintenance.service.ts:279). Consider updating the interface and type guard to handle both properties to ensure all Prisma connection errors are properly detected for retries.
| this.logger.error("Unhandled exception", { | ||
| url: request?.url, | ||
| method: request?.method, | ||
| message: exception instanceof Error ? exception.message : String(exception), | ||
| stack: exception instanceof Error ? exception.stack : undefined | ||
| }); |
There was a problem hiding this comment.
The NestJS Logger.error method signature expects a stack trace string as the second argument, not a context object. Consider restructuring this to pass the stack trace directly as the second parameter, or format it as a string. For example: this.logger.error('Unhandled exception at ${request?.method} ${request?.url}: ${message}', stack) where message and stack are extracted from the exception.
| this.logger.error("Unhandled exception", { | |
| url: request?.url, | |
| method: request?.method, | |
| message: exception instanceof Error ? exception.message : String(exception), | |
| stack: exception instanceof Error ? exception.stack : undefined | |
| }); | |
| const errorMessage = `Unhandled exception at ${request?.method ?? "UNKNOWN"} ${request?.url ?? "UNKNOWN"}: ${ | |
| exception instanceof Error ? exception.message : String(exception) | |
| }`; | |
| const stack = exception instanceof Error ? exception.stack : undefined; | |
| this.logger.error(errorMessage, stack); |
Comprehensive code review identified error handling and type safety issues. No architectural problems found.
Error Handling & Logging
Replaced ad-hoc console logging with NestJS Logger service:
Type Safety
createPrismaTransactionMock()utility to consolidate duplicate test mock codeisPrismaErrorWithCode()type guard for Prisma error handling instead of unsafe assertionsCode Quality Assessment
Reviewed for duplication and magic numbers:
Security
CodeQL scan: 0 vulnerabilities
Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
checkpoint.prisma.io/opt/hostedtoolcache/node/24.13.0/x64/bin/node /opt/hostedtoolcache/node/24.13.0/x64/bin/node /home/REDACTED/work/CorpSim/CorpSim/node_modules/.pnpm/prisma@6.19.2_typescript@5.9.3/node_modules/prisma/build/child {"product":"prisma","version":"6.19.2","cli_install_type":"local","information":"","local_timestamp":"2026-02-15T02:16:53Z","project_hash":"bb9ce37e","cli_path":"/home/REDACTED/work/CorpSim/CorpSim/packages/db/node_modules/prisma/build/index.js","cli_path_h(dns block)If you need me to access, download, or install something from one of these locations, you can either:
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.