Allow #[pyexception] on module attr #6218

youknowone · 2025-10-26T14:44:02Z

Summary by CodeRabbit

New Features
- Introduced specialized SSL exception types (SSLCertVerificationError, SSLZeroReturnError, SSLWantReadError, SSLWantWriteError, SSLSyscallError, SSLEOFError) for improved error handling and compatibility with Python's ssl module.
Refactor
- Enhanced internal exception framework to support custom exception hierarchies with improved attribute handling.

coderabbitai · 2025-10-26T14:44:19Z

Walkthrough

Introduces support for a new #[pyexception] attribute in the derive-impl framework to define exception types, refactors the SSL module to use a structured exception hierarchy with PySslError as the base, makes exception slot methods public, and extends exception metadata validation to support module attributes.

Changes

Cohort / File(s)	Summary
Derive infrastructure for pyexception support `derive-impl/src/pymodule.rs`, `derive-impl/src/util.rs`	Adds Exception variant to attribute name enum, wires it through Display and FromStr conversions, treats pyexception like pyclass in module item creation, includes Exception in validation flow, and adds "module" to ALLOWED_NAMES for ExceptionItemMeta.
Exception slot method visibility `derive-impl/src/pyclass.rs`, `vm/src/exceptions.rs`	Changes `slot_new` and `slot_init` from private/`pub(crate)` to `pub` visibility in PyException and PyOSError implementations; adds new PyPayload implementation path for PyException when no ctx_name is provided.
SSL module exception hierarchy refactoring `stdlib/src/ssl.rs`	Replaces ad-hoc SSL exception creation with a structured hierarchy: introduces PySslError (base, derives from PyOSError) and six subclasses (PySslCertVerificationError, PySslZeroReturnError, PySslWantReadError, PySslWantWriteError, PySslSyscallError, PySslEOFError), all marked as pyexceptions; updates all error construction sites to use the new exception classes and adds __str__ implementation for PySslError.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

stdlib/src/ssl.rs: Requires careful review of the exception hierarchy refactoring, all error construction points, and the __str__ implementation for correctness and consistency across the module. Verify that all old ssl_error(vm) calls have been properly replaced.
derive-impl/src/pymodule.rs and derive-impl/src/util.rs: Review the integration of Exception variant into the attribute parsing and validation pipeline to ensure it follows the same pattern as Class and Function.
Visibility changes: Verify that making slot methods public in derive-impl and vm/src/exceptions.rs does not expose internal implementation details unintentionally.

Possibly related PRs

Impl PyAttributeError args #5805: Modifies exception slot implementations in vm/src/exceptions.rs, including slot_init and slot_new visibility and exception slot implementations, directly overlapping with exception infrastructure changes in this PR.

Poem

🐰 Exceptions now have a home, a hierarchy so grand,
From PySslError's base, the subclasses expand,
The #[pyexception] magic makes definitions neat,
Public slots and module attributes, the feature's 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 PR title "Allow #[pyexception] on module attr" directly describes the primary objective of the changeset. The modifications across multiple files demonstrate this core feature: derive-impl/src/pymodule.rs introduces an Exception variant to support pyexception attributes in modules, derive-impl/src/util.rs adds "module" to the allowed attribute names for ExceptionItemMeta, and stdlib/src/ssl.rs exemplifies the new capability with public pyexception definitions. The title is specific, concise, and clearly communicates the main change without being vague or misleading.
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.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (2)

stdlib/src/ssl.rs (2)

206-226: New SSL exception hierarchy (SSLError + subclasses) — well structured.

Base SSLError derives from PyOSError (CPython parity).
str prefers strerror then args — sensible default.

Minor: you could fallback to exc.str(vm) instead of str(args) for perfect parity, but current code is fine.

Also applies to: 230-236, 239-245, 248-254, 257-263, 266-272, 275-281

1982-2012: convert_openssl_error/convert_ssl_error are solid; add ZERO_RETURN mapping.

Map ssl::ErrorCode::ZERO_RETURN to SSLZeroReturnError for better CPython parity (clean shutdown). Suggested patch:

