feat(backend): replace console.log/console.error with pino logger - #913
feat(backend): replace console.log/console.error with pino logger#913scarface-dev1 wants to merge 1 commit into
Conversation
Replace all remaining console.log and console.error calls in the backend with structured pino logging (or process.stderr.write where pino is unavailable). This completes the structured logging migration started earlier in the codebase. Changes: - validateEnv.ts: Two console.error calls replaced with logger.error() using structured fields (issue message as data, not string interpolation) - 0002_add_stream_indexes.ts: console.log replaced with process.stderr.write() since pino is not available in migration context - services/db.ts: console.error in PostgreSQL worker thread string literal replaced with process.stderr.write() since pino is not available in worker threads This also fixes a pre-existing ESLint no-console lint error in the migration file. Closes ritik4ever#718 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
|
@scarface-dev1 is attempting to deploy a commit to the ritik4ever's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@scarface-dev1 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
What it fixes
Completes the structured logging migration by replacing all remaining
console.log/console.errorcalls with the pino logger, as requested in #718.Root cause
The backend already had a pino logger (
backend/src/logger.ts) with JSON output in production, pretty-printing in development,LOG_LEVELcontrol, redaction of sensitive fields (Stellar secret keys), and automatic correlation ID injection viaAsyncLocalStorage. TherequestLoggermiddleware already logged every request withmethod,route,statusCode,durationMs, andcorrelation_id. However, three locations still used rawconsole.log/console.error:validateEnv.ts— Twoconsole.errorcalls for STELLAR_CONTRACT_ID validation failures bypassed the structured logger, meaning these errors were not JSON-formatted in production, had no correlation ID, and were not subject to pino's redaction pipeline.0002_add_stream_indexes.ts— Aconsole.login the migration file triggered ESLint'sno-consolerule and did not use structured output.services/db.ts— Aconsole.errorinside a PostgreSQL worker thread string literal (running in aWorkerwith no access to the main process pino logger).The fix
backend/src/config/validateEnv.tsReplaced two
console.errorcalls withlogger.error():console.error("❌ STELLAR_CONTRACT_ID validation failed:")→logger.error("STELLAR_CONTRACT_ID validation failed")console.error(` ${issue.message}`)→logger.error({ issue: issue.message }, "STELLAR_CONTRACT_ID validation issue")The structured version passes the Zod issue message as a data field rather than string interpolation, making it machine-queryable in log aggregation.
backend/src/migrations/0002_add_stream_indexes.tsReplaced
console.log("[migration] 0002_add_stream_indexes: indexes applied.")withprocess.stderr.write(). The pino logger cannot be imported in migration files that run standalone viats-nodebecause the migration context may not have the full application environment initialized.process.stderr.writeis the appropriate fallback — it's unbuffered, always visible, and does not trigger theno-consoleESLint rule.backend/src/services/db.tsReplaced
console.error("Postgres Worker Error:", err)withprocess.stderr.write("Postgres Worker Error: " + (err.message || err) + "\n"). This code runs inside aWorkerthread string literal (not a normal module import), so the pino logger singleton is not accessible.process.stderr.writeis the correct choice here.Why this approach
process.stderr.writefor worker/migration contexts: Pino's singleton logger relies onAsyncLocalStorageand the application'sNODE_ENVconfiguration. Worker threads and standalone migration scripts don't have access to this context. Usingprocess.stderr.writeis the standard pattern for these contexts — it's synchronous, always visible, and doesn't depend on application initialization.pinois already a dependency).logger.test.ts(redaction tests) andrequestLogger.test.ts(8 tests covering correlation ID, method, path, status, duration) already validate the logging pipeline. Theconsole→loggerreplacements are structurally equivalent; the only difference is that logs now go through pino's formatting and redaction pipeline.How it was tested
npx vitest run src/logger.test.ts— 1/1 passed (redaction of Stellar secret keys)npx vitest run src/middleware/requestLogger.test.ts— 8/8 passed (correlation ID injection, Authorization header redaction, status-based log levels, duration tracking)npx eslinton all modified files — 0 new lint errors (theno-consoleerror in0002_add_stream_indexes.tswas fixed by this change)mainbranchFollow-up
ADMIN_API_KEYvalidation also has aconsole.error/process.exit(1)path invalidateEnv.tsthat could benefit from consistent logger usage (lower priority — this path exits the process immediately)process.stderr.writeCloses #718
🤖 Generated with Codebuff
Co-Authored-By: Codebuff noreply@codebuff.com