Skip to content

WindowsError#6333

Merged
youknowone merged 1 commit intoRustPython:mainfrom
youknowone:windows-error
Dec 6, 2025
Merged

WindowsError#6333
youknowone merged 1 commit intoRustPython:mainfrom
youknowone:windows-error

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Dec 6, 2025 •
edited by coderabbitai bot
Loading

Summary by CodeRabbit

  • Bug Fixes

    • OSError string output on Windows now shows Windows error codes as "[WinError {code}]" when appropriate, matching CPython formatting.

  • New Features

    • A Windows-specific WindowsError alias for OSError is now available on Windows.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 6, 2025 •
edited
Loading

Walkthrough

Windows-specific updates: OSError initialization now converts a provided winerror to an errno and replaces the first arg on Windows; OSError stringification uses "WinError" when winerror is set and "Errno" otherwise; WindowsError is exported as an alias to OSError only on Windows builds. (50 words)

Changes

Cohort / File(s) Summary
Windows OSError Exception Handling
crates/vm/src/exceptions.rs		 On Windows, slot_init converts a provided winerror to an errno and replaces args[0] with that errno. __str__ now chooses label/code as ("WinError", code) when winerror is set, otherwise ("Errno", errno), and formats output as "[{label} {code}] ...".
Windows Error Alias Export
crates/vm/src/stdlib/builtins.rs		 Added WindowsError as an alias to OSError in module initialization under #[cfg(windows)], exposing it only on Windows builds.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Review winerror → errno conversion logic in crates/vm/src/exceptions.rs for parity with CPython.
  • Verify __str__ formatting covers all argument/path variants and preserves existing non-Windows behavior.
  • Confirm #[cfg(windows)] gating in crates/vm/src/stdlib/builtins.rs prevents exposure on non-Windows builds.

Poem

🐰 I hopped through errors, found their lore,
Turned WinError keys to errno's door,
Now strings proclaim what codes convey,
And WindowsError joins the play.
Rabbit cheers — bugs led astray! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'WindowsError' is too vague and generic. While it relates to the changeset (adding WindowsError alias and Windows-specific error handling), it doesn't meaningfully summarize what the change accomplishes or why it matters. Consider a more descriptive title that explains the purpose, such as 'Add WindowsError alias to builtins and improve Windows OSError handling' or 'Align Windows OSError behavior with CPython'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
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 74c07df and 4f752dd.

⛔ Files ignored due to path filters (1)
  • Lib/test/test_exception_hierarchy.py is excluded by !Lib/**
📒 Files selected for processing (2)
  • crates/vm/src/exceptions.rs (2 hunks)
  • crates/vm/src/stdlib/builtins.rs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/vm/src/exceptions.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 Rust 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/stdlib/builtins.rs
🧬 Code graph analysis (1)
crates/vm/src/stdlib/builtins.rs (1)
crates/vm/src/stdlib/os.rs (1)
  • extend_module (1573-1597)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: Run snippets and cpython tests (windows-latest)
  • GitHub Check: Run snippets and cpython tests on wasm-wasi
  • GitHub Check: Run snippets and cpython tests (macos-latest)
  • GitHub Check: Check the WASM package and demo
  • GitHub Check: Run tests under miri
  • GitHub Check: Check Rust code with clippy
  • GitHub Check: Run snippets and cpython tests (ubuntu-latest)
  • GitHub Check: Run rust tests (windows-latest)
  • GitHub Check: Ensure compilation on various targets
  • GitHub Check: Run rust tests (macos-latest)
  • GitHub Check: Run rust tests (ubuntu-latest)
🔇 Additional comments (1)
crates/vm/src/stdlib/builtins.rs (1)

1191-1195: Windows-only WindowsError alias correctly mirrors CPython behavior

Defining WindowsError as a Windows-only alias to OSError via ctx.exceptions.os_error is consistent with CPython's builtins and fits well alongside the other OSError aliases. Before merging, ensure cargo fmt and cargo clippy pass on this file per the Rust coding guidelines.

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 6, 2025 02:37
@youknowone youknowone merged commit aea956d into RustPython:main Dec 6, 2025
13 checks passed
@youknowone youknowone deleted the windows-error branch December 6, 2025 08:21
This was referenced Dec 12, 2025
Merged
Merged
@coderabbitai coderabbitai bot mentioned this pull request Feb 8, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@youknowone