Retry after route rediscovery on NWK_NO_ROUTE in send_packet

tests/application/test_requests.py

@@ -592,6 +592,95 @@ def set_route_discovered(req):
     await app.shutdown()
 
 
+@pytest.mark.parametrize("device", [FormedLaunchpadCC26X2R1])
+async def test_request_recovery_route_rediscovery_then_assoc_failure(
+    device, make_application, mocker
+):
+    # A NWK_NO_ROUTE failure that consumes the first attempt, followed by a
+    # first-time MAC_TRANSACTION_EXPIRED recovery on the final attempt, must surface
+    # as a DeliveryError rather than silently reporting a successful send.
+    app, znp_server = make_application(server_cls=device)
+
+    await app.startup(auto_form=False)
+
+    mocker.patch("zigpy_znp.zigbee.application.DATA_CONFIRM_TIMEOUT", new=0.1)
+    app._znp._config[conf.CONF_ZNP_CONFIG][conf.CONF_ARSP_TIMEOUT] = 1
+
+    device = app.add_initialized_device(ieee=t.EUI64(range(8)), nwk=0xABCD)
+
+    assoc_device, _ = c.util.Device.deserialize(b"\xFF" * 100)
+    assoc_device.shortAddr = device.nwk
+    assoc_device.nodeRelation = c.util.NodeRelation.CHILD_FFD_RX_IDLE
+
+    # First attempt fails with NWK_NO_ROUTE, the retried attempt fails with a
+    # first-time MAC_TRANSACTION_EXPIRED association failure
+    data_confirm_statuses = iter(
+        [t.Status.NWK_NO_ROUTE, t.Status.MAC_TRANSACTION_EXPIRED]
+    )
+
+    def data_confirm_replier(req):
+        return c.AF.DataConfirm.Callback(
+            Status=next(data_confirm_statuses),
+            Endpoint=1,
+            TSN=1,
+        )
+
+    znp_server.reply_to(
+        c.AF.DataRequestExt.Req(partial=True),
+        responses=[
+            c.AF.DataRequestExt.Rsp(Status=t.Status.SUCCESS),
+            data_confirm_replier,
+        ],
+    )
+
+    znp_server.reply_to(
+        c.UTIL.AssocGetWithAddress.Req(IEEE=device.ieee, partial=True),
+        responses=[c.UTIL.AssocGetWithAddress.Rsp(Device=assoc_device)],
+    )
+
+    did_assoc_remove = znp_server.reply_to(
+        c.UTIL.AssocRemove.Req(IEEE=device.ieee),
+        responses=[c.UTIL.AssocRemove.Rsp(Status=t.Status.SUCCESS)],
+    )
+
+    did_assoc_add = znp_server.reply_to(
+        c.UTIL.AssocAdd.Req(
+            NWK=device.nwk,
+            IEEE=device.ieee,
+            NodeRelation=c.util.NodeRelation.CHILD_FFD_RX_IDLE,
+        ),
+        responses=[c.UTIL.AssocAdd.Rsp(Status=t.Status.SUCCESS)],
+    )
+
+    was_route_discovered = znp_server.reply_to(
+        c.ZDO.ExtRouteDisc.Req(
+            Dst=device.nwk, Options=c.zdo.RouteDiscoveryOptions.UNICAST, partial=True
+        ),
+        responses=[c.ZDO.ExtRouteDisc.Rsp(Status=t.Status.SUCCESS)],
+    )
+
+    with pytest.raises(DeliveryError):
+        await app.request(
+            device=device,
+            profile=260,
+            cluster=1,
+            src_ep=1,
+            dst_ep=1,
+            sequence=1,
+            data=b"\x00",
+        )
+
+    # Route discovery runs once for the first-attempt NWK_NO_ROUTE and once during
+    # the final-attempt MAC_TRANSACTION_EXPIRED recovery
+    assert was_route_discovered.call_count == 2
+    # The association is removed exactly once during recovery and, since the send
+    # ultimately fails, must be re-added exactly once
+    assert len(did_assoc_remove.mock_calls) == 1
+    assert len(did_assoc_add.mock_calls) == 1
+
+    await app.shutdown()
+
+
 @pytest.mark.parametrize("device_cls", FORMED_DEVICES)
 @pytest.mark.parametrize("fw_assoc_remove", [True, False])
 @pytest.mark.parametrize("final_status", [t.Status.SUCCESS, t.Status.APS_NO_ACK])

zigpy_znp/zigbee/application.py

@@ -887,12 +887,14 @@ async def send_packet(self, packet: zigpy.types.ZigbeePacket) -> None:
         succeeded = False
         child_association = None
         tried_assoc_remove = False
+        last_error = None
 
         try:
             # We retry sending twice but the only devices that will use the second retry
             # attempt are sleeping end devices that silently switched parents from the
-            # coordinator, all others will fail immediately
-            for _retry_attempt in range(2):
+            # coordinator, as well as devices for which we just rediscovered a route.
+            # All others will fail immediately.
+            for retry_attempt in range(2):
                 async with self._limit_concurrency(priority=packet.priority):
                     try:
                         # ZDO requests do not generate `AF.DataConfirm` messages
@@ -933,6 +935,8 @@ async def send_packet(self, packet: zigpy.types.ZigbeePacket) -> None:
                         succeeded = True
                         break
                     except InvalidCommandResponse as e:
+                        last_error = e
+
                         # Child aging is disabled so if a child switches parents from
                         # the coordinator to another router, we will not be able to
                         # re-discover a route to it. We have to manually drop the child
@@ -973,11 +977,24 @@ async def send_packet(self, packet: zigpy.types.ZigbeePacket) -> None:
                             else:
                                 continue
 
-                        # Perform route discovery explicitly if the stack fails
-                        if e.response.Status == t.Status.NWK_NO_ROUTE:
+                        # Perform route discovery explicitly if the stack fails and
+                        # then retry the request
+                        if (
+                            e.response.Status == t.Status.NWK_NO_ROUTE
+                            and device is not None
+                            and retry_attempt < 1
+                        ):
                             await self._discover_route(device.nwk)
+                            continue
 
                         raise
+            else:
+                # Every attempt was consumed by a recovery `continue` without a
+                # successful send (e.g. NWK_NO_ROUTE followed by a first-time
+                # MAC_TRANSACTION_EXPIRED on the final attempt). Surface the last
+                # error instead of silently reporting the send as successful.
+                if last_error is not None:
+                    raise last_error
         except InvalidCommandResponse as e:
             status = e.response.Status
             raise DeliveryError(f"Failed to send request: {status!r}", status=status)