-
Notifications
You must be signed in to change notification settings - Fork 30
feat: standardize backend logging, remove direct console usage #75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,7 @@ const execAsync = promisify(exec); | |
| const migrationName = process.argv[2]; | ||
|
|
||
| if (!migrationName) { | ||
| console.error('Please provide a migration name.'); | ||
| logger.error('Please provide a migration name.'); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win This breaks the script’s terminal feedback. These messages are part of the CLI contract, but the shared logger shown in Also applies to: 23-23 🧰 Tools🪛 ast-grep (0.44.0)[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec. (detect-child-process-typescript) 🤖 Prompt for AI Agents |
||
| process.exit(1); | ||
| } | ||
|
|
||
|
|
@@ -20,7 +20,7 @@ const command = `pnpm typeorm migration:generate --outputJs src/migrations/${mig | |
| const { stdout, stderr } = await execAsync(command); | ||
|
|
||
| if (stdout.includes('No changes in database schema were found')) { | ||
| console.warn( | ||
| logger.warn( | ||
| 'No schema changes detected. Skipping migration generation.' | ||
| ); | ||
| return; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,4 +22,62 @@ if (appConfigs.isDev || appConfigs.isTestMode || appConfigs.isLocalDev) { | |
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Log an informational message with optional context metadata. | ||
| * | ||
| * @param message - Human-readable message | ||
| * @param context - Optional key/value metadata (service name, IDs, etc.) | ||
| */ | ||
| export const logInfo = ( | ||
| message: string, | ||
| context?: Record<string, unknown> | ||
| ): void => { | ||
| logger.info(message, context); | ||
| }; | ||
|
|
||
| /** | ||
| * Log a warning with optional context metadata. | ||
| */ | ||
| export const logWarn = ( | ||
| message: string, | ||
| context?: Record<string, unknown> | ||
| ): void => { | ||
| logger.warn(message, context); | ||
| }; | ||
|
|
||
| /** | ||
| * Log an error. Accepts an Error instance, a plain message, or both. | ||
| * | ||
| * @param message - Human-readable description of what went wrong | ||
| * @param error - The caught error/exception (optional) | ||
| * @param context - Additional key/value metadata (optional) | ||
| */ | ||
| export const logError = ( | ||
| message: string, | ||
| error?: unknown, | ||
| context?: Record<string, unknown> | ||
| ): void => { | ||
| const meta: Record<string, unknown> = { ...context }; | ||
|
|
||
| if (error instanceof Error) { | ||
| meta.errorName = error.name; | ||
| meta.errorMessage = error.message; | ||
| meta.stack = error.stack; | ||
| } else if (error !== undefined) { | ||
| meta.error = error; | ||
| } | ||
|
|
||
| logger.error(message, meta); | ||
| }; | ||
|
|
||
| /** | ||
| * Log a debug message (only emitted when log level is set to 'debug'). | ||
| */ | ||
| export const logDebug = ( | ||
| message: string, | ||
| context?: Record<string, unknown> | ||
| ): void => { | ||
| logger.debug(message, context); | ||
|
Comment on lines
+76
to
+80
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Line 6 hard-codes the logger level to 🤖 Prompt for AI Agents |
||
| }; | ||
|
|
||
| export default logger; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Don’t stringify the reset failure here.
String(error)drops the structured fields this PR just standardized (name,message,stack), which makes DB reset failures much harder to diagnose. Route this throughlogError("Error resetting the database", error)or pass equivalent structured metadata.🤖 Prompt for AI Agents