Skip to content

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Dec 31, 2025

support #6428

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved stderr output handling to gracefully manage non-ASCII characters in error messages
    • Enhanced integer-to-code conversion with better fallback mechanisms for edge cases
    • Fixed exception printing to properly leverage Python's error handling system

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 31, 2025

📝 Walkthrough

Walkthrough

The PR modifies the VM's error and IO handling in three areas: setting "backslashreplace" error handler for stderr in TextIOWrapper, implementing fallback logic for PyInt-to-code conversion to handle overflow cases, and redirecting exception printing through Python's sys.stderr.write() method instead of direct writes.

Changes

Cohort / File(s) Summary
Python IO and Exception Handling
crates/vm/src/vm/mod.rs
Modified TextIOWrapper creation to apply "backslashreplace" error handler for stderr (fd == 2); changed exception printing to use sys.stderr.write() instead of direct PyStderr writes; updated PyInt-to-code conversion with fallback logic (try to_u32, then to_i32 with cast, default to -1 on overflow)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • ShaharNaveh

Poem

🐰 A rabbit hops through stderr streams so fine,
With backslash-replace and write-calls divine,
Integer conversions now gracefully fall,
Python's own write method handles it all! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title mentions 'error codec' and 'exit code', which relate to the error handling changes in the PR, but doesn't clearly highlight the main changes like Python IO redirection, error handler propagation, or stderr output handling.
✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cd012f9 and 8cf8029.

📒 Files selected for processing (1)
  • crates/vm/src/vm/mod.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code
Always run clippy to lint code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

  • crates/vm/src/vm/mod.rs
🧬 Code graph analysis (1)
crates/vm/src/vm/mod.rs (4)
crates/vm/src/stdlib/io.rs (2)
  • errors (836-838)
  • errors (3107-3109)
crates/vm/src/builtins/int.rs (1)
  • as_bigint (264-266)
crates/codegen/src/compile.rs (2)
  • to_u32 (6075-6075)
  • to_u32 (6079-6081)
crates/vm/src/stdlib/sys.rs (1)
  • get_stderr (1552-1556)
🔇 Additional comments (3)
crates/vm/src/vm/mod.rs (3)

330-340: LGTM! Proper stderr error handler configuration.

The code correctly sets "backslashreplace" as the error handler for stderr (fd == 2), ensuring that unencodable characters are escaped rather than causing encoding errors. This aligns with CPython's default stderr behavior and properly integrates with the TextIOWrapper constructor call.


881-885: LGTM! Proper use of Python's stderr write method.

Writing through sys.stderr.write() correctly engages the backslashreplace error handler configured at lines 330-335. The use of let _ = to ignore write errors is appropriate here—if stderr itself fails, there's no alternative output mechanism available in this exception handling context.


862-868: Improved overflow handling for exit codes.

The fallback logic properly handles various integer representations:

  • Positive values via to_u32()
  • Negative values via to_i32() with cast to preserve two's complement
  • Overflow cases default to -1 (represented as 4294967295 in u32)

This prevents panics from the previous unwrap() approach. The overflow default of -1 aligns with observed CPython behavior for very large integers in SystemExit; on POSIX systems, only the low 8 bits are used by shells anyway (making -1 resolve to 255, a semantically reasonable error code).


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@youknowone youknowone marked this pull request as ready for review December 31, 2025 02:05
@youknowone youknowone merged commit 00ae4a1 into RustPython:main Dec 31, 2025
13 checks passed
@youknowone youknowone deleted the sys-impl branch December 31, 2025 02:28
@coderabbitai coderabbitai bot mentioned this pull request Jan 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant