Skip to content

fix(win32): handle missing asyncio protocol methods in _MethodProxy#97

Merged
puddly merged 8 commits into
puddly:devfrom
HiDiHo01:dev
Jun 8, 2026
Merged

fix(win32): handle missing asyncio protocol methods in _MethodProxy#97
puddly merged 8 commits into
puddly:devfrom
HiDiHo01:dev

Conversation

@HiDiHo01

@HiDiHo01 HiDiHo01 commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

On Windows, when a serial port closes or disconnects, asyncio's proactor event loop calls eof_received() on the transport's protocol. _MethodProxy looks this up via getattr from a fixed _mapping dict, but eof_received is not in that mapping, so a KeyError is raised: Fatal error: protocol.eof_received() call failed.
...
File ".../serialx/platforms/serial_win32.py", line 434, in getattr
return self._mapping[name]
KeyError: 'eof_received'
This surfaces as a noisy Fatal error log on every HA shutdown/restart cycle when a Zigbee or other serial device is connected on Windows.

On Windows, when a serial port closes or disconnects, asyncio's proactor event loop calls eof_received() on the transport's protocol. _MethodProxy looks this up via __getattr__ from a fixed _mapping dict, but eof_received is not in that mapping, so a KeyError is raised:
Fatal error: protocol.eof_received() call failed.
...
  File ".../serialx/platforms/serial_win32.py", line 434, in __getattr__
    return self._mapping[name]
KeyError: 'eof_received'
This surfaces as a noisy Fatal error log on every HA shutdown/restart cycle when a Zigbee or other serial device is connected on Windows.
@puddly

puddly commented Jun 7, 2026

Copy link
Copy Markdown
Owner

Thanks for the PR. I think it would be better to just proxy eof_received, no?

@HiDiHo01

HiDiHo01 commented Jun 7, 2026

Copy link
Copy Markdown
Contributor Author

Fair point — it's more explicit and doesn't silently swallow arbitrary unknown attribute access. The correct behaviour for eof_received on a serial transport is to return False (meaning "don't keep the connection half-open"), which is what asyncio.Protocol.eof_received() does by default.

So the mapping entry would be:

"eof_received": lambda: False,

That's more honest than a catch-all no-op: it documents exactly which asyncio protocol method was missing and returns the semantically correct value rather than None.

Update the PR description accordingly — replace the proposed __getattr__ change with just adding "eof_received" to _mapping, and note the return value rationale.

Change the __getattr__ method to raise an AttributeError instead of returning a no-op lambda for missing attributes.
@codecov

codecov Bot commented Jun 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.16%. Comparing base (f9f1bf2) to head (071bd15).

Files with missing lines Patch % Lines
serialx/platforms/serial_win32.py 50.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev      #97      +/-   ##
==========================================
- Coverage   92.23%   92.16%   -0.08%     
==========================================
  Files          22       22              
  Lines        3632     3637       +5     
==========================================
+ Hits         3350     3352       +2     
- Misses        282      285       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Add eof_received method to handle EOF in asyncio proactor
Comment thread serialx/platforms/serial_win32.py Outdated
@puddly puddly merged commit fb3ef82 into puddly:dev Jun 8, 2026
20 of 22 checks passed
@puddly

puddly commented Jun 8, 2026

Copy link
Copy Markdown
Owner

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants