Fix install ujson by youknowone · Pull Request #6502 · RustPython/RustPython
📝 Walkthrough
Walkthrough
The changes enhance SSL read handling in non-blocking socket scenarios and timeout behavior, introducing retry logic for feeding TLS data to rustls, improved EOF/connection-closed detection, and a new Timeout error variant to distinguish socket read timeouts from other IO errors.
Changes
| Cohort / File(s) | Summary |
|---|---|
SSL read and timeout handling crates/stdlib/src/ssl/compat.rs |
Modified ssl_read to add a data-driven retry flow for non-blocking sockets: zero-timeout sockets return WantRead immediately, while timed-out sockets attempt additional TLS data reads instead of erroring. Enhanced ssl_read_tls_records to handle cases where rustls cannot consume data due to a full buffer by processing existing packets and retrying. Changed ssl_ensure_data_available timeout semantics to return Timeout error instead of WantRead when a socket with timeout exceeds its time limit. Improved EOF/connection-closed detection by mapping ECONNABORTED/ECONNRESET to Eof. Simplified "buffer full" fallback path to defer plaintext consumption to main loop. Added clarifying comments for non-blocking vs. blocking behavior and timeout semantics. Introduced new public SslError::Timeout(String) variant. |
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
- Share more ssl consts and fix openssl #6462 — Modifies
ssl_readand changes TLS read/EOF/timeout/error handling in the same file with overlapping concerns. - Fix SSL deferred error #6371 — Also updates
ssl_readerror-handling for socket read failures and maps connection-closed conditions to EOF-like behavior. - Fix openssl build and shared ssl/error.rs #6456 — Touches SSL error handling and
ssl/compat.rsimports, particularly around SSL error type organization.
Poem
🐰 A hop, a skip, through timeout's trick,
Non-blocking reads that stick so quick,
Retry the dance, feed rustls once more,
ujson installs—no SSLWantRead's sore! ✨
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Title check | The PR title 'Fix install ujson' is vague and does not accurately represent the actual changes, which are comprehensive SSL read handling improvements. | Update title to reflect the main change, such as 'Fix SSL read handling for blocking sockets with timeout support' or similar. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Linked Issues check | ✅ Passed | The PR successfully addresses the core requirements: fixes SSLWantReadError on blocking sockets (#6500, #6491), implements proper timeout handling, adds Timeout error variant, and adjusts SSL state machine interaction to prevent premature errors during streaming operations. |
| Out of Scope Changes check | ✅ Passed | All changes are directly related to fixing SSL read handling for blocking sockets and streaming scenarios as specified in the linked issues; no unrelated modifications are present. |
| 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.
Comment @coderabbitai help to get the list of available commands and usage tips.