new_last_{os,errno}_error by youknowone · Pull Request #6381 · RustPython/RustPython

youknowone

Walkthrough

This PR systematically refactors error handling across the codebase by introducing new errno/OS error construction helpers (errno_io_error, get_errno) and VM methods (new_last_os_error, new_last_errno_error), then replaces legacy error functions with these new APIs throughout the common and stdlib modules.

Changes

Cohort / File(s)	Summary
Common module errno/OS error helpers `crates/common/src/os.rs`, `crates/common/src/crt_fd.rs`, `crates/common/src/fileutils.rs`	Introduced `errno_io_error()` and `get_errno()` public functions on Windows and non-Windows. Replaced `last_os_error()` and `last_posix_errno()` implementations with errno-based construction. Updated error paths in `cvt`, `as_handle`, and `fstat` to use `errno_io_error()`.
VM error constructor methods `crates/vm/src/vm/vm_new.rs`	Added public API methods `new_last_os_error()` and `new_last_errno_error()` to `VirtualMachine` for creating OSError exceptions from OS/errno errors. Updated imports to include `ToPyException`.
OS module refactoring `crates/vm/src/stdlib/os.rs`	Removed `errno_err()` helper function and `PyBaseExceptionRef` from prelude. Updated `IntoPyException` trait implementations to return `crate::builtins::PyBaseExceptionRef`. Replaced error construction in multiple code paths (mkdir, mkdirat, lseek, copy_file_range) with new error constructors.
Stdlib error handling updates `crates/stdlib/src/fcntl.rs`, `crates/stdlib/src/multiprocessing.rs`, `crates/stdlib/src/overlapped.rs`, `crates/stdlib/src/resource.rs`, `crates/stdlib/src/socket.rs`	Replaced `errno_err(vm)` with `vm.new_last_errno_error()` or `vm.new_last_os_error()` across various error paths. Removed or narrowed imports of `stdlib::os`. Updated Windows socket and I/O completion port error handling.
VM stdlib error handling updates `crates/vm/src/stdlib/io.rs`, `crates/vm/src/stdlib/msvcrt.rs`, `crates/vm/src/stdlib/nt.rs`, `crates/vm/src/stdlib/posix.rs`, `crates/vm/src/stdlib/signal.rs`, `crates/vm/src/stdlib/time.rs`, `crates/vm/src/stdlib/winapi.rs`, `crates/vm/src/windows.rs`	Systematically replaced `errno_err(vm)` with `vm.new_last_errno_error()` or `vm.new_last_os_error()` across numerous error paths (getrlimit, siginterrupt, performance counter queries, file operations, process/mutex/event APIs). Removed associated imports. Inverted success condition logic in `WindowsSysResult::into_pyresult`.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Areas requiring extra attention:
- crates/vm/src/vm/vm_new.rs: Verify new new_last_os_error() and new_last_errno_error() implementations correctly delegate to errno_io_error() and last_os_error() with proper platform-specific behavior.
- crates/common/src/os.rs: Confirm errno_io_error() and get_errno() implementations correctly use errno_to_winerror() on Windows and std::io::Error::last_os_error() on non-Windows.
- crates/vm/src/stdlib/os.rs: Verify removal of errno_err() helper and its replacement sites; confirm updated IntoPyException return types are consistent.
- Spot-check 2–3 stdlib modules (e.g., nt.rs, posix.rs) to ensure error replacement sites correctly map to the new VM methods.

Possibly related PRs

Convert new_X_error to use a macro #5814: Refactors error-constructor methods in crates/vm/src/vm/vm_new.rs using a macro, overlapping with this PR's additions of new_last_os_error() and new_last_errno_error().
nt is_dir,is_file,listmount,listvolume #6373: Refactors errno/OS-error helpers that directly impact Windows error-construction calls in nt.rs and other modules affected by this PR.
Rework crt_fd to be more aligned with io-safety #5789: Modifies common/src/crt_fd.rs error paths (cvt, as_handle) that overlap with this PR's errno/OS error refactoring.

Suggested reviewers

ShaharNaveh
coolreader18

Poem

🐰 Errors once scattered, now consolidated tight,
errno meets OS, both handled just right,
new_last_errno_error hops through the code,
Legacy helpers now rest on their road,
A refactor complete, error paths unified and bright! 🌙

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 'new_last_{os,errno}_error' directly references the two new public APIs being introduced (new_last_os_error and new_last_errno_error) and accurately captures the primary change across the entire changeset.
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

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.}