refactor(logging): replace console calls with Winston logger in server files#289
refactor(logging): replace console calls with Winston logger in server files#289Muneerali199 wants to merge 1 commit intoAOSSIE-Org:mainfrom
Conversation
- Replace all console.log/warn/error in InputHandler.ts, ydotool.ts, tokenStore.ts with the centralized Winston logger - Remove duplicate console.error next to logger.error in websocket.ts - Pass Error objects directly to logger.error() to preserve stack traces - Redact raw key/combo content from debug logs to prevent keystroke leakage - Include caught error in ydotool warn call for better diagnostics - Downgrade high-frequency upgrade-request log from info to debug - Remove unused console interception block from logger.ts Closes AOSSIE-Org#213
WalkthroughThis change refactors server-side logging by systematically replacing console-based logging (console.log, console.error, console.warn) with a centralized logger module across five server files. Additionally, the console override shim in the logger utility is removed, shifting from runtime console interception to explicit logger usage at call sites. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~13 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server/InputHandler.ts (1)
263-277:⚠️ Potential issue | 🟡 MinorMake the shared logger level configurable.
The logger in
src/utils/logger.tshardcodeslevel: "info", which suppresses the newlogger.debug()calls at lines 263, 277, 317, and 340. These debugging breadcrumbs are unavailable in every environment unless the logger level is changed.Suggested fix in
src/utils/logger.ts- level: "info", + level: process.env.LOG_LEVEL ?? (process.env.NODE_ENV === "production" ? "info" : "debug"),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/server/InputHandler.ts` around lines 263 - 277, The shared logger is hardcoded to level "info" so the new logger.debug() calls (e.g., in InputHandler using logger.debug) are never emitted; update the logger factory (the exported logger object created in utils/logger.ts, e.g., createLogger or default exported logger) to accept/configure its level from an environment/config variable (for example process.env.LOG_LEVEL) or a passed option and default to "info", then use that configurable level when instantiating/creating the logger so debug messages from InputHandler and other modules are emitted when the level is set to "debug".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/server/InputHandler.ts`:
- Around line 263-277: The shared logger is hardcoded to level "info" so the new
logger.debug() calls (e.g., in InputHandler using logger.debug) are never
emitted; update the logger factory (the exported logger object created in
utils/logger.ts, e.g., createLogger or default exported logger) to
accept/configure its level from an environment/config variable (for example
process.env.LOG_LEVEL) or a passed option and default to "info", then use that
configurable level when instantiating/creating the logger so debug messages from
InputHandler and other modules are emitted when the level is set to "debug".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 21c3ed6a-2261-4a76-a621-c49e42757ef7
📒 Files selected for processing (5)
src/server/InputHandler.tssrc/server/tokenStore.tssrc/server/websocket.tssrc/server/ydotool.tssrc/utils/logger.ts
💤 Files with no reviewable changes (1)
- src/utils/logger.ts
Closes #213
What changed
Replaced all raw
console.log/warn/errorcalls in the server-side files with the existing Winstonloggerfromsrc/utils/logger.ts.Files modified:
src/server/InputHandler.ts— added logger import, replaced ~12 console calls, redacted raw key content from debug logs, fixed error objects passed directly instead of interpolated as strings, fixed dead catch block in scroll handler to actually log rejectionssrc/server/ydotool.ts— replaced 3 console calls, included the caughterrin thelogger.warncall so it isn't swallowedsrc/server/tokenStore.ts— replacedconsole.errorwithlogger.error, passing the Error object directlysrc/server/websocket.ts— removed duplicateconsole.errornext tologger.error, fixed error interpolation on 3 calls, downgraded high-frequency "Upgrade request received" log frominfotodebugsrc/utils/logger.ts— removed unused_originalConsoleLog/_originalConsoleErrorvariables and the dead console interception blockWhat was not changed
electron/main.cjs— CJS module, cannot import the TypeScript loggersrc/routes/settings.tsx— frontend file, out of scope for this issueSecurity note
Raw keystroke content (
msg.key) is not logged at any level. Debug messages use redacted placeholders to avoid leaking passwords or sensitive input character-by-character into log files.Summary by CodeRabbit