Treat read() returning no bytes as a permanent failure#87
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #87 +/- ##
==========================================
+ Coverage 90.86% 91.61% +0.75%
==========================================
Files 22 22
Lines 3482 3603 +121
==========================================
+ Hits 3164 3301 +137
+ Misses 318 302 -16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR hardens disconnect handling by treating “readable but read() returns no bytes” (common on POSIX after device unplug) as a persistent, instance-wide failure. This prevents consumers from silently receiving EOF (b"") and then continuing to write or query/set modem pins against a broken connection.
Changes:
- Add a persistent “broken” state to serial instances/transports and check it across reads, writes, and modem-pin operations.
- Treat zero-byte reads as a disconnect on POSIX (sync and asyncio fd-based transports), closing/errored appropriately instead of EOFing.
- Extend test infrastructure with an “unplug” capability (socat) and add new tests asserting that operations raise after unplug.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_sync_transports.py | Adds sync unplug regression test ensuring operations raise after disconnect. |
| tests/test_async_transports.py | Adds async unplug regression tests (AsyncSerial + StreamReader) expecting exceptions after disconnect. |
| tests/conftest.py | Threads unplug callables through the serial_pair fixture. |
| tests/common.py | Adds NO_UNPLUG quirk, extends SerialPair, and upgrades create_socat_pair() to provide unplug callables. |
| tests/test_serial_linux.py | Updates create_socat_pair() tuple unpacking after signature change. |
| tests/test_serial_esphome.py | Updates create_socat_pair() tuple unpacking after signature change. |
| serialx/common.py | Introduces persistent broken-state tracking and enforces it on read/write/modem-pin APIs (sync + transport-side checks). |
| serialx/platforms/serial_posix.py | Marks serial instance broken when a low-level read returns 0 bytes and raises EIO. |
| serialx/descriptor_transport.py | Converts zero-byte fd reads into a broken-state + errored close (prevents busy-looping on b""). |
| serialx/platforms/serial_rfc2217/init.py | Marks RFC2217 as broken on connection close and on TCP connection lost. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
a0b44d1 to
487419d
Compare
Serial ports on POSIX enter a strange state on device disconnect:
selectwill return thatnbytes can be read butread(n)returns no bytes. We should enter an instance-wide failure state if this is a case (not just on read) because writes and pin state changes should fail.Related: home-assistant/core#170257