Skip to content

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Nov 15, 2025

Summary by CodeRabbit

  • Bug Fixes
    • Improved error message formatting to properly handle cases where filename information is unavailable, ensuring clean and correct error output in all scenarios.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 15, 2025

Walkthrough

Added a None-check guard in PyOSError.str to safely handle cases where the filename is None. When filename is None, the method now returns a simplified error message format instead of attempting to access filename2, preventing potential access errors.

Changes

Cohort / File(s) Summary
OSError formatting fix
vm/src/exceptions.rs
Added None-guard for filename in PyOSError.str to prevent accessing filename2 when filename is None; falls back to simpler error message format

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Specific area of focus: Verify the fallback error message format matches expected OSError behavior in Python
  • Testing consideration: Ensure both code paths (with and without filename) produce correct error strings

Poem

🐰 A filename that vanishes in the night,
No longer crashes with a fright!
A guard so small, a fix so neat,
Makes error handling complete! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: fixing OSError.str to prevent displaying 'None' for filename, which directly aligns with the file modification in vm/src/exceptions.rs.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 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 2071fa2 and 556682d.

📒 Files selected for processing (1)
  • vm/src/exceptions.rs (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
vm/src/exceptions.rs (1)
vm/src/ospath.rs (3)
  • filename (79-81)
  • filename (126-131)
  • filename (150-154)
🔇 Additional comments (1)
vm/src/exceptions.rs (1)

1514-1544: OSError behavior mismatch with CPython identified

The verification reveals a significant incompatibility: CPython displays [Errno 2] No such file or directory: None when filename is explicitly None, but the implemented fix prevents this output entirely (the guard on line 1525 skips filename formatting when it's None).

This causes RustPython to output [Errno 2] No such file or directory instead, deviating from CPython's behavior.

Before approving, clarify:

  • Is this intentional deviation from CPython acceptable?
  • What was the actual issue the fix was addressing (e.g., "None" appearing when filename wasn't explicitly set)?

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 November 15, 2025 03:24
@youknowone youknowone merged commit 477c9b3 into RustPython:main Nov 15, 2025
13 checks passed
@youknowone youknowone deleted the os-error-filename2 branch November 15, 2025 03:29
@coderabbitai coderabbitai bot mentioned this pull request Dec 17, 2025
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