@@ fn convert_ssl_error(vm: &VirtualMachine, e: impl std::borrow::Borrow<ssl::Error>) -> PyBaseExceptionRef {
-        let (cls, msg) = match e.code() {
+        let (cls, msg) = match e.code() {
             ssl::ErrorCode::WANT_READ => (
                 PySslWantReadError::class(&vm.ctx).to_owned(),
                 "The operation did not complete (read)",
             ),
             ssl::ErrorCode::WANT_WRITE => (
                 PySslWantWriteError::class(&vm.ctx).to_owned(),
                 "The operation did not complete (write)",
             ),
+            ssl::ErrorCode::ZERO_RETURN => (
+                PySslZeroReturnError::class(&vm.ctx).to_owned(),
+                "TLS/SSL connection has been closed (EOF)",
+            ),
             ssl::ErrorCode::SYSCALL => match e.io_error() {
                 Some(io_err) => return io_err.to_pyexception(vm),
                 None => (
                     PySslSyscallError::class(&vm.ctx).to_owned(),
                     "EOF occurred in violation of protocol",
                 ),
             },
             ssl::ErrorCode::SSL => match e.ssl_error() {
                 Some(e) => return convert_openssl_error(vm, e.clone()),
                 None => (
                     PySslError::class(&vm.ctx).to_owned(),
                     "A failure in the SSL library occurred",
                 ),
             },
             _ => (
                 PySslError::class(&vm.ctx).to_owned(),
                 "A failure in the SSL library occurred",
             ),
         };

Run your SSL tests that exercise graceful close to verify behavior change.

Also applies to: 2034-2068

📜 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 2e7a8b4 and ded211d.

📒 Files selected for processing (5)

derive-impl/src/pyclass.rs (3 hunks)
derive-impl/src/pymodule.rs (5 hunks)
derive-impl/src/util.rs (1 hunks)
stdlib/src/ssl.rs (11 hunks)
vm/src/exceptions.rs (2 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.rs

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

**/*.rs: Format Rust code with the default rustfmt style (run cargo fmt)
Run clippy and fix any warnings or lints introduced by your changes
Follow Rust best practices for error handling and memory management

Files:

derive-impl/src/util.rs
derive-impl/src/pymodule.rs
derive-impl/src/pyclass.rs
stdlib/src/ssl.rs
vm/src/exceptions.rs

{vm,stdlib}/**/*.rs

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

Use RustPython macros (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

stdlib/src/ssl.rs
vm/src/exceptions.rs

🧬 Code graph analysis (4)

derive-impl/src/pymodule.rs (1)

derive-impl/src/util.rs (1)

attr_name (106-108)

derive-impl/src/pyclass.rs (2)

vm/src/exceptions.rs (12)

class (43-45)

slot_new (1453-1465)

slot_new (1468-1470)

slot_init (1272-1279)

slot_init (1314-1330)

slot_init (1349-1369)

slot_init (1473-1489)

slot_init (1655-1687)

slot_init (1749-1756)

slot_init (1791-1805)

slot_init (1842-1855)

slot_init (1893-1905)

vm/src/class.rs (1)

static_type (25-33)

stdlib/src/ssl.rs (3)

derive/src/lib.rs (1)

pyexception (139-143)

derive-impl/src/lib.rs (1)

pyexception (50-56)

vm/src/exceptions.rs (10)

__str__ (637-644)

__str__ (1405-1415)

__str__ (1492-1521)

__str__ (1690-1734)

__str__ (1808-1831)

__str__ (1858-1882)

__str__ (1908-1928)

class (43-45)

args (567-569)

args (573-573)

vm/src/exceptions.rs (1)

vm/src/types/slot.rs (4)

slot_new (787-790)

slot_new (810-812)

slot_new (821-823)

slot_init (836-840)

⏰ 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). (8)

GitHub Check: Run snippets and cpython tests (macos-latest)
GitHub Check: Run snippets and cpython tests (ubuntu-latest)
GitHub Check: Run snippets and cpython tests (windows-latest)
GitHub Check: Run rust tests (ubuntu-latest)
GitHub Check: Check Rust code with rustfmt and clippy
GitHub Check: Run rust tests (windows-latest)
GitHub Check: Run rust tests (macos-latest)
GitHub Check: Ensure compilation on various targets

🔇 Additional comments (10)

derive-impl/src/util.rs (1)

449-451: ALLOWED_NAMES now includes "module" — good alignment with pyclass metadata.

This enables #[pyexception(module = ...)] parsing consistently. No issues.

derive-impl/src/pymodule.rs (3)

19-20: Add AttrName::Exception with Display/FromStr — looks correct.

Enum wiring is complete and error strings remain coherent.

Also applies to: 28-29, 37-43

239-243: Treat pyexception like pyclass in module items.

Correct: both define types. This keeps generation paths uniform.

309-316: Allow #[pyexception] to follow #[pyattr].

The diagnostic string is updated accordingly. Good UX improvement.

derive-impl/src/pyclass.rs (3)

554-560: PyException PyPayload fallback via StaticType — good default.

Removes ctx dependency for external exceptions. Ensure new_class initialization runs before first class() call, otherwise StaticType::static_type() panics. Please verify module init order.

622-629: Make generated slot_new public.

Necessary for cross-crate calls like ::Base::slot_new when Base lives in vm. Good.

643-650: Make generated slot_init public.

Same rationale as slot_new. Looks good.

vm/src/exceptions.rs (1)

1452-1469: Expose PyOSError slot_new/slot_init as public.

Enables derive-generated calls from other crates. Safe and consistent with the new macro behavior.

Also applies to: 1473-1489

stdlib/src/ssl.rs (2)

41-44: Import AsObject/PyOSError — required for new exception plumbing.

Imports are correct and used.

642-647: Consistent use of PySslError for module errors.

Raised errors now consistently use the new SSL classes. Good consistency upgrade.

Also applies to: 771-782, 963-972, 1418-1421, 1451-1454, 1883-1886

Expand #[pyexception] macro working with module attr

c8ff4b9

pyssl errors

ded211d

youknowone force-pushed the pyexcetion branch from 810fc12 to ded211d Compare October 27, 2025 00:37

youknowone marked this pull request as ready for review October 27, 2025 00:37

youknowone requested review from ShaharNaveh and coolreader18 October 27, 2025 00:38

coderabbitai bot reviewed Oct 27, 2025

View reviewed changes

ShaharNaveh approved these changes Oct 27, 2025

View reviewed changes

youknowone merged commit 517b55b into RustPython:main Oct 27, 2025
12 checks passed

youknowone deleted the pyexcetion branch October 27, 2025 09:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Allow #[pyexception] on module attr #6218

Allow #[pyexception] on module attr #6218

Uh oh!

youknowone commented Oct 26, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Oct 26, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Allow #[pyexception] on module attr #6218

Allow #[pyexception] on module attr #6218

Uh oh!

Conversation

youknowone commented Oct 26, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Oct 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

youknowone commented Oct 26, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Oct 26, 2025 •

edited

Loading