Skip to content

Commit bf403d9

Browse files
committed
fix: timing
1 parent 2abd322 commit bf403d9

3 files changed

Lines changed: 68 additions & 34 deletions

File tree

‎azure-iot-device/azure/iot/device/common/pipeline/pipeline_stages_mqtt.py‎

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,8 @@ def on_disconnect_returned():
163163
# an unexpected-drop callback, it was queued first and consumed this
164164
# operation. Otherwise, complete the explicit disconnection path now.
165165
logger.info("{}: MQTT disconnected".format(self.name))
166-
self._handle_mqtt_disconnected()
166+
self.send_event_up(pipeline_events_base.DisconnectedEvent())
167+
self._complete_pending_connection_op_after_disconnect()
167168

168169
try:
169170
self.transport.disconnect(clear_inflight=op.hard)
@@ -294,7 +295,25 @@ def _on_mqtt_message_received(self, topic, payload):
294295
def _on_mqtt_connection_dropped(self, cause):
295296
"""Handle a transport-reported unexpected connection loss."""
296297
logger.info("{}: MQTT connection dropped unexpectedly: {}".format(self.name, cause))
297-
self._handle_mqtt_disconnected(cause)
298+
self.send_event_up(pipeline_events_base.DisconnectedEvent())
299+
try:
300+
self._reconcile_mqtt_operation_tracking_after_connection_drop()
301+
302+
# Higher layers will see that we're disconnected and may reconnect as necessary.
303+
error = transport_exceptions.ConnectionDroppedError("Unexpected disconnection")
304+
error.__cause__ = cause
305+
self.report_background_exception(error)
306+
finally:
307+
# Completion callbacks can synchronously start work on a replacement connection.
308+
self._complete_pending_connection_op_after_disconnect(cause)
309+
310+
@pipeline_thread.runs_on_pipeline_thread
311+
def _reconcile_mqtt_operation_tracking_after_connection_drop(self):
312+
"""Reconcile MQTT operation tracking with the connection recovery policy.
313+
314+
This cannot be encapsulated inside the MQTTTransport because it has to do with
315+
connection_retry policy.
316+
"""
298317

299318
# If there is no connection retry, complete tracked MQTT operations as cancelled so
300319
# they do not remain pending indefinitely.
@@ -317,25 +336,9 @@ def _on_mqtt_connection_dropped(self, cause):
317336
)
318337
self.transport._op_manager.stop_tracking_non_publish_operations()
319338

320-
# Higher layers will see that we're disconnected and may reconnect as necessary.
321-
error = transport_exceptions.ConnectionDroppedError("Unexpected disconnection")
322-
error.__cause__ = cause
323-
self.report_background_exception(error)
324-
325339
@pipeline_thread.runs_on_pipeline_thread
326-
def _handle_mqtt_disconnected(self, cause=None):
327-
"""Apply disconnected-state effects on the pipeline thread.
328-
329-
Called after either an unexpected transport callback or a successful explicit disconnect.
330-
331-
:param Exception cause: The Exception that caused the disconnection, if any (optional)
332-
"""
333-
# Send an event to tell other pipeline stages that we're disconnected. Do this before
334-
# we do anything else (in case upper stages have any "are we connected" logic.)
335-
# NOTE: Other stages rely on the fact that this occurs before any op that may be in
336-
# progress is completed. Be careful with changing the order things occur here.
337-
self.send_event_up(pipeline_events_base.DisconnectedEvent())
338-
340+
def _complete_pending_connection_op_after_disconnect(self, cause=None):
341+
"""Complete a pending connection operation after disconnection effects are applied."""
339342
if self._pending_connection_op:
340343

341344
connection_op = self._pending_connection_op

