Skip to content

Fail cleanly when sending while disconnected - #282

Merged
puddly merged 1 commit into
devfrom
zigpy-bot/send-packet-disconnected
Oct 28, 2026
Merged

puddly merged 1 commit into
devfrom
zigpy-bot/send-packet-disconnected

Conversation

@zigpy-review-bot

Copy link
Copy Markdown
Collaborator

Summary

A request sent, or still queued, after the radio disconnected called aps_data_request on None, and the failure was logged as an AttributeError traceback. In home-assistant/core#189366 this was an OTA Query Next Image reply sent after the failed startup (#280) had already disconnected.

Changes

send_packet checks for a disconnected radio once it has its turn and raises a DeliveryError, like any other failed send. zigpy's OTA reply task, like other callers using create_catching_task, ignores those quietly.

Testing

New test sending with the radio disconnected; on dev it fails with the same AttributeError as the report. 146 passed.

Related: #280

A request that is still queued when the radio disconnects, e.g. a reply
to a device after startup has failed, called `aps_data_request` on
`None` and logged an `AttributeError` traceback. Raise a `DeliveryError`
instead, like any other failed send.
@zigpy-review-bot zigpy-review-bot added the bugfix This PR fixes a bug label Oct 28, 2026
@codecov

codecov Bot commented Oct 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.15%. Comparing base (1bb30c8) to head (fcd721a).

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #282   +/-   ##
=======================================
  Coverage   99.15%   99.15%           
=======================================
  Files           7        7           
  Lines         945      947    +2     
=======================================
+ Hits          937      939    +2     
  Misses          8        8           

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

@TheJulianJES
TheJulianJES requested a balanced review from Copilot October 28, 2026 23:13
@TheJulianJES
TheJulianJES marked this pull request as ready for review October 28, 2026 23:13

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues were identified for the disconnected-send error-handling change.

No open issues 🎉

What changed in this PR

This PR makes disconnected sends fail with a DeliveryError instead of an AttributeError, allowing existing caller error handling to handle them quietly. It addresses a secondary failure seen after unsuccessful startup, not the underlying endpoint-slot issue.

Changes:

  • Check radio connectivity after a send acquires its concurrency slot, covering requests queued before disconnection.
  • Add a regression test verifying the error message and absence of pending requests.
File Description
zigpy_deconz/​zigbee/​application.py Raise DeliveryError when the radio is disconnected before sending.
tests/​test_send_receive.py Verify disconnected sends fail cleanly without leaving pending requests.

Review effort: Balanced

@puddly
puddly merged commit d7433f1 into dev Oct 28, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix This PR fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants