fix: route CLI error tracebacks to stderr instead of stdout #255
Conversation
WalkthroughCreated a new module-level Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI entry
participant ConsoleOut as console (stdout)
participant ErrConsole as err_console (stderr)
participant RichTB as Rich traceback handler
CLI->>ConsoleOut: normal output (prints, progress)
CLI->>RichTB: install_rich_traceback(target=ErrConsole)
Note over RichTB,ErrConsole: RichTB configured to send tracebacks to ErrConsole
CLI->>ErrConsole: exception occurs -> RichTB renders traceback to ErrConsole
CLI->>ErrConsole: prints error banner and GitHub guidance
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/mcpm/cli.py (2)
56-61: Send tracebacks and error messages to the stderr console.With a dedicated stderr console, print exceptions and error text there.
- console.print(Traceback(show_locals=True)) - console.print("[bold red]An unexpected error occurred.[/bold red]") - console.print( + err_console.print(Traceback(show_locals=True)) + err_console.print("[bold red]An unexpected error occurred.[/bold red]") + err_console.print( "Please report this issue on our GitHub repository: " "[link=https://github.com/pathintegral-institute/mcpm.sh/issues]https://github.com/pathintegral-institute/mcpm.sh/issues[/link]" )
52-63: Exit non‑zero on unhandled exceptions.The decorator swallows exceptions and exits 0, which breaks scripting/automation.
Within the except block:
err_console.print( "Please report this issue on our GitHub repository: " "[link=https://github.com/pathintegral-institute/mcpm.sh/issues]https://github.com/pathintegral-institute/mcpm.sh/issues[/link]" ) + sys.exit(1)Add at top:
-from typing import Any, Dict +from typing import Any, Dict +import sysWant me to open a quick follow-up PR for this?
🧹 Nitpick comments (1)
src/mcpm/cli.py (1)
5-5: Comment/import order mismatch.The comment says to import rich-click config “before anything else,” but it’s imported later (Line 32). Either move that import up or adjust the comment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/mcpm/cli.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Always format Python code with
ruff.
Files:
src/mcpm/cli.py
Create separate Console instances for regular CLI output (stdout) and error handling (stderr). This ensures: - Logo, help, and normal UI go to stdout (console) - Error tracebacks and error messages go to stderr (err_console) Follows Unix conventions for proper stream separation without mixing normal program output with error diagnostics. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
pyq
force-pushed
the
fix/console-stderr-routing
branch
from
e7f7efc to
7179309
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/mcpm/cli.py (1)
57-63: Unhandled exceptions currently return exit code 0 — re-raise with non-zero.Catching Exception and not re-raising exits successfully, breaking scripts that rely on non-zero on failure.
Apply:
except Exception: err_console.print(Traceback(show_locals=True)) err_console.print("[bold red]An unexpected error occurred.[/bold red]") err_console.print( "Please report this issue on our GitHub repository: " "[link=https://github.com/pathintegral-institute/mcpm.sh/issues]https://github.com/pathintegral-institute/mcpm.sh/issues[/link]" ) + raise SystemExit(1)
♻️ Duplicate comments (1)
src/mcpm/cli.py (1)
34-35: Good stdout/stderr split; prior concern addressed.Using separate Console instances for stdout (console) and stderr (err_console) is the correct fix and avoids polluting stderr with normal UI.
🧹 Nitpick comments (2)
src/mcpm/cli.py (2)
57-59: Avoid leaking locals in tracebacks; gate under debug flag.Printing locals can expose tokens/paths. Gate it behind an env flag.
Within wrapper:
- err_console.print(Traceback(show_locals=True)) + err_console.print(Traceback(show_locals=bool(os.getenv("MCPM_DEBUG"))))Add import near the top:
import os
57-61: Add a small integration test to assert stderr routing.Use Click’s CliRunner to ensure traceback text lands in stderr, not stdout.
I can open a follow-up PR with a test like:
from click.testing import CliRunner from src.mcpm.cli import main def test_errors_to_stderr(monkeypatch): @main.command() def boom(): raise RuntimeError("boom") runner = CliRunner(mix_stderr=False) result = runner.invoke(main, ["boom"]) assert result.exit_code != 0 assert "Traceback" in result.stderr assert "Traceback" not in result.output
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/mcpm/cli.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Always format Python code with
ruff.
Files:
src/mcpm/cli.py
…rting Added installation of Rich's global exception handler to route tracebacks to stderr instead of stdout. This change enhances error diagnostics by preventing mixing of normal output with error messages.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/mcpm/cli.py (1)
62-69: Return non-zero on errors and also gate locals here.
- CLI currently swallows exceptions and exits 0, which breaks scripting/CI.
- Mirror the debug-guard for locals to prevent accidental data exposure.
Apply:
- err_console.print(Traceback(show_locals=True)) + err_console.print(Traceback(show_locals=SHOW_LOCALS)) err_console.print("[bold red]An unexpected error occurred.[/bold red]") err_console.print( "Please report this issue on our GitHub repository: " "[link=https://github.com/pathintegral-institute/mcpm.sh/issues]https://github.com/pathintegral-institute/mcpm.sh/issues[/link]" ) + # Ensure a failing exit code for scripting/CI + raise SystemExit(1)
♻️ Duplicate comments (1)
src/mcpm/cli.py (1)
35-36: Good split between stdout and stderr consoles.This addresses the earlier concern about mixing normal UI with error output. LGTM.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/mcpm/cli.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Always format Python code with
ruff.
Files:
src/mcpm/cli.py
🧬 Code graph analysis (1)
src/mcpm/cli.py (2)
src/mcpm/commands/install.py (1)
install(123-448)src/mcpm/utils/logging_config.py (1)
setup_logging(12-44)
🔇 Additional comments (1)
src/mcpm/cli.py (1)
10-10: Import alias for Rich traceback installer is appropriate.Keeps call sites explicit and readable. LGTM.
|
🎉 This PR is included in version 2.8.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
User description
The console object in
cli.pywas created withoutstderr=True, causing unhandled exception tracebacks to be printed tostdoutinstead ofstderr. This fix ensures proper error stream routing by configuring the console to usestderr, matching the behavior of the logging system.PR Type
Bug fix
Description
Configure CLI console to route error tracebacks to stderr
Fix improper error stream routing in command-line interface
Diagram Walkthrough
File Walkthrough
cli.py
Configure console for stderr error routingsrc/mcpm/cli.py
stderr=TrueparameterSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
Tests
Chores
Revert