‎tests/unit/common/pipeline/test_pipeline_stages_mqtt.py‎

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -945,9 +945,7 @@ def test_disconnect_op_completed(self, stage, arbitrary_exception, pending_conne
945945
assert pending_connection_op.completed
946946
assert pending_connection_op.error is None
947947

948-
@pytest.mark.it(
949-
"Applies connection-drop handling after completing the pending DisconnectOperation"
950-
)
948+
@pytest.mark.it("Applies connection-drop handling with a pending DisconnectOperation")
951949
@pytest.mark.parametrize(
952950
"connection_retry",
953951
[
@@ -981,6 +979,50 @@ def test_connection_drop_handling(
981979
assert isinstance(background_exception, transport_exceptions.ConnectionDroppedError)
982980
assert background_exception.__cause__ is arbitrary_exception
983981

982+
@pytest.mark.it(
983+
"Applies connection-drop handling before completing the pending DisconnectOperation"
984+
)
985+
@pytest.mark.parametrize(
986+
"connection_retry, cleanup_method_name",
987+
[
988+
pytest.param(
989+
False,
990+
"complete_all_tracked_operations_as_cancelled",
991+
id="Connection retry disabled",
992+
),
993+
pytest.param(
994+
True,
995+
"stop_tracking_non_publish_operations",
996+
id="Connection retry enabled",
997+
),
998+
],
999+
)
1000+
def test_connection_drop_handling_order(
1001+
self,
1002+
stage,
1003+
arbitrary_exception,
1004+
connection_retry,
1005+
cleanup_method_name,
1006+
):
1007+
call_order = []
1008+
stage.nucleus.pipeline_configuration.connection_retry = connection_retry
1009+
stage.send_event_up.side_effect = lambda event: call_order.append(type(event))
1010+
cleanup_method = getattr(stage.transport._op_manager, cleanup_method_name)
1011+
cleanup_method.side_effect = lambda: call_order.append(cleanup_method_name)
1012+
pending_connection_op = pipeline_ops_base.DisconnectOperation(
1013+
callback=lambda op, error: call_order.append(type(op))
1014+
)
1015+
stage._pending_connection_op = pending_connection_op
1016+
1017+
stage.transport.on_mqtt_connection_dropped_handler(arbitrary_exception)
1018+
1019+
assert call_order == [
1020+
pipeline_events_base.DisconnectedEvent,
1021+
cleanup_method_name,
1022+
pipeline_events_base.BackgroundExceptionEvent,
1023+
pipeline_ops_base.DisconnectOperation,
1024+
]
1025+
9841026

9851027
@pytest.mark.describe(
9861028
"MQTTTransportStage - OCCURRENCE: MQTT connection dropped with pending ConnectOperation"

‎tests/unit/iothub/test_sync_clients.py‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
import time
1111
import urllib
1212
import sys
13-
import warnings
1413
from azure.iot.device.iothub import IoTHubDeviceClient, IoTHubModuleClient
1514
from azure.iot.device import exceptions as client_exceptions
1615
from azure.iot.device.common.auth import sastoken as st
@@ -1455,16 +1454,6 @@ def test_sets_on_c2d_message_received_handler_in_pipeline(
14551454
client._mqtt_pipeline.on_c2d_message_received == client._inbox_manager.route_c2d_message
14561455
)
14571456

1458-
@pytest.mark.it("Constructs a public client without Paho callback API deprecation warnings")
1459-
def test_no_paho_callback_api_deprecation_warning(self):
1460-
with warnings.catch_warnings():
1461-
warnings.simplefilter("error", DeprecationWarning)
1462-
client = IoTHubDeviceClient.create_from_connection_string(
1463-
"HostName=hostname.azure-devices.net;DeviceId=MyDevice;SharedAccessKey=Zm9vYmFy"
1464-
)
1465-
1466-
client.shutdown()
1467-
14681457

14691458
@pytest.mark.describe("IoTHubDeviceClient (Synchronous) - .create_from_connection_string()")
14701459
class TestIoTHubDeviceClientCreateFromConnectionString(

0 commit comments

Comments
 (0)