fix: remove console.info statements that pollute stdout#541
fix: remove console.info statements that pollute stdout#541
Conversation
📝 WalkthroughWalkthroughESLint configuration was updated to permit Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
Linking to #231 - rather than removing them completely, I like the idea of a verbose mode (and do we need to check debug, etc level logs too?). What do you think? |
console.info writes to stdout, which pollutes output for agents and tools consuming the SDK. console.debug writes to stderr instead. Also update eslint no-console rule to disallow console.info going forward. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1c6a38d to
454e97f
Compare
|
This is a first step to address stdout pollution — switching As a follow-up, we should plan how proper log-level support could work for this library (e.g. a |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.eslintrc.json:
- Line 24: The ESLint rule currently allows console.debug via "no-console":
["error", { "allow": ["debug", "warn", "error"] }], but console.debug still
writes to stdout in Node and violates the "no stdout pollution" goal; update the
lint config to remove "debug" from the allowed list and instead route
debug-level logging through a dedicated logger interface (e.g., replace usages
of console.debug with a Logger.debug call or a noop default logger), so keep
"warn" and "error" allowed but ensure all debug output uses the new logger
rather than console.debug.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 561eae9a-7025-416e-8c0c-27bdd6ebd274
📒 Files selected for processing (5)
.eslintrc.jsonsrc/nwc/NWAClient.tssrc/nwc/NWCClient.tssrc/nwc/NWCWalletService.tssrc/webln/OauthWeblnProvider.ts
| ], | ||
| "@typescript-eslint/no-unused-vars": ["warn", { "args": "none" }], | ||
| "no-console": ["error", { "allow": ["info", "warn", "error"] }], | ||
| "no-console": ["error", { "allow": ["debug", "warn", "error"] }], |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In current Node.js documentation, does console.debug()write to stdout or stderr, and is it an alias ofconsole.log()?
💡 Result:
In the current Node.js docs, console.debug() is an alias of console.log() [1]. Since console.log() prints to stdout, console.debug() also writes to stdout (not stderr) [2].
Sources: [1] [2]
console.debug writes to stdout and will still violate the "no stdout pollution" goal.
Allowing console.debug in this config still emits to stdout in Node.js (it's an alias of console.log), which keeps the original parsing risk in place.
Suggested fix
- "no-console": ["error", { "allow": ["debug", "warn", "error"] }],
+ "no-console": ["error", { "allow": ["warn", "error"] }],Route debug logs through an explicit logger interface (noop by default, opt-in by consumers).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "no-console": ["error", { "allow": ["debug", "warn", "error"] }], | |
| "no-console": ["error", { "allow": ["warn", "error"] }], |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.eslintrc.json at line 24, The ESLint rule currently allows console.debug
via "no-console": ["error", { "allow": ["debug", "warn", "error"] }], but
console.debug still writes to stdout in Node and violates the "no stdout
pollution" goal; update the lint config to remove "debug" from the allowed list
and instead route debug-level logging through a dedicated logger interface
(e.g., replace usages of console.debug with a Logger.debug call or a noop
default logger), so keep "warn" and "error" allowed but ensure all debug output
uses the new logger rather than console.debug.
Summary
console.infocalls across NWAClient, NWCClient, NWCWalletService, and OauthWeblnProviderTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit