diff --git a/azure-iot-device/azure/iot/device/common/mqtt_transport.py b/azure-iot-device/azure/iot/device/common/mqtt_transport.py index be1f3abc9..60257680f 100644 --- a/azure-iot-device/azure/iot/device/common/mqtt_transport.py +++ b/azure-iot-device/azure/iot/device/common/mqtt_transport.py @@ -5,30 +5,71 @@ # -------------------------------------------------------------------------- import paho.mqtt.client as mqtt +import contextlib +import functools import logging import ssl import threading import traceback import weakref import socket +from enum import Enum from . import transport_exceptions as exceptions import socks logger = logging.getLogger(__name__) -# Mapping of Paho CONNACK rc codes to Error object classes -# Used for connection callbacks -paho_connack_rc_to_error = { - mqtt.CONNACK_REFUSED_PROTOCOL_VERSION: exceptions.ProtocolClientError, - mqtt.CONNACK_REFUSED_IDENTIFIER_REJECTED: exceptions.ProtocolClientError, - mqtt.CONNACK_REFUSED_SERVER_UNAVAILABLE: exceptions.ConnectionFailedError, - mqtt.CONNACK_REFUSED_BAD_USERNAME_PASSWORD: exceptions.UnauthorizedError, - mqtt.CONNACK_REFUSED_NOT_AUTHORIZED: exceptions.UnauthorizedError, +CONNECTION_TIMEOUT = 60 + + +def serialize_connection_lifecycle(fn): + """Serialize public MQTT connection lifecycle operations.""" + + @functools.wraps(fn) + def wrapper(self, *args, **kwargs): + with self._op_manager.connection_context(): + with self._connection_lock: + return fn(self, *args, **kwargs) + + return wrapper + + +def coordinate_operation(fn): + """Coordinate a Paho operation call with its completion tracking.""" + + @functools.wraps(fn) + def wrapper(self, *args, **kwargs): + with self._op_manager.operation_context(): + return fn(self, *args, **kwargs) + + return wrapper + + +# This transport speaks MQTT 3.1.1, but Paho callback API v2 represents callback results +# with MQTT 5 ReasonCode and Properties types. For MQTT 3.1.1, Paho synthesizes these values: +# - CONNACK and SUBACK ReasonCode objects from their MQTT 3.1.1 Return Codes +# - a disconnect ReasonCode from Paho's own MQTTErrorCode +# - a successful ReasonCode for publish completion +# - empty Properties objects, and an empty reason_codes list for UNSUBACK +# These are Paho API values, not fields received in MQTT 3.1.1 Control Packets. +# Maps Paho's synthesized CONNACK reason names to SDK exception types. +paho_connack_reason_name_to_error_type = { + "Unsupported protocol version": exceptions.ProtocolClientError, + "Client identifier not valid": exceptions.ProtocolClientError, + "Server unavailable": exceptions.ConnectionFailedError, + "Bad user name or password": exceptions.UnauthorizedError, + "Not authorized": exceptions.UnauthorizedError, } -# Mapping of Paho rc codes to Error object classes -# Used for responses to Paho APIs and non-connection callbacks -paho_rc_to_error = { +# Maps Paho's synthesized disconnect reason names to SDK exception types. MQTT 3.1.1 has no +# server-to-client DISCONNECT packet or disconnect reason field. +paho_disconnect_reason_name_to_error_type = { + "Unspecified error": exceptions.ConnectionDroppedError, + "Keep alive timeout": exceptions.ConnectionDroppedError, +} + +# Maps Paho library error codes to SDK exception types. +paho_error_code_to_error_type = { mqtt.MQTT_ERR_NOMEM: exceptions.ProtocolClientError, mqtt.MQTT_ERR_PROTOCOL: exceptions.ProtocolClientError, mqtt.MQTT_ERR_INVAL: exceptions.ProtocolClientError, @@ -48,43 +89,171 @@ } -def _create_error_from_connack_rc_code(rc): - """ - Given a paho CONNACK rc code, return an Exception that can be raised - """ - message = mqtt.connack_string(rc) - if rc in paho_connack_rc_to_error: - return paho_connack_rc_to_error[rc](message) +def _create_error_from_paho_connack_reason(reason_code): + """Translate Paho's synthesized CONNACK ReasonCode into an SDK transport exception.""" + paho_reason_name = str(reason_code) + if paho_reason_name in paho_connack_reason_name_to_error_type: + return paho_connack_reason_name_to_error_type[paho_reason_name](paho_reason_name) else: - return exceptions.ProtocolClientError("Unknown CONNACK rc={}".format(rc)) + return exceptions.ProtocolClientError("Unknown Paho CONNACK reason={}".format(reason_code)) -def _create_error_from_rc_code(rc): - """ - Given a paho rc code, return an Exception that can be raised - """ - if rc == 1: - # Paho returns rc=1 to mean "something went wrong. stop". We manually translate this to a ConnectionDroppedError. - return exceptions.ConnectionDroppedError("Paho returned rc==1") - elif rc in paho_rc_to_error: - message = mqtt.error_string(rc) - return paho_rc_to_error[rc](message) +def _create_error_from_paho_disconnect_reason(reason_code): + """Translate Paho's synthesized disconnect ReasonCode into an SDK transport exception.""" + paho_reason_name = str(reason_code) + if paho_reason_name in paho_disconnect_reason_name_to_error_type: + return paho_disconnect_reason_name_to_error_type[paho_reason_name](paho_reason_name) else: - return exceptions.ProtocolClientError("Unknown rc=={}".format(rc)) + return exceptions.ProtocolClientError( + "Unknown Paho disconnect reason={}".format(reason_code) + ) + + +def _create_error_from_paho_error_code(error_code): + """Translate a Paho library error code into an SDK transport exception.""" + if error_code in paho_error_code_to_error_type: + message = mqtt.error_string(error_code) + return paho_error_code_to_error_type[error_code](message) + else: + return exceptions.ProtocolClientError("Unknown Paho error code={}".format(error_code)) + + +class ConnectionState(Enum): + # No CONNACK or pre-completion disconnection has been processed. + WAITING_FOR_CONNACK = "WAITING_FOR_CONNACK" + # A successful CONNACK arrived, but connect() has not yet committed success. + CONNACK_ACCEPTED = "CONNACK_ACCEPTED" + # connect() consumed the successful CONNACK and may return to its caller. + CONNECTED = "CONNECTED" + # An explicit disconnect began after connect() completed successfully. + DISCONNECTING = "DISCONNECTING" + # Network connection closure has been processed. + DISCONNECTED = "DISCONNECTED" + # Connection establishment ended unsuccessfully; its stored error is authoritative. + FAILED = "FAILED" + + +class ConnectionLifecycle(object): + """Synchronize Paho lifecycle callbacks with blocking transport operations.""" + + def __init__(self): + self._condition = threading.Condition() + self._state = ConnectionState.DISCONNECTED + self._error = None + + def begin_connect(self): + """Reset state for a new connection after the previous network loop exits.""" + with self._condition: + self._state = ConnectionState.WAITING_FOR_CONNACK + self._error = None + + def record_connack_accepted(self): + """Record an accepted CONNACK received by Paho.""" + with self._condition: + if self._state is ConnectionState.WAITING_FOR_CONNACK: + self._state = ConnectionState.CONNACK_ACCEPTED + self._condition.notify_all() + + def record_connack_rejected(self, error): + """Record a rejected CONNACK received by Paho.""" + with self._condition: + if self._state in ( + ConnectionState.WAITING_FOR_CONNACK, + ConnectionState.CONNACK_ACCEPTED, + ): + self._state = ConnectionState.FAILED + self._error = error + self._condition.notify_all() + + def wait_for_connection(self, timeout): + """Wait for connection establishment to succeed, fail, or time out.""" + with self._condition: + if not self._condition.wait_for( + lambda: self._state is not ConnectionState.WAITING_FOR_CONNACK, + timeout=timeout, + ): + self._state = ConnectionState.FAILED + self._error = exceptions.ConnectionTimeoutError( + "Timed out waiting for MQTT CONNACK" + ) + + if self._state is ConnectionState.CONNACK_ACCEPTED: + self._state = ConnectionState.CONNECTED + return + + error = self._error + self._error = None + raise error + + def record_disconnection(self, cause): + """Record connection closure and indicate whether the connection dropped unexpectedly. + + A closure during connection establishment completes connect() with an error. A closure + after establishment is reported only if explicit disconnect has not begun. Duplicate + closures are ignored. + """ + with self._condition: + if self._state is ConnectionState.WAITING_FOR_CONNACK: + # Paho 2.1 skips on_connect for an MQTT 3.1.1 protocol-version refusal + # when automatic reconnect is disabled. Its callback API v2 reports that + # case and an ordinary pre-CONNACK network loss identically, so this + # transport cannot preserve the protocol-specific classification here. + self._state = ConnectionState.FAILED + self._error = exceptions.ConnectionFailedError( + "Connection closed before MQTT CONNACK outcome" + ) + self._condition.notify_all() + return False + elif self._state is ConnectionState.CONNACK_ACCEPTED: + self._state = ConnectionState.FAILED + self._error = cause or exceptions.ConnectionDroppedError( + "Network connection closed during connect" + ) + self._condition.notify_all() + return False + elif self._state is ConnectionState.CONNECTED: + self._state = ConnectionState.DISCONNECTED + return True + elif self._state is ConnectionState.DISCONNECTING: + self._state = ConnectionState.DISCONNECTED + return False + else: + return False + + def finish_failed_connect(self): + """Reset terminal state after a failed connection has been cleaned up.""" + with self._condition: + self._state = ConnectionState.DISCONNECTED + self._error = None + + def begin_disconnect(self): + """Record that an intentional disconnect has begun.""" + with self._condition: + if self._state is ConnectionState.CONNECTED: + self._state = ConnectionState.DISCONNECTING + + def finish_disconnect(self): + """Record completion when explicit disconnect returns without a Paho callback.""" + with self._condition: + if self._state is ConnectionState.DISCONNECTING: + self._state = ConnectionState.DISCONNECTED class MQTTTransport(object): """ - A wrapper class that provides an implementation-agnostic MQTT message broker interface. + A wrapper class that provides an implementation-agnostic MQTT Server interface. + This transport uses MQTT 3.1.1. - :ivar on_mqtt_connected_handler: Event handler callback, called upon establishing a connection. - :type on_mqtt_connected_handler: Function - :ivar on_mqtt_disconnected_handler: Event handler callback, called upon a disconnection. - :type on_mqtt_disconnected_handler: Function + Calls to connect(), disconnect(), and shutdown() are serialized internally. Event handlers can + run concurrently with the calling thread and must not invoke connection lifecycle methods + synchronously. Multiple publish, subscribe, and unsubscribe operations can remain outstanding + and complete out of order; their callback tracking is synchronized internally. + + :ivar on_mqtt_connection_dropped_handler: Event handler callback, called when an established + connection closes unexpectedly. + :type on_mqtt_connection_dropped_handler: Function :ivar on_mqtt_message_received_handler: Event handler callback, called upon receiving a message. :type on_mqtt_message_received_handler: Function - :ivar on_mqtt_connection_failure_handler: Event handler callback, called upon a connection failure. - :type on_mqtt_connection_failure_handler: Function """ def __init__( @@ -101,11 +270,11 @@ def __init__( ): """ Constructor to instantiate an MQTT protocol wrapper. - :param str client_id: The id of the client connecting to the broker. - :param str hostname: Hostname or IP address of the remote broker. - :param str username: Username for login to the remote broker. + :param str client_id: The Client Identifier used to connect to the MQTT Server. + :param str hostname: Hostname or IP address of the remote MQTT Server. + :param str username: User Name for authentication with the MQTT Server. :param str server_verification_cert: Certificate which can be used to validate a server-side TLS connection (optional). - :param x509_cert: Certificate which can be used to authenticate connection to a server in lieu of a password (optional). + :param x509_cert: Certificate which can be used to authenticate with the MQTT Server in lieu of a password (optional). :param bool websockets: Indicates whether or not to enable a websockets connection in the Transport. :param str cipher: Cipher string in OpenSSL cipher list format :param proxy_options: Options for sending traffic through proxy servers. @@ -120,11 +289,11 @@ def __init__( self._cipher = cipher self._proxy_options = proxy_options self._keep_alive = keep_alive + self._connection_lock = threading.Lock() + self._connection_lifecycle = ConnectionLifecycle() - self.on_mqtt_connected_handler = None - self.on_mqtt_disconnected_handler = None + self.on_mqtt_connection_dropped_handler = None self.on_mqtt_message_received_handler = None - self.on_mqtt_connection_failure_handler = None self._op_manager = OperationManager() @@ -134,30 +303,32 @@ def _create_mqtt_client(self): """ Create the MQTT client object and assign all necessary event handler callbacks. """ - logger.debug("creating mqtt client") + logger.debug("creating Paho client") # Instantiate the client if self._websockets: - logger.info("Creating client for connecting using MQTT over websockets") + logger.info("Creating Paho client for MQTT over websockets") mqtt_client = mqtt.Client( - callback_api_version=mqtt.CallbackAPIVersion.VERSION1, + callback_api_version=mqtt.CallbackAPIVersion.VERSION2, client_id=self._client_id, clean_session=False, protocol=mqtt.MQTTv311, transport="websockets", + reconnect_on_failure=False, ) mqtt_client.ws_set_options(path="/$iothub/websocket") else: - logger.info("Creating client for connecting using MQTT over TCP") + logger.info("Creating Paho client for MQTT over TCP") mqtt_client = mqtt.Client( - callback_api_version=mqtt.CallbackAPIVersion.VERSION1, + callback_api_version=mqtt.CallbackAPIVersion.VERSION2, client_id=self._client_id, clean_session=False, protocol=mqtt.MQTTv311, + reconnect_on_failure=False, ) if self._proxy_options: - logger.info("Setting custom proxy options on mqtt client") + logger.info("Configuring Paho client proxy options") mqtt_client.proxy_set( proxy_type=self._proxy_options.proxy_type_socks, proxy_addr=self._proxy_options.proxy_address, @@ -175,98 +346,135 @@ def _create_mqtt_client(self): # Set event handlers. Use weak references back into this object to prevent leaks self_weakref = weakref.ref(self) - def get_transport_from_weakref_or_stop_loop(client, callback_name): + def get_transport_from_weakref_or_cleanup_client(client, callback_name): + """Acquire a strong transport reference for the duration of a Paho callback. + + The transport can be collected before a callback running on Paho's thread resolves + its weak reference. If it is already gone, disconnect the orphaned client and stop + its thread; otherwise, the returned reference keeps it alive through callback handling. + """ this = self_weakref() if this is None: logger.info( - "{} called after MQTTTransport was garbage collected; stopping Paho network loop".format( + "Paho callback {} invoked after MQTTTransport was garbage collected; disconnecting Paho Client and stopping network loop".format( callback_name ) ) - client.loop_stop() + client.on_disconnect = None + try: + client.disconnect() + finally: + # From a Paho callback, this requests the current network thread to exit + # without attempting to join itself. + client.loop_stop() return this - def on_connect(client, userdata, flags, rc): - logger.info("connected with result code: {}".format(rc)) - this = get_transport_from_weakref_or_stop_loop(client, "on_connect") + def on_connect(client, userdata, flags, reason_code, properties): + # Paho synthesizes this ReasonCode from the MQTT 3.1.1 Connect Return Code. + logger.info("MQTT CONNACK received; Paho synthesized ReasonCode={}".format(reason_code)) + this = get_transport_from_weakref_or_cleanup_client(client, "on_connect") if this is None: return - if rc: # i.e. if there is an error - if this.on_mqtt_connection_failure_handler: - try: - this.on_mqtt_connection_failure_handler( - _create_error_from_connack_rc_code(rc) - ) - except Exception: - logger.warning( - "Unexpected error calling on_mqtt_connection_failure_handler" - ) - logger.warning(traceback.format_exc()) - else: - logger.warning( - "connection failed, but no on_mqtt_connection_failure_handler handler callback provided" - ) - elif this.on_mqtt_connected_handler: - try: - this.on_mqtt_connected_handler() - except Exception: - logger.warning("Unexpected error calling on_mqtt_connected_handler") - logger.warning(traceback.format_exc()) + connection_lifecycle = this._connection_lifecycle + if connection_lifecycle is None: + logger.warning("MQTT CONNACK received without an active connection attempt") + return + + if reason_code.is_failure: + connection_lifecycle.record_connack_rejected( + _create_error_from_paho_connack_reason(reason_code) + ) else: - logger.debug("No event handler callback set for on_mqtt_connected_handler") + connection_lifecycle.record_connack_accepted() - def on_disconnect(client, userdata, rc): - logger.info("disconnected with result code: {}".format(rc)) - this = get_transport_from_weakref_or_stop_loop(client, "on_disconnect") + def on_disconnect(client, userdata, disconnect_flags, reason_code, properties): + # Paho synthesizes this ReasonCode from its own disconnection error code. + logger.info( + "Paho reported network connection closure; synthesized ReasonCode={}".format( + reason_code + ) + ) + this = get_transport_from_weakref_or_cleanup_client(client, "on_disconnect") if this is None: return cause = None - if rc: # i.e. if there is an error + if reason_code.is_failure: logger.debug("".join(traceback.format_stack())) - cause = _create_error_from_rc_code(rc) - this._force_transport_disconnect_and_cleanup() + cause = _create_error_from_paho_disconnect_reason(reason_code) - if this.on_mqtt_disconnected_handler: - try: - this.on_mqtt_disconnected_handler(cause) - except Exception: - logger.warning("Unexpected error calling on_mqtt_disconnected_handler") - logger.warning(traceback.format_exc()) - else: - logger.warning("No event handler callback set for on_mqtt_disconnected_handler") + connection_lifecycle = this._connection_lifecycle + if connection_lifecycle is None: + return + connection_dropped = connection_lifecycle.record_disconnection(cause) + if not connection_dropped: + return + if cause is None: + cause = exceptions.ConnectionDroppedError("Network connection closed unexpectedly") - def on_subscribe(client, userdata, mid, granted_qos): - logger.info("suback received for {}".format(mid)) - this = get_transport_from_weakref_or_stop_loop(client, "on_subscribe") + try: + if this.on_mqtt_connection_dropped_handler: + this.on_mqtt_connection_dropped_handler(cause) + else: + logger.warning("No on_mqtt_connection_dropped_handler is configured") + except Exception: + logger.warning("Unexpected error calling on_mqtt_connection_dropped_handler") + logger.warning(traceback.format_exc()) + + def on_subscribe(client, userdata, mid, reason_codes, properties): + logger.info( + "MQTT SUBACK received for Packet Identifier {}; Paho synthesized ReasonCodes={}".format( + mid, reason_codes + ) + ) + this = get_transport_from_weakref_or_cleanup_client(client, "on_subscribe") if this is None: return - # subscribe failures are returned from the subscribe() call. This is just - # a notification that a SUBACK was received, so there is no failure case here - this._op_manager.complete_operation(mid) + # Paho synthesizes each ReasonCode from an MQTT 3.1.1 SUBACK Return Code. + # This transport sends one Topic Filter per SUBSCRIBE by design, but handles Paho's + # general callback shape containing one ReasonCode for each bundled subscription. + failed_reason_codes = [ + reason_code for reason_code in reason_codes if reason_code.is_failure + ] + if failed_reason_codes: + error = exceptions.ProtocolClientError( + "Subscription rejected by MQTT Server: {}".format( + ", ".join(str(reason_code) for reason_code in failed_reason_codes) + ) + ) + this._op_manager.complete_operation(mid, error=error) + else: + this._op_manager.complete_operation(mid) - def on_unsubscribe(client, userdata, mid): - logger.info("UNSUBACK received for {}".format(mid)) - this = get_transport_from_weakref_or_stop_loop(client, "on_unsubscribe") + def on_unsubscribe(client, userdata, mid, reason_codes, properties): + logger.info("MQTT UNSUBACK received for Packet Identifier {}".format(mid)) + this = get_transport_from_weakref_or_cleanup_client(client, "on_unsubscribe") if this is None: return - # unsubscribe failures are returned from the unsubscribe() call. This is just - # a notification that a SUBACK was received, so there is no failure case here + # MQTT 3.1.1 UNSUBACK contains only the Packet Identifier, so Paho supplies + # an empty reason_codes list. this._op_manager.complete_operation(mid) - def on_publish(client, userdata, mid): - logger.info("payload published for {}".format(mid)) - this = get_transport_from_weakref_or_stop_loop(client, "on_publish") + def on_publish(client, userdata, mid, reason_code, properties): + logger.info( + "Paho reported publish completion for MID {}; synthesized ReasonCode={}".format( + mid, reason_code + ) + ) + this = get_transport_from_weakref_or_cleanup_client(client, "on_publish") if this is None: return - # publish failures are returned from the publish() call. This is just - # a notification that a PUBACK was received, so there is no failure case here + # MQTT 3.1.1 has no publish-completion reason code or properties, so Paho + # synthesizes successful values. QoS 0 has no acknowledgment, QoS 1 completes + # with PUBACK, and QoS 2 with PUBCOMP. this._op_manager.complete_operation(mid) def on_message(client, userdata, mqtt_message): - logger.info("message received on {}".format(mqtt_message.topic)) - this = get_transport_from_weakref_or_stop_loop(client, "on_message") + logger.info( + "MQTT Application Message received on Topic Name {}".format(mqtt_message.topic) + ) + this = get_transport_from_weakref_or_cleanup_client(client, "on_message") if this is None: return @@ -278,7 +486,7 @@ def on_message(client, userdata, mqtt_message): logger.warning(traceback.format_exc()) else: logger.debug( - "No event handler callback set for on_mqtt_message_received_handler - DROPPING MESSAGE" + "No on_mqtt_message_received_handler is configured; dropping Application Message" ) mqtt_client.on_connect = on_connect @@ -288,64 +496,98 @@ def on_message(client, userdata, mqtt_message): mqtt_client.on_publish = on_publish mqtt_client.on_message = on_message - # Set paho automatic-reconnect delay to 2 hours. Ideally we would turn - # paho auto-reconnect off entirely, but this is the best we can do. Without - # this, we run the risk of our auto-reconnect code and the paho auto-reconnect - # code conflicting with each other. - # The choice of 2 hours is completely arbitrary - mqtt_client.reconnect_delay_set(120 * 60) - - logger.debug("Created MQTT protocol client, assigned callbacks") + logger.debug("Created Paho client and assigned MQTT callbacks") return mqtt_client - def _force_transport_disconnect_and_cleanup(self): - """ - After disconnecting because of an error, Paho was designed to keep the loop running and - to try reconnecting after the reconnect interval. We don't want Paho to reconnect because - we want to control the timing of the reconnect, so we force the loop to stop. - - We are relying on intimate knowledge of Paho behavior here. If this becomes a problem, - it may be necessary to write our own Paho thread and stop using thread_start()/thread_stop(). - This is certainly supported by Paho, but the thread that Paho provides works well enough - (so far) and making our own would be more complex than is currently justified. + def _disconnect_and_stop_network_loop(self): + """Disconnect the Paho client, then stop and join its network loop.""" + + logger.info("Disconnecting Paho client and stopping network loop") + + try: + self._mqtt_client.disconnect() + finally: + # Always stop and join the network thread, even if disconnect() fails. + self._mqtt_client.loop_stop() + + logger.debug("Finished disconnecting Paho client and stopping network loop") + + def _cleanup_failed_connect(self): + """Clean up a failed connection setup without reporting a second lifecycle result. + + connect() reports these failures synchronously by raising an exception. Suppress Paho's + disconnect callback during teardown so the same attempt is not also reported as a + disconnection, then restore it for future connection attempts. """ + on_disconnect = self._mqtt_client.on_disconnect + self._mqtt_client.on_disconnect = None + try: + self._disconnect_and_stop_network_loop() + finally: + self._mqtt_client.on_disconnect = on_disconnect + self._connection_lifecycle.finish_failed_connect() - logger.info("Forcing paho disconnect to prevent it from automatically reconnecting") + def _cleanup_failed_connect_best_effort(self): + """Try to clean up after connect() fails, but do not raise cleanup errors. - # Note: We are calling this inside our on_disconnect() handler, so we might be inside the - # Paho thread at this point. This is perfectly valid. Comments in Paho's client.py - # loop_forever() function re-comment calling disconnect() from a callback to exit the - # Paho thread/loop. + The caller is already handling the error that caused connect() to fail. Preserve that + error by logging any later cleanup failure instead of raising it. The partial thread-start + recovery calls _cleanup_failed_connect() directly because it needs to detect cleanup + failure and replace the unusable Paho client. + """ + try: + self._cleanup_failed_connect() + except Exception: + logger.warning( + "Paho cleanup failed after connection failure; preserving original connection error" + ) + logger.warning(traceback.format_exc()) - self._mqtt_client.disconnect() + def _cleanup_after_network_loop_start_failure(self): + """Clean up after Paho raises while starting its network thread. - # Calling disconnect() isn't enough. We also need to call loop_stop to make sure - # Paho is as clean as possible. Our call to disconnect() above is enough to stop the - # loop and exit the tread, but the call to loop_stop() is necessary to complete the cleanup. + Paho can retain an unstarted thread if Thread.start() raises, which also causes + loop_stop() to raise rather than clean up. If normal cleanup encounters that state, + discard the unusable Paho client without mutating its private thread state. + """ + failed_client = self._mqtt_client + try: + self._cleanup_failed_connect() + except Exception: + logger.warning( + "Paho cleanup failed after network loop startup failure; replacing client" + ) + logger.warning(traceback.format_exc()) - self._mqtt_client.loop_stop() + failed_client.on_disconnect = None + failed_socket = failed_client.socket() + if failed_socket is not None: + try: + failed_socket.close() + except Exception: + logger.warning("Unexpected error closing failed Paho client socket") + logger.warning(traceback.format_exc()) - # Finally, because of a bug in Paho, we need to null out the _thread pointer. This - # is necessary because the code that sets _thread to None only gets called if you - # call loop_stop from an external thread (and we're still inside the Paho thread here). - if threading.current_thread() == self._mqtt_client._thread: - logger.debug("in paho thread. nulling _thread") - self._mqtt_client._thread = None + try: + self._mqtt_client = self._create_mqtt_client() + except Exception: + logger.warning("Unexpected error replacing failed Paho client") + logger.warning(traceback.format_exc()) - logger.debug("Done forcing paho disconnect") + self._op_manager.complete_all_tracked_operations_as_cancelled() def _create_ssl_context(self): """ This method creates the SSLContext object used by Paho to authenticate the connection. """ - logger.debug("creating a SSL context") + logger.debug("creating SSL context") ssl_context = ssl.SSLContext(protocol=ssl.PROTOCOL_TLS_CLIENT) if self._server_verification_cert: - logger.debug("configuring SSL context with custom server verification cert") + logger.debug("configuring SSL context with custom server verification certificate") ssl_context.load_verify_locations(cadata=self._server_verification_cert) else: - logger.debug("configuring SSL context with default certs") + logger.debug("configuring SSL context with default certificates") ssl_context.load_default_certs() if self._cipher: @@ -357,7 +599,7 @@ def _create_ssl_context(self): raise e if self._x509_cert is not None: - logger.debug("configuring SSL context with client-side certificate and key") + logger.debug("configuring SSL context with client certificate and key") ssl_context.load_cert_chain( self._x509_cert.certificate_file, self._x509_cert.key_file, @@ -369,18 +611,22 @@ def _create_ssl_context(self): return ssl_context + @serialize_connection_lifecycle def shutdown(self): """Shut down the transport. This is (currently) irreversible.""" # Remove the disconnect handler from Paho. We don't want to trigger any events in response # to the shutdown and confuse the higher level layers of code. Just end it. self._mqtt_client.on_disconnect = None - # Now disconnect and do some additional cleanup. - self._force_transport_disconnect_and_cleanup() - self._op_manager.cancel_all_operations() + try: + self._disconnect_and_stop_network_loop() + finally: + self._connection_lifecycle = None + self._op_manager.complete_all_tracked_operations_as_cancelled() - def connect(self, password=None): + @serialize_connection_lifecycle + def connect(self, password=None, timeout=CONNECTION_TIMEOUT): """ - Connect to the MQTT broker, using hostname and username set at instantiation. + Connect to the MQTT Server, using hostname and username set at instantiation. This method should be called as an entry point before sending any telemetry. @@ -389,9 +635,11 @@ def connect(self, password=None): If MQTT connection has been proxied, connection will take a bit longer to allow negotiation with the proxy server. Any errors in the proxy connection process will trigger exceptions - :param str password: The password for connecting with the MQTT broker (Optional). + :param str password: The password for connecting with the MQTT Server (Optional). + :param float timeout: Maximum time to wait for MQTT CONNACK, in seconds. :raises: ConnectionFailedError if connection could not be established. + :raises: ConnectionTimeoutError if MQTT CONNACK was not received before timeout. :raises: ConnectionDroppedError if connection is dropped during execution. :raises: UnauthorizedError if there is an error authenticating. :raises: NoConnectionError in certain failure scenarios where a connection could not be established @@ -399,23 +647,31 @@ def connect(self, password=None): :raises: TlsExchangeAuthError if there a failure with TLS certificate exchange :raises: ProtocolProxyError if there is a proxy-specific error """ - logger.debug("connecting to mqtt broker") + logger.debug("connecting to MQTT Server") + + # An unexpected disconnect callback can run just before Paho's network thread exits. + # loop_stop() blocks until that prior thread exits; before the first connect, its + # no-thread result is harmless. + self._mqtt_client.loop_stop() + + connection_lifecycle = self._connection_lifecycle + connection_lifecycle.begin_connect() self._mqtt_client.username_pw_set(username=self._username, password=password) try: if self._websockets: - logger.info("Connect using port 443 (websockets)") - rc = self._mqtt_client.connect( + logger.info("Connecting to MQTT Server over websockets on port 443") + paho_error_code = self._mqtt_client.connect( host=self._hostname, port=443, keepalive=self._keep_alive ) else: - logger.info("Connect using port 8883 (TCP)") - rc = self._mqtt_client.connect( + logger.info("Connecting to MQTT Server over TCP on port 8883") + paho_error_code = self._mqtt_client.connect( host=self._hostname, port=8883, keepalive=self._keep_alive ) except socket.error as e: - self._force_transport_disconnect_and_cleanup() + self._cleanup_failed_connect_best_effort() # Only this type will raise a special error # To stop it from retrying. @@ -427,8 +683,8 @@ def connect(self, password=None): raise exceptions.TlsExchangeAuthError() from e elif isinstance(e, socks.ProxyError): if isinstance(e, socks.SOCKS5AuthError): - # TODO This is the only I felt like specializing raise exceptions.UnauthorizedError() from e + # NOTE: add other specialized error handling here as necessary else: raise exceptions.ProtocolProxyError() from e else: @@ -437,274 +693,474 @@ def connect(self, password=None): raise exceptions.ConnectionFailedError() from e except Exception as e: - self._force_transport_disconnect_and_cleanup() - + self._cleanup_failed_connect_best_effort() raise exceptions.ProtocolClientError("Unexpected Paho failure during connect") from e - logger.debug("_mqtt_client.connect returned rc={}".format(rc)) - if rc: - raise _create_error_from_rc_code(rc) - self._mqtt_client.loop_start() + logger.debug("Paho client.connect() returned MQTTErrorCode={}".format(paho_error_code)) + if paho_error_code: + self._cleanup_failed_connect_best_effort() + raise _create_error_from_paho_error_code(paho_error_code) + + # Start the network loop to process incoming and outgoing MQTT messages + try: + paho_error_code = self._mqtt_client.loop_start() + except Exception as e: + self._cleanup_after_network_loop_start_failure() + raise exceptions.ProtocolClientError( + "Unexpected Paho failure starting network loop" + ) from e + logger.debug("Paho client.loop_start() returned MQTTErrorCode={}".format(paho_error_code)) + if paho_error_code: + self._cleanup_failed_connect_best_effort() + raise _create_error_from_paho_error_code(paho_error_code) + + logger.debug("Waiting for MQTT CONNACK") + try: + connection_lifecycle.wait_for_connection(timeout=timeout) + except Exception: + self._cleanup_failed_connect_best_effort() + raise + @serialize_connection_lifecycle def disconnect(self, clear_inflight=False): """ - Disconnect from the MQTT broker. + Disconnect from the MQTT Server and wait for the network loop to stop. + + If clear_inflight is True, complete all tracked operations as cancelled. Otherwise, + preserve resumable QoS 1 and QoS 2 publishes and complete non-resumable operations as + cancelled. :raises: ProtocolClientError if there is some client error. :raises: ConnectionDroppedError in unexpected cases. :raises: UnauthorizedError in unexpected cases. :raises: ConnectionFailedError in unexpected cases. """ - logger.info("disconnecting MQTT client") + logger.info("disconnecting from MQTT Server") + if self._connection_lifecycle: + self._connection_lifecycle.begin_disconnect() try: - rc = self._mqtt_client.disconnect() + paho_error_code = self._mqtt_client.disconnect() except Exception as e: raise exceptions.ProtocolClientError("Unexpected Paho failure during disconnect") from e finally: + # Always stop and join the network thread, even if disconnect() fails. self._mqtt_client.loop_stop() - if threading.current_thread() == self._mqtt_client._thread: - logger.debug("in paho thread. nulling _thread") - self._mqtt_client._thread = None - - logger.debug("_mqtt_client.disconnect returned rc={}".format(rc)) - if rc: - # Special case: MQTT_ERR_NO_CONN (rc=4) during disconnect means the socket + logger.debug("Paho client.disconnect() returned MQTTErrorCode={}".format(paho_error_code)) + if paho_error_code: + # Special case: MQTT_ERR_NO_CONN during disconnect means the socket # is already closed. In Paho 2.x, this can happen even after a successful - # disconnect because the on_disconnect callback fires (with rc=0) before + # disconnect because the on_disconnect callback fires successfully before # disconnect() returns, and Paho's internal cleanup closes the socket. # Since we wanted to disconnect and we're disconnected, treat this as success. - if rc == mqtt.MQTT_ERR_NO_CONN: + if paho_error_code == mqtt.MQTT_ERR_NO_CONN: logger.debug( - "disconnect returned MQTT_ERR_NO_CONN - socket already closed, treating as success" + "Paho client.disconnect() returned MQTT_ERR_NO_CONN; network connection is already closed" ) # Still clear inflight operations since we're effectively disconnected if clear_inflight: - self._op_manager.cancel_all_operations() + self._op_manager.complete_all_tracked_operations_as_cancelled() + else: + self._op_manager.complete_non_resumable_operations_as_cancelled() + if self._connection_lifecycle: + self._connection_lifecycle.finish_disconnect() else: # This could result in ConnectionDroppedError or ProtocolClientError - err = _create_error_from_rc_code(rc) + err = _create_error_from_paho_error_code(paho_error_code) raise err else: # Clear pending ops if instructed, but only if the disconnect was successful. # Technically the disconnect could still fail upon response, however that would then - # cause a force disconnect via the on_disconnect handler, thus it is safe to clear + # stop the network loop via the on_disconnect handler, thus it is safe to clear # ops here and now. if clear_inflight: - self._op_manager.cancel_all_operations() + self._op_manager.complete_all_tracked_operations_as_cancelled() + else: + self._op_manager.complete_non_resumable_operations_as_cancelled() + if self._connection_lifecycle: + self._connection_lifecycle.finish_disconnect() + @coordinate_operation def subscribe(self, topic, qos=1, callback=None): """ - This method subscribes the client to one topic from the MQTT broker. + Subscribe the Client to one Topic Filter on the MQTT Server. - :param str topic: a single string specifying the subscription topic to subscribe to - :param int qos: the desired quality of service level for the subscription. Defaults to 1. - :param callback: A callback to be triggered upon completion (Optional). + :param str topic: A single Topic Filter to subscribe to. + :param int qos: The maximum QoS requested for the Subscription. Defaults to 1. + :param callback: A callback to be invoked upon completion (Optional). :raises: ValueError if qos is not 0, 1 or 2. :raises: ValueError if topic is None or has zero string length. :raises: ConnectionDroppedError if connection is dropped during execution. :raises: ProtocolClientError if there is some other client error. - :raises: NoConnectionError if the client isn't actually connected. + :raises: NoConnectionError if the client is not connected. """ - logger.info("subscribing to {} with qos {}".format(topic, qos)) + logger.info( + "sending MQTT SUBSCRIBE for Topic Filter {} with requested maximum QoS {}".format( + topic, qos + ) + ) try: - (rc, mid) = self._mqtt_client.subscribe(topic, qos=qos) + paho_error_code, mid = self._mqtt_client.subscribe(topic, qos=qos) except ValueError: raise except Exception as e: raise exceptions.ProtocolClientError("Unexpected Paho failure during subscribe") from e - logger.debug("_mqtt_client.subscribe returned rc={}".format(rc)) - if rc: + logger.debug("Paho client.subscribe() returned MQTTErrorCode={}".format(paho_error_code)) + if paho_error_code: # This could result in ConnectionDroppedError or ProtocolClientError - raise _create_error_from_rc_code(rc) - self._op_manager.establish_operation(mid, callback) + raise _create_error_from_paho_error_code(paho_error_code) + self._op_manager.register_operation( + mid=mid, callback=callback, operation_type=OperationType.SUBSCRIBE + ) + @coordinate_operation def unsubscribe(self, topic, callback=None): """ - Unsubscribe the client from one topic on the MQTT broker. + Unsubscribe the Client from one Topic Filter on the MQTT Server. - :param str topic: a single string which is the subscription topic to unsubscribe from. - :param callback: A callback to be triggered upon completion (Optional). + :param str topic: A single Topic Filter to unsubscribe from. + :param callback: A callback to be invoked upon completion (Optional). :raises: ValueError if topic is None or has zero string length. :raises: ConnectionDroppedError if connection is dropped during execution. :raises: ProtocolClientError if there is some other client error. :raises: NoConnectionError if the client isn't actually connected. """ - logger.info("unsubscribing from {}".format(topic)) + logger.info("sending MQTT UNSUBSCRIBE for Topic Filter {}".format(topic)) try: - (rc, mid) = self._mqtt_client.unsubscribe(topic) + paho_error_code, mid = self._mqtt_client.unsubscribe(topic) except ValueError: raise except Exception as e: raise exceptions.ProtocolClientError( "Unexpected Paho failure during unsubscribe" ) from e - logger.debug("_mqtt_client.unsubscribe returned rc={}".format(rc)) - if rc: + logger.debug("Paho client.unsubscribe() returned MQTTErrorCode={}".format(paho_error_code)) + if paho_error_code: # This could result in ConnectionDroppedError or ProtocolClientError - raise _create_error_from_rc_code(rc) - self._op_manager.establish_operation(mid, callback) + raise _create_error_from_paho_error_code(paho_error_code) + self._op_manager.register_operation( + mid=mid, callback=callback, operation_type=OperationType.UNSUBSCRIBE + ) + @coordinate_operation def publish(self, topic, payload, qos=1, callback=None): """ - Send a message via the MQTT broker. + Publish an Application Message to the MQTT Server. - :param str topic: topic: The topic that the message should be published on. - :param payload: The actual message to send. + :param str topic: The Topic Name on which to publish the Application Message. + :param payload: The Application Message payload. :type payload: str, bytes, int, float or None - :param int qos: the desired quality of service level for the subscription. Defaults to 1. - :param callback: A callback to be triggered upon completion (Optional). + :param int qos: The QoS level for delivery of the Application Message. Defaults to 1. + :param callback: A callback to be invoked upon completion (Optional). :raises: ValueError if qos is not 0, 1 or 2 :raises: ValueError if topic is None or has zero string length - :raises: ValueError if topic contains a wildcard ("+") + :raises: ValueError if topic contains a wildcard character ("+" or "#") :raises: ValueError if the length of the payload is greater than 268435455 bytes :raises: TypeError if payload is not a valid type :raises: ConnectionDroppedError if connection is dropped during execution. :raises: ProtocolClientError if there is some other client error. - :raises: NoConnectionError if the client isn't actually connected. + :raises: NoConnectionError if a QoS 0 message is published while the client is not connected. """ - logger.info("publishing on {}".format(topic)) + logger.info("sending MQTT PUBLISH on Topic Name {} with QoS {}".format(topic, qos)) try: - (rc, mid) = self._mqtt_client.publish(topic=topic, payload=payload, qos=qos) + # NOTE: Paho MQTTMessageInfo allows you to wait upon the completion with + # `wait_for_publish()`, but that is only supported for PUBLISH. + # We don't take advantage of it in favor of a general solution (i.e. OperationManager) + # which can track SUBSCRIBE and UNSUBSCRIBE operations as well. + # Furthermore, `wait_for_publish()` is buggy when sending a message while disconnected, + # and does not accurately report the success or failure of the publish operation. + message_info = self._mqtt_client.publish(topic=topic, payload=payload, qos=qos) except ValueError: raise except TypeError: raise except Exception as e: raise exceptions.ProtocolClientError("Unexpected Paho failure during publish") from e - logger.debug("_mqtt_client.publish returned rc={}".format(rc)) - if rc: + paho_error_code = message_info.rc + mid = message_info.mid + logger.debug( + "Paho client.publish() returned MQTTMessageInfo with MQTTErrorCode={}".format( + paho_error_code + ) + ) + publish_retained_for_next_connection = paho_error_code == mqtt.MQTT_ERR_NO_CONN and qos > 0 + if paho_error_code and not publish_retained_for_next_connection: # This could result in ConnectionDroppedError or ProtocolClientError - raise _create_error_from_rc_code(rc) - self._op_manager.establish_operation(mid, callback) + raise _create_error_from_paho_error_code(paho_error_code) + if publish_retained_for_next_connection: + logger.debug( + "Paho retained QoS {} PUBLISH with MID {} for the next connection".format(qos, mid) + ) + self._op_manager.register_operation( + mid=mid, + callback=callback, + operation_type={ + 0: OperationType.PUBLISH_QOS_0, + 1: OperationType.PUBLISH_QOS_1, + 2: OperationType.PUBLISH_QOS_2, + }[qos], + ) + + +class OperationType(Enum): + PUBLISH_QOS_0 = "PUBLISH_QOS_0" + PUBLISH_QOS_1 = "PUBLISH_QOS_1" + PUBLISH_QOS_2 = "PUBLISH_QOS_2" + SUBSCRIBE = "SUBSCRIBE" + UNSUBSCRIBE = "UNSUBSCRIBE" + + +class PendingOperation(object): + def __init__(self, operation_type, callback): + self.operation_type = operation_type + self.callback = callback class OperationManager(object): - """Tracks pending operations and their associated callbacks until completion.""" + """Tracks operation callbacks, unmatched completions, and cancellations by Paho MID.""" def __init__(self): - # Maps mid->callback for operations where a request has been sent - # but the response has not yet been received - self._pending_operation_callbacks = {} + # Maps Paho MID to operations awaiting a response. + self._pending_operations = {} - # Maps mid->mid for responses received that are NOT established in the _pending_operation_callbacks dict. + # Maps Paho MIDs with no currently registered operation to optional completion errors. # Necessary because sometimes an operation will complete with a response before the # Paho call returns. - # TODO: make this map mid to something more useful (result code?) self._unknown_operation_completions = {} + # Tracks cancelled MIDs whose Paho operations may still complete. + self._cancelled_operation_mids = set() + self._lock = threading.Lock() + self._cancelled_mid_completion_deferral_count = 0 + self._deferred_cancellation_callbacks = threading.local() + + @contextlib.contextmanager + def operation_context(self): + """Provide a safe context for initiating and tracking one Paho operation. + + Use around a transport method that makes one Paho PUBLISH, SUBSCRIBE, or UNSUBSCRIBE call + and registers its completion tracking before returning. This context handles callbacks + that arrive before Paho returns the operation's MID, including ambiguity caused by reuse + of a cancelled MID. Callers do not need to coordinate those timing details themselves. + """ + with self._lock: + self._cancelled_mid_completion_deferral_count += 1 + try: + yield + finally: + with self._lock: + self._cancelled_mid_completion_deferral_count -= 1 + if self._cancelled_mid_completion_deferral_count == 0: + unclaimed_cancelled_mids = ( + self._cancelled_operation_mids & self._unknown_operation_completions.keys() + ) + for mid in unclaimed_cancelled_mids: + logger.debug("Discarding completion for cancelled Paho MID {}".format(mid)) + self._cancelled_operation_mids.remove(mid) + del self._unknown_operation_completions[mid] + + @contextlib.contextmanager + def connection_context(self): + """Provide a safe context for changing the MQTT connection lifecycle. + + Use around a serialized connect, disconnect, or shutdown call. Operation tracking changes + take effect immediately, but callbacks caused by cancellation are held until context exit + so they cannot re-enter a partially completed connection lifecycle transition. + """ + pending_operations = [] + self._deferred_cancellation_callbacks.pending_operations = pending_operations + try: + yield + finally: + del self._deferred_cancellation_callbacks.pending_operations + self._invoke_cancellation_callbacks(pending_operations) - def establish_operation(self, mid, callback=None): - """Establish a pending operation identified by MID, and store its completion callback. + def register_operation(self, mid, callback, operation_type): + """Register a pending operation under its Paho MID. - If the operation has already been completed, the callback will be triggered. + If a completion has already been recorded for the MID, the callback will be invoked. + Otherwise, the callback will be invoked when the completion is received. """ - trigger_callback = False + invoke_callback = False + completion_error = None with self._lock: - # Check to see if a response was already received for this MID before this method was - # able to be called due to threading shenanigans + # If Paho reuses a cancelled MID without completing its previous operation, its next + # completion belongs to the newly registered operation. + self._cancelled_operation_mids.discard(mid) + + # Paho can invoke the response callback before its API call returns the MID, + # thus, the operation might have already completed. if mid in self._unknown_operation_completions: - # Clear the recorded unknown response now that it has been resolved - del self._unknown_operation_completions[mid] + # Claim the unknown completion now that its operation has been established. + completion_error = self._unknown_operation_completions.pop(mid) - # Since the operation has already completed, indicate callback should trigger - trigger_callback = True + # Since a completion was already recorded, indicate callback should be invoked. + invoke_callback = True else: - # Store the operation as pending, along with callback - self._pending_operation_callbacks[mid] = callback - logger.debug("Waiting for response on MID: {}".format(mid)) + self._pending_operations[mid] = PendingOperation( + operation_type=operation_type, callback=callback + ) + logger.debug("Waiting for response on Paho MID {}".format(mid)) - # Now that the lock has been released, if the callback should be triggered, - # go ahead and trigger it now. - if trigger_callback: + # Invoke the callback only after releasing the lock. + if invoke_callback: logger.debug( - "Response for MID: {} was received early - triggering callback".format(mid) + "Completion for previously unknown Paho MID {} matched registered operation; invoking callback".format( + mid + ) ) if callback: try: - callback() + # Not all operation callbacks accept the optional error argument. + if completion_error is not None: + callback(error=completion_error) + else: + callback() except Exception: - logger.debug("Unexpected error calling callback for MID: {}".format(mid)) + logger.debug("Unexpected error calling callback for Paho MID {}".format(mid)) logger.debug(traceback.format_exc()) else: - # Not entirely unexpected because of QOS=1 - logger.debug("No callback for MID: {}".format(mid)) + # Completion callbacks are optional. + logger.debug("No callback for Paho MID {}".format(mid)) - def complete_operation(self, mid): - """Complete an operation identified by MID and trigger the associated completion callback. + def complete_operation(self, mid, error=None): + """Complete an operation by Paho MID and invoke its callback (if any was set). - If the operation MID is unknown, the completion status will be stored until - the operation is established. + If the MID is unknown, retain its completion in case its operation is registered later. """ callback = None - trigger_callback = False + invoke_callback = False with self._lock: - # If the mid is associated with an established pending operation, trigger the associated callback - if mid in self._pending_operation_callbacks: + if mid in self._cancelled_operation_mids: + if self._cancelled_mid_completion_deferral_count: + logger.debug( + "Completion for cancelled Paho MID {} arrived during operation registration; retaining".format( + mid + ) + ) + self._unknown_operation_completions[mid] = error + else: + logger.debug("Discarding completion for cancelled Paho MID {}".format(mid)) + self._cancelled_operation_mids.remove(mid) - # Retrieve the callback, and clear the pending operation now that it has been completed - callback = self._pending_operation_callbacks[mid] - del self._pending_operation_callbacks[mid] + # If the Paho MID has a pending operation, invoke its callback. + elif mid in self._pending_operations: - # Since the operation is complete, indicate the callback should be triggered - trigger_callback = True + # Retrieve the callback, and clear the pending operation now that it has completed. + callback = self._pending_operations.pop(mid).callback + # Since the operation is complete, indicate the callback should be invoked. + invoke_callback = True + # Otherwise, store the mid as an unknown response else: - # Otherwise, store the mid as an unknown response - logger.debug("Response received for unknown MID: {}".format(mid)) - self._unknown_operation_completions[ - mid - ] = mid # TODO: set something more useful here + logger.debug("Completion received for unknown Paho MID {}; retaining".format(mid)) + self._unknown_operation_completions[mid] = error - # Now that the lock has been released, if the callback should be triggered, - # go ahead and trigger it now. - if trigger_callback: + # Invoke the callback only after releasing the lock. + if invoke_callback: logger.debug( - "Response received for recognized MID: {} - triggering callback".format(mid) + "Response received for registered Paho MID {}; invoking callback".format(mid) ) if callback: try: - callback() + # Not all operation callbacks accept the optional error argument. + if error is not None: + callback(error=error) + else: + callback() except Exception: - logger.debug("Unexpected error calling callback for MID: {}".format(mid)) + logger.debug("Unexpected error calling callback for Paho MID {}".format(mid)) logger.debug(traceback.format_exc()) else: - # fully expected. QOS=1 means we might get 2 PUBACKs - logger.debug("No callback set for MID: {}".format(mid)) + # Completion callbacks are optional. + logger.debug("No callback set for Paho MID {}".format(mid)) + + def complete_non_resumable_operations_as_cancelled(self): + """Complete operations Paho cannot resume as cancelled. - def cancel_all_operations(self): - """Complete all pending operations with cancellation, removing MID tracking""" - logger.debug("Cancelling all pending operations") + Paho retains MQTT 3.1.1 QoS 1 and QoS 2 PUBLISH operations for a later connection. + SUBSCRIBE, UNSUBSCRIBE, and QoS 0 PUBLISH operations are not retained by Paho, so remove + their tracking, tombstone their MIDs, and invoke their callbacks with ``cancelled=True``. + """ + logger.debug("Completing non-resumable tracked operations as cancelled") + with self._lock: + pending_ops = [ + (mid, pending_operation) + for mid, pending_operation in self._pending_operations.items() + if pending_operation.operation_type + not in (OperationType.PUBLISH_QOS_1, OperationType.PUBLISH_QOS_2) + ] + for mid, _ in pending_ops: + del self._pending_operations[mid] + self._cancelled_operation_mids.update(mid for mid, _ in pending_ops) + + self._defer_or_invoke_cancellation_callbacks(pending_ops) + + def complete_all_tracked_operations_as_cancelled(self): + """Complete all tracked SDK operations as cancelled and clear unknown completions. + + This manager owns only local completion tracking: pending callbacks are invoked with + ``cancelled=True``. Their MIDs remain as tombstones so later Paho completions can be + discarded. Paho owns MQTT protocol state for accepted operations, including QoS 1 and + QoS 2 packets retained for redelivery, so those operations may still complete or take + effect. + """ + logger.debug("Completing all tracked operations as cancelled") with self._lock: - # Clear pending operations - pending_ops = list(self._pending_operation_callbacks.items()) - for pending_op in pending_ops: - mid = pending_op[0] - del self._pending_operation_callbacks[mid] - - # Clear unknown responses - unknown_mids = [mid for mid in self._unknown_operation_completions] - for mid in unknown_mids: - del self._unknown_operation_completions[mid] - - # Trigger cancel in pending operation callbacks - for pending_op in pending_ops: - mid = pending_op[0] - callback = pending_op[1] + # Preserve callbacks for invocation after releasing the lock. + pending_ops = list(self._pending_operations.items()) + self._cancelled_operation_mids.update(self._pending_operations) + self._pending_operations.clear() + self._unknown_operation_completions.clear() + + self._defer_or_invoke_cancellation_callbacks(pending_ops) + + def _defer_or_invoke_cancellation_callbacks(self, pending_operations): + deferred_operations = getattr( + self._deferred_cancellation_callbacks, "pending_operations", None + ) + if deferred_operations is not None: + deferred_operations.extend(pending_operations) + else: + self._invoke_cancellation_callbacks(pending_operations) + + def _invoke_cancellation_callbacks(self, pending_operations): + """Invoke callbacks for operations whose tracking was cancelled.""" + for mid, pending_operation in pending_operations: + callback = pending_operation.callback if callback: - logger.debug("Cancelling {} - Triggering callback".format(mid)) + logger.debug( + "Completing tracked operation for Paho MID {} as cancelled; invoking callback".format( + mid + ) + ) try: callback(cancelled=True) except Exception: - logger.debug("Unexpected error calling callback for MID: {}".format(mid)) + logger.debug("Unexpected error calling callback for Paho MID {}".format(mid)) logger.debug(traceback.format_exc()) else: - logger.debug("Cancelling {} - No callback set for MID".format(mid)) + logger.debug( + "Completing tracked operation for Paho MID {} as cancelled; no callback set".format( + mid + ) + ) + + +# TODO: Clarify hard-disconnect semantics because cancelling an SDK publish operation does not +# prevent Paho from delivering a retained QoS 1 or QoS 2 message after a later connection. +# Re-evaluate the inclusion of "hard" disconnect. + +# NOTE: Connection lifecycle calls are deliberately serialized here and by ConnectionStateStage. +# CONNECTION_TIMEOUT bounds the wait for CONNACK, allowing queued lifecycle operations such as +# shutdown to proceed after a failed connection attempt. It does not impose an absolute shutdown +# deadline: Paho socket setup and loop_stop() are blocking and have no safe cancellation API. +# Running shutdown concurrently with another lifecycle call would race Paho's lifecycle state. diff --git a/azure-iot-device/azure/iot/device/common/pipeline/pipeline_ops_base.py b/azure-iot-device/azure/iot/device/common/pipeline/pipeline_ops_base.py index 50557af32..638d607ba 100644 --- a/azure-iot-device/azure/iot/device/common/pipeline/pipeline_ops_base.py +++ b/azure-iot-device/azure/iot/device/common/pipeline/pipeline_ops_base.py @@ -235,9 +235,7 @@ class ConnectOperation(PipelineOperation): Even though this is an base operation, it will most likely be handled by a more specific stage (such as an IoTHub or MQTT stage). """ - def __init__(self, callback): - self.watchdog_timer = None - super().__init__(callback) + pass class ReauthorizeConnectionOperation(PipelineOperation): diff --git a/azure-iot-device/azure/iot/device/common/pipeline/pipeline_stages_base.py b/azure-iot-device/azure/iot/device/common/pipeline/pipeline_stages_base.py index 26f90ffdf..78b8ca228 100644 --- a/azure-iot-device/azure/iot/device/common/pipeline/pipeline_stages_base.py +++ b/azure-iot-device/azure/iot/device/common/pipeline/pipeline_stages_base.py @@ -780,7 +780,7 @@ def __init__(self): self.timeout_intervals = { pipeline_ops_mqtt.MQTTSubscribeOperation: 10, pipeline_ops_mqtt.MQTTUnsubscribeOperation: 10, - # Only Sub and Unsub are here because MQTT auto retries pub + # Only Sub and Unsub are here because MQTT client will resend QoS 1 publishes after reconnect automatically } @pipeline_thread.runs_on_pipeline_thread @@ -838,7 +838,7 @@ def __init__(self): self.retry_intervals = { pipeline_ops_mqtt.MQTTSubscribeOperation: 20, pipeline_ops_mqtt.MQTTUnsubscribeOperation: 20, - # Only Sub and Unsub are here because MQTT auto retries pub + # Only Sub and Unsub are here because MQTT client will resend QoS 1 publishes after reconnect automatically } self.ops_waiting_to_retry = [] diff --git a/azure-iot-device/azure/iot/device/common/pipeline/pipeline_stages_mqtt.py b/azure-iot-device/azure/iot/device/common/pipeline/pipeline_stages_mqtt.py index 236ca244b..d70af53b4 100644 --- a/azure-iot-device/azure/iot/device/common/pipeline/pipeline_stages_mqtt.py +++ b/azure-iot-device/azure/iot/device/common/pipeline/pipeline_stages_mqtt.py @@ -6,7 +6,6 @@ import logging import traceback -import threading import weakref from . import ( pipeline_ops_base, @@ -18,20 +17,17 @@ pipeline_events_base, ) from azure.iot.device.common.mqtt_transport import MQTTTransport -from azure.iot.device.common import handle_exceptions, transport_exceptions +from azure.iot.device.common import transport_exceptions logger = logging.getLogger(__name__) -# Maximum amount of time we wait for ConnectOperation to complete -# TODO: This whole logic of timeout should probably be handled in the TimeoutStage -WATCHDOG_INTERVAL = 60 - class MQTTTransportStage(PipelineStage): """ - PipelineStage object which is responsible for interfacing with the MQTT protocol wrapper object. - This stage handles all MQTT operations and any other operations (such as ConnectOperation) which - is not in the MQTT group of operations, but can only be run at the protocol level. + PipelineStage responsible for interfacing with MQTTTransport. + + This stage handles MQTT operations and connection lifecycle operations that must run at the + transport level. """ def __init__(self): @@ -39,90 +35,28 @@ def __init__(self): # The transport will be instantiated upon receiving the InitializePipelineOperation self.transport = None - # The current in-progress op that affects connection state (Connect, Disconnect, Reauthorize) + # The pending ConnectOperation or DisconnectOperation, if any. self._pending_connection_op = None @pipeline_thread.runs_on_pipeline_thread - def _cancel_pending_connection_op(self, error=None): - """ - Cancel any running connect, disconnect or reauthorize connection op. Since our ability to "cancel" is fairly limited, - all this does (for now) is to fail the operation + def _fail_pending_connection_op(self, error=None): + """Complete the pending connection operation with an error. + + If no error is supplied, the operation is superseded by a newer connection operation and + is completed with OperationCancelled. """ - op = self._pending_connection_op - if op: + pending_op = self._pending_connection_op + if pending_op: # NOTE: This code path should NOT execute in normal flow. There should never already be a pending # connection op when another is added, due to the ConnectionLock stage. # If this block does execute, there is a bug in the codebase. - if not error: + if error is None: error = pipeline_exceptions.OperationCancelled( "Cancelling because new ConnectOperation or DisconnectOperation was issued" ) - self._cancel_connection_watchdog(op) self._pending_connection_op = None - op.complete(error=error) - - @pipeline_thread.runs_on_pipeline_thread - def _start_connection_watchdog(self, connection_op): - """ - Start a watchdog on the connection operation. This protects against cases where transport.connect() - succeeds but the CONNACK never arrives. This is like a timeout, but it is handled at this level - because specific cleanup needs to take place on timeout (see below), and this cleanup doesn't - belong anywhere else since it is very specific to this stage. - """ - logger.debug("{}({}): Starting watchdog".format(self.name, connection_op.name)) - - self_weakref = weakref.ref(self) - op_weakref = weakref.ref(connection_op) - - @pipeline_thread.invoke_on_pipeline_thread - def watchdog_function(): - this = self_weakref() - op = op_weakref() - if this and op and this._pending_connection_op is op: - logger.info( - "{}({}): Connection watchdog expired. Cancelling op".format(this.name, op.name) - ) - try: - this.transport.disconnect() - except Exception: - # If we don't catch this, the pending connection op might not ever be cancelled. - # Most likely, the transport isn't actually connected, but other failures are theoretically - # possible. Either way, if disconnect fails, we should assume that we're disconnected. - logger.info( - "transport.disconnect raised error while disconnecting in watchdog. Safe to ignore." - ) - logger.info(traceback.format_exc()) - - if this.nucleus.connected: - - logger.info( - "{}({}): Pipeline is still connected on watchdog expiration. Sending DisconnectedEvent".format( - this.name, op.name - ) - ) - this.send_event_up(pipeline_events_base.DisconnectedEvent()) - this._cancel_pending_connection_op( - error=pipeline_exceptions.OperationTimeout( - "Transport timeout on connection operation" - ) - ) - else: - logger.debug("Connection watchdog expired, but pending op is not the same op") - - connection_op.watchdog_timer = threading.Timer(WATCHDOG_INTERVAL, watchdog_function) - connection_op.watchdog_timer.daemon = True - connection_op.watchdog_timer.start() - - @pipeline_thread.runs_on_pipeline_thread - def _cancel_connection_watchdog(self, op): - try: - if op.watchdog_timer: - logger.debug("{}({}): cancelling watchdog".format(self.name, op.name)) - op.watchdog_timer.cancel() - op.watchdog_timer = None - except AttributeError: - pass + pending_op.complete(error=error) @pipeline_thread.runs_on_pipeline_thread def _run_op(self, op): @@ -145,7 +79,7 @@ def _run_op(self, op): ) hostname = self.nucleus.pipeline_configuration.hostname - # Create the Transport object, set it's handlers + # Create the transport and set its handlers. logger.debug("{}({}): got connection args".format(self.name, op.name)) self.transport = MQTTTransport( client_id=op.client_id, @@ -158,24 +92,11 @@ def _run_op(self, op): proxy_options=self.nucleus.pipeline_configuration.proxy_options, keep_alive=self.nucleus.pipeline_configuration.keep_alive, ) - self.transport.on_mqtt_connected_handler = self._on_mqtt_connected - self.transport.on_mqtt_connection_failure_handler = self._on_mqtt_connection_failure - self.transport.on_mqtt_disconnected_handler = self._on_mqtt_disconnected + self.transport.on_mqtt_connection_dropped_handler = self._on_mqtt_connection_dropped self.transport.on_mqtt_message_received_handler = self._on_mqtt_message_received - # There can only be one pending connection operation (Connect, Disconnect) - # at a time. The existing one must be completed or canceled before a new one is set. - - # Currently, this means that if, say, a connect operation is the pending op and is executed - # but another connection op is begins by the time the CONNACK is received, the original - # operation will be cancelled, but the CONNACK for it will still be received, and complete the - # NEW operation. This is not desirable, but it is how things currently work. - - # We are however, checking the type, so the CONNACK from a cancelled Connect, cannot successfully - # complete a Disconnect operation. - - # Note that a ReauthorizeConnectionOperation will never be pending because it will - # instead spawn separate Connect and Disconnect operations. + # Only one ConnectOperation or DisconnectOperation can be pending. Reauthorization + # sequences worker operations and is never stored here directly. self._pending_connection_op = None op.complete() @@ -193,9 +114,8 @@ def _run_op(self, op): elif isinstance(op, pipeline_ops_base.ConnectOperation): logger.debug("{}({}): connecting".format(self.name, op.name)) - self._cancel_pending_connection_op() + self._fail_pending_connection_op() self._pending_connection_op = op - self._start_connection_watchdog(op) # Use SasToken as password if present. If not present (e.g. using X509), # then no password is required because auth is handled via other means. if self.nucleus.pipeline_configuration.sastoken: @@ -204,31 +124,57 @@ def _run_op(self, op): password = None try: self.transport.connect(password=password) + except transport_exceptions.ConnectionTimeoutError as e: + logger.info("transport.connect timed out") + logger.info("{}: MQTT connection failed: {}".format(self.name, e)) + logger.debug("{}: failing connect op".format(self.name)) + self._pending_connection_op = None + timeout_error = pipeline_exceptions.OperationTimeout( + "Transport timeout on connection operation" + ) + timeout_error.__cause__ = e + op.complete(error=timeout_error) except Exception as e: logger.info("transport.connect raised error") logger.info(traceback.format_exc()) - self._cancel_connection_watchdog(op) + logger.info("{}: MQTT connection failed: {}".format(self.name, e)) + logger.debug("{}: failing connect op".format(self.name)) self._pending_connection_op = None op.complete(error=e) + else: + logger.info("{}: MQTT connected".format(self.name)) + self.send_event_up(pipeline_events_base.ConnectedEvent()) + logger.debug("{}: completing connect op".format(self.name)) + self._pending_connection_op = None + op.complete() elif isinstance(op, pipeline_ops_base.DisconnectOperation): logger.debug("{}({}): disconnecting".format(self.name, op.name)) - self._cancel_pending_connection_op() + self._fail_pending_connection_op() self._pending_connection_op = op - # We don't need a watchdog on disconnect because there's no callback to wait for - # and we respond to a watchdog timeout by calling disconnect, which is what we're - # already doing. + + @pipeline_thread.invoke_on_pipeline_thread_deferred + def on_disconnect_returned(): + if self._pending_connection_op is not op: + return + + # disconnect() blocks until Paho's network thread exits. If Paho emitted + # an unexpected-drop callback, it was queued first and consumed this + # operation. Otherwise, complete the explicit disconnection path now. + logger.info("{}: MQTT disconnected".format(self.name)) + self.send_event_up(pipeline_events_base.DisconnectedEvent()) + self._complete_pending_connection_op_after_disconnect() try: - # The connect after the disconnect will be triggered upon completion of the - # disconnect in the on_disconnected handler self.transport.disconnect(clear_inflight=op.hard) except Exception as e: logger.info("transport.disconnect raised error while disconnecting") logger.info(traceback.format_exc()) self._pending_connection_op = None op.complete(error=e) + else: + on_disconnect_returned() elif isinstance(op, pipeline_ops_base.ReauthorizeConnectionOperation): logger.debug( @@ -236,23 +182,25 @@ def _run_op(self, op): self.name, op.name ) ) - self_weakref = weakref.ref(self) - reauth_op = op # rename for clarity + stage_weakref = weakref.ref(self) + reauthorization_op = op - def on_disconnect_complete(op, error): - this = self_weakref() + def on_reauthorization_disconnect_complete(op, error): + stage = stage_weakref() if error: # Failing a disconnect should still get us disconnected, so can proceed anyway logger.debug( "Disconnect failed during reauthorization, continuing with connect" ) - connect_op = reauth_op.spawn_worker_op(pipeline_ops_base.ConnectOperation) + connect_op = reauthorization_op.spawn_worker_op(pipeline_ops_base.ConnectOperation) # NOTE: this relies on the fact that before the disconnect is completed it is # unset as the pending connection op. Otherwise there would be issues here. - this.run_op(connect_op) + stage.run_op(connect_op) - disconnect_op = pipeline_ops_base.DisconnectOperation(callback=on_disconnect_complete) + disconnect_op = pipeline_ops_base.DisconnectOperation( + callback=on_reauthorization_disconnect_complete + ) disconnect_op.hard = False self.run_op(disconnect_op) @@ -261,7 +209,7 @@ def on_disconnect_complete(op, error): logger.debug("{}({}): publishing on {}".format(self.name, op.name, op.topic)) @pipeline_thread.invoke_on_pipeline_thread_nowait - def on_complete(cancelled=False): + def on_publish_complete(cancelled=False): if cancelled: op.complete( error=pipeline_exceptions.OperationCancelled( @@ -275,7 +223,9 @@ def on_complete(cancelled=False): op.complete() try: - self.transport.publish(topic=op.topic, payload=op.payload, callback=on_complete) + self.transport.publish( + topic=op.topic, payload=op.payload, callback=on_publish_complete + ) except Exception as e: op.complete(error=e) @@ -283,13 +233,15 @@ def on_complete(cancelled=False): logger.debug("{}({}): subscribing to {}".format(self.name, op.name, op.topic)) @pipeline_thread.invoke_on_pipeline_thread_nowait - def on_complete(cancelled=False): + def on_subscribe_complete(cancelled=False, error=None): if cancelled: op.complete( error=pipeline_exceptions.OperationCancelled( "Operation cancelled before SUBACK received" ) ) + elif error is not None: + op.complete(error=error) else: logger.debug( "{}({}): SUBACK received. completing op.".format(self.name, op.name) @@ -297,7 +249,7 @@ def on_complete(cancelled=False): op.complete() try: - self.transport.subscribe(topic=op.topic, callback=on_complete) + self.transport.subscribe(topic=op.topic, callback=on_subscribe_complete) except Exception as e: op.complete(error=e) @@ -305,7 +257,7 @@ def on_complete(cancelled=False): logger.debug("{}({}): unsubscribing from {}".format(self.name, op.name, op.topic)) @pipeline_thread.invoke_on_pipeline_thread_nowait - def on_complete(cancelled=False): + def on_unsubscribe_complete(cancelled=False): if cancelled: op.complete( error=pipeline_exceptions.OperationCancelled( @@ -319,7 +271,7 @@ def on_complete(cancelled=False): op.complete() try: - self.transport.unsubscribe(topic=op.topic, callback=on_complete) + self.transport.unsubscribe(topic=op.topic, callback=on_unsubscribe_complete) except Exception as e: op.complete(error=e) @@ -331,7 +283,7 @@ def on_complete(cancelled=False): @pipeline_thread.invoke_on_pipeline_thread_nowait def _on_mqtt_message_received(self, topic, payload): """ - Handler that gets called by the protocol library when an incoming message arrives. + Handler that gets called by the transport when an incoming message arrives. Convert that message into a pipeline event and pass it up for someone to handle. """ logger.debug("{}: message received on topic {}".format(self.name, topic)) @@ -340,125 +292,75 @@ def _on_mqtt_message_received(self, topic, payload): ) @pipeline_thread.invoke_on_pipeline_thread_nowait - def _on_mqtt_connected(self): - """ - Handler that gets called by the transport when it connects. - """ - logger.info("_on_mqtt_connected called") - # Send an event to tell other pipeline stages that we're connected. Do this before - # we do anything else (in case upper stages have any "are we connected" logic. - self.send_event_up(pipeline_events_base.ConnectedEvent()) - - if isinstance(self._pending_connection_op, pipeline_ops_base.ConnectOperation): - logger.debug("{}: completing connect op".format(self.name)) - op = self._pending_connection_op - self._cancel_connection_watchdog(op) - self._pending_connection_op = None - op.complete() - else: - # This should indicate something odd is going on. - # If this occurs, either a connect was completed while there was no pending op, - # OR that a connect was completed while a disconnect op was pending - logger.info( - "{}: Connection was unexpected (no connection op pending)".format(self.name) - ) - - @pipeline_thread.invoke_on_pipeline_thread_nowait - def _on_mqtt_connection_failure(self, cause): - """ - Handler that gets called by the transport when a connection fails. - - :param Exception cause: The Exception that caused the connection failure. - """ + def _on_mqtt_connection_dropped(self, cause): + """Handle a transport-reported unexpected connection loss.""" + logger.info("{}: MQTT connection dropped unexpectedly: {}".format(self.name, cause)) + self.send_event_up(pipeline_events_base.DisconnectedEvent()) + try: + self._reconcile_mqtt_operation_tracking_after_connection_drop() - logger.info("{}: _on_mqtt_connection_failure called: {}".format(self.name, cause)) + # Higher layers will see that we're disconnected and may reconnect as necessary. + error = transport_exceptions.ConnectionDroppedError("Unexpected disconnection") + error.__cause__ = cause + self.report_background_exception(error) + finally: + # Completion callbacks can synchronously start work on a replacement connection. + self._complete_pending_connection_op_after_disconnect(cause) - if isinstance(self._pending_connection_op, pipeline_ops_base.ConnectOperation): - logger.debug("{}: failing connect op".format(self.name)) - op = self._pending_connection_op - self._cancel_connection_watchdog(op) - self._pending_connection_op = None - op.complete(error=cause) - else: - logger.debug("{}: Connection failure was unexpected".format(self.name)) - handle_exceptions.swallow_unraised_exception( - cause, - log_msg="Unexpected connection failure (no pending operation). Safe to ignore.", - log_lvl="info", - ) + @pipeline_thread.runs_on_pipeline_thread + def _reconcile_mqtt_operation_tracking_after_connection_drop(self): + """Reconcile MQTT operation tracking with the connection recovery policy. - @pipeline_thread.invoke_on_pipeline_thread_nowait - def _on_mqtt_disconnected(self, cause=None): + This cannot be encapsulated inside the MQTTTransport because it has to do with + connection_retry policy. """ - Handler that gets called by the transport when the transport disconnects. - :param Exception cause: The Exception that caused the disconnection, if any (optional) - """ - if cause: - logger.info("{}: _on_mqtt_disconnect called: {}".format(self.name, cause)) + # If there is no connection retry, complete tracked MQTT operations as cancelled so + # they do not remain pending indefinitely. + if not self.nucleus.pipeline_configuration.connection_retry: + logger.debug( + "{}: Connection Retry disabled - completing tracked MQTT operations as cancelled".format( + self.name + ) + ) + # TODO: Remove private access to the op manager (this layer shouldn't know about it) + # This is a stopgap. I didn't want to invest too much infrastructure into a cancel flow + # given that future development of individual operation cancels might affect the + # approach to completing tracked transport operations as cancelled. + self.transport._op_manager.complete_all_tracked_operations_as_cancelled() else: - logger.info("{}: _on_mqtt_disconnect called".format(self.name)) - - # Send an event to tell other pipeline stages that we're disconnected. Do this before - # we do anything else (in case upper stages have any "are we connected" logic.) - # NOTE: Other stages rely on the fact that this occurs before any op that may be in - # progress is completed. Be careful with changing the order things occur here. - self.send_event_up(pipeline_events_base.DisconnectedEvent()) + logger.debug( + "{}: Connection Retry enabled - preserving resumable PUBLISH tracking and completing non-resumable MQTT operations as cancelled".format( + self.name + ) + ) + self.transport._op_manager.complete_non_resumable_operations_as_cancelled() + @pipeline_thread.runs_on_pipeline_thread + def _complete_pending_connection_op_after_disconnect(self, cause=None): + """Complete a pending connection operation after disconnection effects are applied.""" if self._pending_connection_op: - op = self._pending_connection_op + connection_op = self._pending_connection_op - if isinstance(op, pipeline_ops_base.DisconnectOperation): - logger.debug( - "{}: Expected disconnect - completing pending disconnect op".format(self.name) - ) - # Swallow any errors if we intended to disconnect - even if something went wrong, we - # got to the state we wanted to be in! - if cause: - handle_exceptions.swallow_unraised_exception( - cause, - log_msg="Unexpected error while disconnecting - swallowing error", - ) + if isinstance(connection_op, pipeline_ops_base.DisconnectOperation): + logger.debug("{}: Completing pending disconnect op".format(self.name)) # Disconnect complete, no longer pending self._pending_connection_op = None - op.complete() + connection_op.complete() else: logger.debug( - "{}: Unexpected disconnect - completing pending {} operation".format( - self.name, op.name + "{}: Completing pending {} after disconnection".format( + self.name, connection_op.name ) ) - # Cancel any potential connection watchdog, and clear the pending op - self._cancel_connection_watchdog(op) + # Clear and complete the pending operation. self._pending_connection_op = None # Complete if cause: - op.complete(error=cause) + connection_op.complete(error=cause) else: - op.complete( + connection_op.complete( error=transport_exceptions.ConnectionDroppedError("transport disconnected") ) - else: - logger.info("{}: Unexpected disconnect (no pending connection op)".format(self.name)) - - # If there is no connection retry, cancel any transport operations waiting on response - # so that they do not get stuck there. - if not self.nucleus.pipeline_configuration.connection_retry: - logger.debug( - "{}: Connection Retry disabled - cancelling in-flight operations".format( - self.name - ) - ) - # TODO: Remove private access to the op manager (this layer shouldn't know about it) - # This is a stopgap. I didn't want to invest too much infrastructure into a cancel flow - # given that future development of individual operation cancels might affect the - # approach to cancelling inflight ops waiting in the transport. - self.transport._op_manager.cancel_all_operations() - - # Regardless of cause, it is now a ConnectionDroppedError. Log it and swallow it. - # Higher layers will see that we're disconnected and may reconnect as necessary. - e = transport_exceptions.ConnectionDroppedError("Unexpected disconnection") - e.__cause__ = cause - self.report_background_exception(e) diff --git a/azure-iot-device/azure/iot/device/common/pipeline/pipeline_thread.py b/azure-iot-device/azure/iot/device/common/pipeline/pipeline_thread.py index e431b3681..a1e6fea6b 100644 --- a/azure-iot-device/azure/iot/device/common/pipeline/pipeline_thread.py +++ b/azure-iot-device/azure/iot/device/common/pipeline/pipeline_thread.py @@ -16,11 +16,13 @@ This module contains decorators that are used to marshal code into pipeline and callback threads and to assert that code is being called in the correct thread. +The `invoke_on_pipeline_thread`, `invoke_on_pipeline_thread_nowait`, and +`invoke_on_pipeline_thread_deferred` decorators cause decorated functions to run on +the pipeline thread. + The intention of these decorators is to ensure the following: -1. All pipeline functions execute in a single thread, known as the "pipeline - thread". The `invoke_on_pipeline_thread` and `invoke_on_pipeline_thread_nowait` - decorators cause the decorated function to run on the pipeline thread. +1. All pipeline functions execute in a single thread, known as the "pipeline thread". 2. If the pipeline thread is busy running a different function, the invoke decorators will wait until that function is complete before invoking another @@ -84,11 +86,13 @@ def _get_named_executor(thread_name): return _executors[thread_name] -def _invoke_on_executor_thread(func, thread_name, block=True): +def _invoke_on_executor_thread(func, thread_name, block=True, always_queue=False): """ Return wrapper to run the function on a given thread. If block==False, the call returns immediately without waiting for the decorated function to complete. If block==True, the call waits for the decorated function to complete before returning. + If always_queue==True, the function is submitted even when called from the target thread. + The block argument still determines whether the caller waits for the submitted function. """ # Mocks and other callable objects don't have a __name__ attribute. @@ -100,7 +104,7 @@ def _invoke_on_executor_thread(func, thread_name, block=True): @functools.wraps(func) def wrapper(*args, **kwargs): - if threading.current_thread().name is not thread_name: + if always_queue or threading.current_thread().name is not thread_name: logger.debug("Starting {} in {} thread".format(function_name, thread_name)) def thread_proc(): @@ -153,6 +157,16 @@ def invoke_on_pipeline_thread_nowait(func): return _invoke_on_executor_thread(func=func, thread_name="pipeline", block=False) +def invoke_on_pipeline_thread_deferred(func): + """ + Queue the decorated function for later execution on the pipeline thread, even if it is already + on the pipeline thread. Do not wait for it to complete. + """ + return _invoke_on_executor_thread( + func=func, thread_name="pipeline", block=False, always_queue=True + ) + + def invoke_on_callback_thread_nowait(func): """ Run the decorated function on the callback thread, but don't wait for it to complete @@ -178,9 +192,7 @@ def _assert_executor_thread(func, thread_name): @functools.wraps(func) def wrapper(*args, **kwargs): - assert ( - threading.current_thread().name == thread_name - ), """ + assert threading.current_thread().name == thread_name, """ Function {function_name} is not running inside {thread_name} thread. It should be. You should use invoke_on_{thread_name}_thread(_nowait) to enter the {thread_name} thread before calling this function. If you're hitting this from diff --git a/azure-iot-device/azure/iot/device/common/transport_exceptions.py b/azure-iot-device/azure/iot/device/common/transport_exceptions.py index 1c572ec59..bd22d4270 100644 --- a/azure-iot-device/azure/iot/device/common/transport_exceptions.py +++ b/azure-iot-device/azure/iot/device/common/transport_exceptions.py @@ -14,6 +14,12 @@ class ConnectionFailedError(Exception): pass +class ConnectionTimeoutError(ConnectionFailedError): + """Connection was not established before the timeout expired.""" + + pass + + class ConnectionDroppedError(Exception): """ Previously established connection was dropped diff --git a/pyproject.toml b/pyproject.toml index 1da3dc173..d36ed8f6d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,7 +28,7 @@ classifiers = [ dependencies = [ "deprecation>=2.1.0,<3.0.0", "janus>=2.0.0,<3.0.0", - "paho-mqtt>=2.0.0,<3.0.0", + "paho-mqtt>=2.1.0,<3.0.0", "PySocks", "requests>=2.32.3,<3.0.0", "requests-unixsocket>=0.4.1", diff --git a/tests/e2e/iothub_e2e/aio/test_infrastructure.py b/tests/e2e/iothub_e2e/aio/test_infrastructure.py index 1587d5c71..691a6f2d4 100644 --- a/tests/e2e/iothub_e2e/aio/test_infrastructure.py +++ b/tests/e2e/iothub_e2e/aio/test_infrastructure.py @@ -9,7 +9,7 @@ class TestServiceHelper(object): @pytest.mark.it("returns None when wait_for_event_arrival times out") async def test_validate_wait_for_eventhub_arrival_timeout( - self, client, random_message, service_helper + self, client, random_message, service_helper, leak_tracker ): # Because we have to support py27, we can't use `threading.Condition.wait_for`. # make sure our stand-in functionality behaves the same way when dealing with diff --git a/tests/e2e/iothub_e2e/aio/test_send_message.py b/tests/e2e/iothub_e2e/aio/test_send_message.py index 7450f8754..62e37521f 100644 --- a/tests/e2e/iothub_e2e/aio/test_send_message.py +++ b/tests/e2e/iothub_e2e/aio/test_send_message.py @@ -205,8 +205,9 @@ async def test_connects_after_automatic_disconnect_retry_disabled( @pytest.mark.it("Fails if connection disconnects before sending") @pytest.mark.uses_iptables - # TODO: Re-enable leak tracking after the MQTT cancellation refactor. - async def test_fails_if_disconnect_before_sending(self, client, random_message, dropper): + async def test_fails_if_disconnect_before_sending( + self, client, random_message, dropper, service_helper, leak_tracker + ): assert client.connected @@ -220,11 +221,18 @@ async def test_fails_if_disconnect_before_sending(self, client, random_message, with pytest.raises(OperationCancelled): await asyncio.wait_for(send_task, timeout=const.E2E_TIMEOUT) + # ----------------------------------------------------------------------------------------- + # The SDK operation is cancelled, but Paho still owns the accepted QoS publish. Reconnect + # and let the MQTT exchange finish so the normal leak check sees no active session state. + dropper.restore_all() + await client.connect() + event = await service_helper.wait_for_eventhub_arrival(random_message.message_id) + assert json.dumps(event.message_body) == random_message.data + @pytest.mark.it("Fails if connection drops before sending") @pytest.mark.uses_iptables - # TODO: Re-enable leak tracking after the MQTT cancellation refactor. async def test_fails_if_drop_before_sending_retry_disabled( - self, client, random_message, dropper + self, client, random_message, dropper, service_helper, leak_tracker ): assert client.connected @@ -234,3 +242,11 @@ async def test_fails_if_drop_before_sending_retry_disabled( await client.send_message(random_message) assert not client.connected + + # ----------------------------------------------------------------------------------------- + # The SDK operation is cancelled, but Paho still owns the accepted QoS publish. Reconnect + # and let the MQTT exchange finish so the normal leak check sees no active session state. + dropper.restore_all() + await client.connect() + event = await service_helper.wait_for_eventhub_arrival(random_message.message_id) + assert json.dumps(event.message_body) == random_message.data diff --git a/tests/e2e/iothub_e2e/aio/test_twin.py b/tests/e2e/iothub_e2e/aio/test_twin.py index bef8a03a2..a368a2fe5 100644 --- a/tests/e2e/iothub_e2e/aio/test_twin.py +++ b/tests/e2e/iothub_e2e/aio/test_twin.py @@ -107,9 +107,8 @@ class TestReportedPropertiesDroppedConnection(object): # TODO: split drop tests between first and second patches @pytest.mark.it("Updates reported properties if connection drops before sending") - # TODO: Re-enable leak tracking after the MQTT cancellation refactor. async def test_updates_reported_if_drop_before_sending( - self, client, random_reported_props, dropper, service_helper + self, client, random_reported_props, dropper, service_helper, leak_tracker ): assert client.connected @@ -138,9 +137,8 @@ async def test_updates_reported_if_drop_before_sending( ) @pytest.mark.it("Updates reported properties if connection rejects send") - # TODO: Re-enable leak tracking after the MQTT cancellation refactor. async def test_updates_reported_if_reject_before_sending( - self, client, random_reported_props, dropper, service_helper + self, client, random_reported_props, dropper, service_helper, leak_tracker ): assert client.connected diff --git a/tests/e2e/iothub_e2e/sync/test_sync_infrastructure.py b/tests/e2e/iothub_e2e/sync/test_sync_infrastructure.py index 0919ad392..e3df018d9 100644 --- a/tests/e2e/iothub_e2e/sync/test_sync_infrastructure.py +++ b/tests/e2e/iothub_e2e/sync/test_sync_infrastructure.py @@ -8,7 +8,9 @@ @pytest.mark.describe("ServiceHelper object") class TestServiceHelper(object): @pytest.mark.it("returns None when wait_for_event_arrival times out") - def test_sync_wait_for_event_arrival(self, client, random_message, service_helper): + def test_sync_wait_for_event_arrival( + self, client, random_message, service_helper, leak_tracker + ): event = service_helper.wait_for_eventhub_arrival(uuid.uuid4(), timeout=2) assert event is None diff --git a/tests/e2e/iothub_e2e/sync/test_sync_send_message.py b/tests/e2e/iothub_e2e/sync/test_sync_send_message.py index 804dc523a..3055fe7f5 100644 --- a/tests/e2e/iothub_e2e/sync/test_sync_send_message.py +++ b/tests/e2e/iothub_e2e/sync/test_sync_send_message.py @@ -193,9 +193,14 @@ def test_sync_connects_after_automatic_disconnect_with_retry_disabled( @pytest.mark.it("Fails if connection disconnects before sending") @pytest.mark.uses_iptables - # TODO: Re-enable leak tracking after the MQTT cancellation refactor. def test_sync_fails_if_disconnect_before_sending_with_retry_disabled( - self, client, random_message, dropper, run_in_daemon_thread + self, + client, + random_message, + dropper, + run_in_daemon_thread, + service_helper, + leak_tracker, ): assert client.connected @@ -208,11 +213,18 @@ def test_sync_fails_if_disconnect_before_sending_with_retry_disabled( with pytest.raises(OperationCancelled): send_task.result(timeout=const.E2E_TIMEOUT) + # ----------------------------------------------------------------------------------------- + # The SDK operation is cancelled, but Paho still owns the accepted QoS publish. Reconnect + # and let the MQTT exchange finish so the normal leak check sees no active session state. + dropper.restore_all() + client.connect() + event = service_helper.wait_for_eventhub_arrival(random_message.message_id) + assert json.dumps(event.message_body) == random_message.data + @pytest.mark.it("Fails if connection drops before sending") @pytest.mark.uses_iptables - # TODO: Re-enable leak tracking after the MQTT cancellation refactor. def test_sync_fails_if_drop_before_sending_with_retry_disabled( - self, client, random_message, dropper + self, client, random_message, dropper, service_helper, leak_tracker ): assert client.connected @@ -222,3 +234,11 @@ def test_sync_fails_if_drop_before_sending_with_retry_disabled( client.send_message(random_message) assert not client.connected + + # ----------------------------------------------------------------------------------------- + # The SDK operation is cancelled, but Paho still owns the accepted QoS publish. Reconnect + # and let the MQTT exchange finish so the normal leak check sees no active session state. + dropper.restore_all() + client.connect() + event = service_helper.wait_for_eventhub_arrival(random_message.message_id) + assert json.dumps(event.message_body) == random_message.data diff --git a/tests/e2e/iothub_e2e/sync/test_sync_twin.py b/tests/e2e/iothub_e2e/sync/test_sync_twin.py index c7815d0ad..af4b7174e 100644 --- a/tests/e2e/iothub_e2e/sync/test_sync_twin.py +++ b/tests/e2e/iothub_e2e/sync/test_sync_twin.py @@ -106,7 +106,6 @@ class TestReportedPropertiesDroppedConnection(object): # TODO: split drop tests between first and second patches @pytest.mark.it("Updates reported properties if connection drops before sending") - # TODO: Re-enable leak tracking after the MQTT cancellation refactor. def test_sync_updates_reported_if_drop_before_sending( self, client, @@ -114,6 +113,7 @@ def test_sync_updates_reported_if_drop_before_sending( dropper, service_helper, run_in_daemon_thread, + leak_tracker, ): assert client.connected @@ -138,7 +138,6 @@ def test_sync_updates_reported_if_drop_before_sending( ) @pytest.mark.it("Updates reported properties if connection rejects send") - # TODO: Re-enable leak tracking after the MQTT cancellation refactor. def test_sync_updates_reported_if_reject_before_sending( self, client, @@ -146,6 +145,7 @@ def test_sync_updates_reported_if_reject_before_sending( dropper, service_helper, run_in_daemon_thread, + leak_tracker, ): assert client.connected diff --git a/tests/unit/common/pipeline/conftest.py b/tests/unit/common/pipeline/conftest.py index d751312ab..33247da6e 100644 --- a/tests/unit/common/pipeline/conftest.py +++ b/tests/unit/common/pipeline/conftest.py @@ -8,6 +8,7 @@ arbitrary_event, arbitrary_op, fake_pipeline_thread, + fake_pipeline_thread_queue, fake_non_pipeline_thread, pipeline_connected_mock, nucleus, diff --git a/tests/unit/common/pipeline/fixtures.py b/tests/unit/common/pipeline/fixtures.py index e02dd253b..dc3503dfc 100644 --- a/tests/unit/common/pipeline/fixtures.py +++ b/tests/unit/common/pipeline/fixtures.py @@ -3,12 +3,14 @@ # Licensed under the MIT License. See License.txt in the project root for # license information. # -------------------------------------------------------------------------- +import concurrent.futures import pytest import threading from azure.iot.device.common.pipeline import ( pipeline_events_base, pipeline_ops_base, pipeline_nucleus, + pipeline_thread, ) @@ -34,6 +36,34 @@ def arbitrary_op(mocker): return op +class FakePipelineThreadQueue(object): + def __init__(self): + self._queued_calls = [] + + def submit(self, call): + future = concurrent.futures.Future() + self._queued_calls.append((call, future)) + return future + + def run_next(self): + call, future = self._queued_calls.pop(0) + if future.set_running_or_notify_cancel(): + try: + result = call() + except BaseException as e: + future.set_exception(e) + else: + future.set_result(result) + return future + + def run_all(self): + while self._queued_calls: + self.run_next() + + def __len__(self): + return len(self._queued_calls) + + @pytest.fixture def pipeline_connected_mock(mocker): """This mock can have it's return value altered by any test to indicate whether or not the @@ -88,6 +118,15 @@ def fake_pipeline_thread(): this_thread.name = old_name +@pytest.fixture +def fake_pipeline_thread_queue(mocker, fake_pipeline_thread): + """Capture work queued for deferred execution on the pipeline thread.""" + thread_queue = FakePipelineThreadQueue() + executor = mocker.patch.object(pipeline_thread, "_get_named_executor").return_value + executor.submit.side_effect = thread_queue.submit + return thread_queue + + @pytest.fixture def fake_non_pipeline_thread(): """ diff --git a/tests/unit/common/pipeline/test_pipeline_ops_base.py b/tests/unit/common/pipeline/test_pipeline_ops_base.py index 1e35351fa..795f05ccf 100644 --- a/tests/unit/common/pipeline/test_pipeline_ops_base.py +++ b/tests/unit/common/pipeline/test_pipeline_ops_base.py @@ -61,18 +61,10 @@ def init_kwargs(self, mocker): return kwargs -class ConnectOperationInstantiationTests(ConnectOperationTestConfig): - @pytest.mark.it("Initializes 'watchdog_timer' attribute to 'None'") - def test_retry_timer(self, cls_type, init_kwargs): - op = cls_type(**init_kwargs) - assert op.watchdog_timer is None - - pipeline_ops_test.add_operation_tests( test_module=this_module, op_class_under_test=pipeline_ops_base.ConnectOperation, op_test_config_class=ConnectOperationTestConfig, - extended_op_instantiation_test_class=ConnectOperationInstantiationTests, ) diff --git a/tests/unit/common/pipeline/test_pipeline_stages_base.py b/tests/unit/common/pipeline/test_pipeline_stages_base.py index 462671e2b..c794a5266 100644 --- a/tests/unit/common/pipeline/test_pipeline_stages_base.py +++ b/tests/unit/common/pipeline/test_pipeline_stages_base.py @@ -2538,6 +2538,10 @@ def test_op_completes_success(self, stage, op): pytest.param( [pipeline_ops_base.DisconnectOperation(callback=None)], id="Single op waiting" ), + pytest.param( + [pipeline_ops_base.ShutdownPipelineOperation(callback=None)], + id="Shutdown waiting", + ), pytest.param( [ pipeline_ops_base.ReauthorizeConnectionOperation(callback=None), diff --git a/tests/unit/common/pipeline/test_pipeline_stages_mqtt.py b/tests/unit/common/pipeline/test_pipeline_stages_mqtt.py index ae65958fb..01f52d047 100644 --- a/tests/unit/common/pipeline/test_pipeline_stages_mqtt.py +++ b/tests/unit/common/pipeline/test_pipeline_stages_mqtt.py @@ -7,7 +7,7 @@ import pytest import sys import threading -from azure.iot.device.common import transport_exceptions, handle_exceptions +from azure.iot.device.common import transport_exceptions from azure.iot.device.common.pipeline import ( pipeline_ops_base, pipeline_ops_mqtt, @@ -32,9 +32,10 @@ @pytest.fixture def mock_transport(mocker): - return mocker.patch( + transport_class = mocker.patch( "azure.iot.device.common.pipeline.pipeline_stages_mqtt.MQTTTransport", autospec=True ) + return transport_class @pytest.fixture @@ -194,10 +195,8 @@ def test_creates_transport( def test_sets_transport_handlers(self, mocker, stage, op, mock_transport): stage.run_op(op) - assert stage.transport.on_mqtt_disconnected_handler == stage._on_mqtt_disconnected - assert stage.transport.on_mqtt_connected_handler == stage._on_mqtt_connected assert ( - stage.transport.on_mqtt_connection_failure_handler == stage._on_mqtt_connection_failure + stage.transport.on_mqtt_connection_dropped_handler == stage._on_mqtt_connection_dropped ) assert stage.transport.on_mqtt_message_received_handler == stage._on_mqtt_message_received @@ -242,6 +241,7 @@ def stage(self, mocker, cls_type, init_kwargs, nucleus, mock_transport): stage.run_op(op) assert stage.transport is mock_transport.return_value + stage.transport._op_manager = mocker.MagicMock() return stage @@ -286,10 +286,13 @@ class TestMQTTTransportStageRunOpCalledWithConnectOperation( def op(self, mocker): return pipeline_ops_base.ConnectOperation(callback=mocker.MagicMock()) - @pytest.mark.it("Sets the operation as the stage's pending connection operation") - def test_sets_pending_operation(self, stage, op): + @pytest.mark.it("Completes the operation after the MQTTTransport connects") + def test_completes_operation(self, stage, op): stage.run_op(op) - assert stage._pending_connection_op is op + + assert op.completed + assert op.error is None + assert stage._pending_connection_op is None @pytest.mark.it("Cancels any already pending connection operation") @pytest.mark.parametrize( @@ -317,17 +320,9 @@ def test_pending_operation_cancelled(self, mocker, stage, op, pending_connection assert pending_connection_op.completed assert type(pending_connection_op.error) is pipeline_exceptions.OperationCancelled - # New operation is now the pending operation - assert stage._pending_connection_op is op - - @pytest.mark.it("Starts the connection watchdog") - def test_starts_watchdog(self, mocker, stage, op, mock_timer): - stage.run_op(op) - - assert mock_timer.call_count == 1 - assert mock_timer.call_args == mocker.call(60, mocker.ANY) - assert mock_timer.return_value.daemon is True - assert mock_timer.return_value.start.call_count == 1 + assert op.completed + assert op.error is None + assert stage._pending_connection_op is None @pytest.mark.it( "Performs an MQTT connect via the MQTTTransport, using the PipelineNucleus' SasToken as a password, if using SAS-based authentication" @@ -350,6 +345,15 @@ def test_mqtt_connect_no_sastoken(self, mocker, stage, op): assert stage.transport.connect.call_count == 1 assert stage.transport.connect.call_args == mocker.call(password=None) + @pytest.mark.it("Sends a ConnectedEvent before completing the operation") + def test_sends_connected_event(self, stage, op): + stage.run_op(op) + + assert stage.send_event_up.call_count == 1 + assert isinstance( + stage.send_event_up.call_args.args[0], pipeline_events_base.ConnectedEvent + ) + @pytest.mark.it( "Completes the operation unsuccessfully if there is a failure connecting via the MQTTTransport, using the error raised by the MQTTTransport" ) @@ -367,22 +371,20 @@ def test_clears_pending_op_on_failure(self, mocker, stage, op, arbitrary_excepti stage.run_op(op) assert stage._pending_connection_op is None - @pytest.mark.it( - "Leaves the watchdog running while waiting for the connect operation to complete" - ) - def test_leaves_watchdog_running(self, mocker, stage, op, arbitrary_exception, mock_timer): - stage.run_op(op) - assert mock_timer.return_value.cancel.call_count == 0 - assert op.watchdog_timer is mock_timer.return_value + @pytest.mark.it("Maps an MQTTTransport connection timeout to OperationTimeout") + def test_connection_timeout(self, stage, op): + transport_error = transport_exceptions.ConnectionTimeoutError( + "Timed out waiting for MQTT CONNACK" + ) + stage.transport.connect.side_effect = transport_error - @pytest.mark.it( - "Cancels the connection watchdog if the MQTTTransport connect operation raises an exception" - ) - def test_cancels_watchdog(self, mocker, stage, op, arbitrary_exception, mock_timer): - stage.transport.connect.side_effect = arbitrary_exception stage.run_op(op) - assert mock_timer.return_value.cancel.call_count == 1 - assert op.watchdog_timer is None + + assert op.completed + assert isinstance(op.error, pipeline_exceptions.OperationTimeout) + assert op.error.__cause__ is transport_error + assert stage._pending_connection_op is None + assert stage.send_event_up.call_count == 0 @pytest.mark.describe( @@ -498,10 +500,63 @@ class TestMQTTTransportStageRunOpCalledWithDisconnectOperation( def op(self, mocker): return pipeline_ops_base.DisconnectOperation(callback=mocker.MagicMock()) - @pytest.mark.it("Sets the operation as the stage's pending connection operation") - def test_sets_pending_operation(self, stage, op): + @pytest.mark.it("Completes the operation when MQTTTransport.disconnect() returns") + def test_completes_operation(self, stage, op, fake_pipeline_thread_queue): + stage.run_op(op) + fake_pipeline_thread_queue.run_next() + + assert op.completed + assert op.error is None + assert stage._pending_connection_op is None + + @pytest.mark.it("Reports disconnection if the MQTTTransport returns without a callback") + def test_transport_returns_without_callback(self, stage, op, fake_pipeline_thread_queue): + stage.transport.disconnect.side_effect = None + + stage.run_op(op) + fake_pipeline_thread_queue.run_next() + + assert op.completed + assert op.error is None + assert stage._pending_connection_op is None + assert stage.send_event_up.call_count == 1 + + @pytest.mark.it("Reports disconnection once when callback precedes transport return") + def test_callback_precedes_transport_return_fallback( + self, mocker, stage, fake_pipeline_thread_queue + ): + disconnect_callback = mocker.MagicMock() + op = pipeline_ops_base.DisconnectOperation(callback=disconnect_callback) + + def report_drop(**kwargs): + current_thread = threading.current_thread() + original_thread_name = current_thread.name + current_thread.name = "paho" + try: + stage.transport.on_mqtt_connection_dropped_handler( + transport_exceptions.ConnectionDroppedError("connection dropped") + ) + finally: + current_thread.name = original_thread_name + + stage.transport.disconnect.side_effect = report_drop + stage.run_op(op) - assert stage._pending_connection_op is op + + assert len(fake_pipeline_thread_queue) == 2 + assert not op.completed + + fake_pipeline_thread_queue.run_all() + + assert op.completed + assert op.error is None + assert disconnect_callback.call_count == 1 + disconnected_events = [ + call.args[0] + for call in stage.send_event_up.call_args_list + if isinstance(call.args[0], pipeline_events_base.DisconnectedEvent) + ] + assert len(disconnected_events) == 1 @pytest.mark.it("Cancels any already pending connection operation") @pytest.mark.parametrize( @@ -517,7 +572,9 @@ def test_sets_pending_operation(self, stage, op): ), ], ) - def test_pending_operation_cancelled(self, mocker, stage, op, pending_connection_op): + def test_pending_operation_cancelled( + self, mocker, stage, op, pending_connection_op, fake_pipeline_thread_queue + ): # Set up a pending op stage._pending_connection_op = pending_connection_op assert not pending_connection_op.completed @@ -529,26 +586,70 @@ def test_pending_operation_cancelled(self, mocker, stage, op, pending_connection assert pending_connection_op.completed assert type(pending_connection_op.error) is pipeline_exceptions.OperationCancelled - # New operation is now the pending operation - assert stage._pending_connection_op is op + # The new disconnect operation completed after the transport returned. + fake_pipeline_thread_queue.run_next() + assert op.completed + assert op.error is None + assert stage._pending_connection_op is None @pytest.mark.it( "Performs an MQTT disconnect via the MQTTTransport, using the 'clear_inflight' option only if the operation is configured for a hard disconnect" ) - def test_mqtt_connect(self, mocker, stage, op): + def test_mqtt_disconnect(self, mocker, stage, op, fake_pipeline_thread_queue): # Hard disconnect assert op.hard is True stage.run_op(op) assert stage.transport.disconnect.call_count == 1 assert stage.transport.disconnect.call_args == mocker.call(clear_inflight=True) + fake_pipeline_thread_queue.run_next() stage.transport.disconnect.reset_mock() # Soft disconnect - op.hard = False - stage.run_op(op) + soft_op = pipeline_ops_base.DisconnectOperation(callback=mocker.MagicMock()) + soft_op.hard = False + stage.run_op(soft_op) assert stage.transport.disconnect.call_count == 1 assert stage.transport.disconnect.call_args == mocker.call(clear_inflight=False) + fake_pipeline_thread_queue.run_next() + + @pytest.mark.it("Sends a DisconnectedEvent when MQTTTransport.disconnect() returns") + def test_sends_disconnected_event(self, stage, op, fake_pipeline_thread_queue): + stage.run_op(op) + fake_pipeline_thread_queue.run_next() + + assert stage.send_event_up.call_count == 1 + assert isinstance( + stage.send_event_up.call_args.args[0], pipeline_events_base.DisconnectedEvent + ) + + @pytest.mark.it("Sends a DisconnectedEvent before completing the operation") + def test_sends_disconnected_event_before_completing( + self, mocker, stage, fake_pipeline_thread_queue + ): + def on_complete(op, error): + assert stage.send_event_up.call_count == 1 + + op = pipeline_ops_base.DisconnectOperation(callback=on_complete) + + stage.run_op(op) + fake_pipeline_thread_queue.run_next() + + assert op.completed + + @pytest.mark.it("Does not apply unexpected-drop handling") + def test_does_not_apply_connection_drop_handling(self, stage, op, fake_pipeline_thread_queue): + stage.run_op(op) + fake_pipeline_thread_queue.run_next() + + assert ( + stage.transport._op_manager.complete_all_tracked_operations_as_cancelled.call_count == 0 + ) + assert ( + stage.transport._op_manager.complete_non_resumable_operations_as_cancelled.call_count + == 0 + ) + assert stage.report_background_exception.call_count == 0 @pytest.mark.it( "Completes the operation unsuccessfully if there is a failure disconnecting via the MQTTTransport, using the error raised by the MQTTTransport" @@ -559,6 +660,15 @@ def test_fails_operation(self, mocker, stage, op, arbitrary_exception): assert op.completed assert op.error is arbitrary_exception + @pytest.mark.it("Allows any BaseExceptions raised by MQTTTransport to propagate") + def test_base_exception_propagates(self, stage, op, arbitrary_base_exception): + stage.transport.disconnect.side_effect = arbitrary_base_exception + + with pytest.raises(type(arbitrary_base_exception)) as e_info: + stage.run_op(op) + + assert e_info.value is arbitrary_base_exception + @pytest.mark.it( "Resets the stage's pending connection operation to None, if there is a failure disconnecting via the MQTTTransport" ) @@ -602,7 +712,7 @@ def test_complete(self, mocker, stage, op): assert op.error is None @pytest.mark.it( - "Completes the operation with an OperationCancelled error upon cancellation of the MQTT unsubscribe by the MQTTTransport" + "Completes the operation with an OperationCancelled error when the MQTTTransport reports publish cancellation" ) def test_complete_with_cancel(self, mocker, stage, op): # Begin publish @@ -639,7 +749,7 @@ def op(self, mocker): ) @pytest.mark.it("Performs an MQTT subscribe via the MQTTTransport") - def test_mqtt_publish(self, mocker, stage, op): + def test_mqtt_subscribe(self, mocker, stage, op): stage.run_op(op) assert stage.transport.subscribe.call_count == 1 assert stage.transport.subscribe.call_args == mocker.call( @@ -662,10 +772,23 @@ def test_complete(self, mocker, stage, op): assert op.error is None @pytest.mark.it( - "Completes the operation with an OperationCancelled error upon cancellation of the MQTT unsubscribe by the MQTTTransport" + "Completes the operation with an error received from the MQTT subscribe callback" + ) + def test_complete_with_error(self, stage, op, arbitrary_exception): + stage.run_op(op) + + assert not op.completed + + stage.transport.subscribe.call_args[1]["callback"](error=arbitrary_exception) + + assert op.completed + assert op.error is arbitrary_exception + + @pytest.mark.it( + "Completes the operation with an OperationCancelled error when the MQTTTransport reports subscribe cancellation" ) def test_complete_with_cancel(self, mocker, stage, op): - # Begin unsubscribe + # Begin subscribe stage.run_op(op) assert not op.completed @@ -699,7 +822,7 @@ def op(self, mocker): ) @pytest.mark.it("Performs an MQTT unsubscribe via the MQTTTransport") - def test_mqtt_publish(self, mocker, stage, op): + def test_mqtt_unsubscribe(self, mocker, stage, op): stage.run_op(op) assert stage.transport.unsubscribe.call_count == 1 assert stage.transport.unsubscribe.call_args == mocker.call( @@ -722,7 +845,7 @@ def test_complete(self, mocker, stage, op): assert op.error is None @pytest.mark.it( - "Completes the operation with an OperationCancelled error upon cancellation of the MQTT unsubscribe by the MQTTTransport" + "Completes the operation with an OperationCancelled error when the MQTTTransport reports unsubscribe cancellation" ) def test_complete_with_cancel(self, mocker, stage, op): # Begin unsubscribe @@ -739,7 +862,7 @@ def test_complete_with_cancel(self, mocker, stage, op): @pytest.mark.it( "Completes the operation using the exception that was raised, if an exception was raised from the MQTTTransport" ) - def test_publish_error(self, stage, op, arbitrary_exception): + def test_unsubscribe_error(self, stage, op, arbitrary_exception): stage.transport.unsubscribe.side_effect = arbitrary_exception stage.run_op(op) @@ -789,320 +912,146 @@ def test_verify_incoming_message_attributes(self, stage, mocker): assert event.topic == fake_topic -@pytest.mark.describe("MQTTTransportStage - OCCURRENCE: MQTT connected") -class TestMQTTTransportStageOnConnected(MQTTTransportStageTestConfigComplex): - @pytest.mark.it("Sends a ConnectedEvent up the pipeline") - @pytest.mark.parametrize( - "pending_connection_op", - [ - pytest.param(None, id="No pending operation"), - pytest.param( - pipeline_ops_base.ConnectOperation(callback=fake_callback), - id="Pending ConnectOperation", - ), - pytest.param( - pipeline_ops_base.ReauthorizeConnectionOperation(callback=fake_callback), - id="Pending ReauthorizeConnectionOperation", - ), - pytest.param( - pipeline_ops_base.DisconnectOperation(callback=fake_callback), - id="Pending DisconnectOperation", - ), - ], - ) - def test_sends_event_up(self, stage, pending_connection_op): - stage._pending_connection_op = pending_connection_op - # Trigger connect completion - stage.transport.on_mqtt_connected_handler() - - assert stage.send_event_up.call_count == 1 - connect_event = stage.send_event_up.call_args[0][0] - assert isinstance(connect_event, pipeline_events_base.ConnectedEvent) - - @pytest.mark.it("Completes a pending ConnectOperation successfully") - def test_completes_pending_connect_op(self, mocker, stage): - # Set a pending connect operation - op = pipeline_ops_base.ConnectOperation(callback=mocker.MagicMock()) - stage.run_op(op) - assert not op.completed - assert stage._pending_connection_op is op - - # Trigger connect completion - stage.transport.on_mqtt_connected_handler() - - # Connect operation completed successfully - assert op.completed - assert op.error is None - assert stage._pending_connection_op is None - - @pytest.mark.it( - "Does not complete a pending DisconnectOperation when the transport connected event fires" - ) - def test_does_not_complete_pending_disconnect_op(self, mocker, stage): - # Set a pending disconnect operation - op = pipeline_ops_base.DisconnectOperation(callback=mocker.MagicMock()) - stage.run_op(op) - assert not op.completed - assert stage._pending_connection_op is op - - # Trigger connect completion - stage.transport.on_mqtt_connected_handler() - - # Disconnect operation was NOT completed - assert not op.completed - assert stage._pending_connection_op is op - - @pytest.mark.it( - "Cancels the connection watchdog if the pending operation is a ConnectOperation" - ) - def test_cancels_watchdog_on_pending_connect(self, mocker, stage, mock_timer): - # Set a pending connect operation - op = pipeline_ops_base.ConnectOperation(callback=mocker.MagicMock()) - stage.run_op(op) +@pytest.mark.describe( + "MQTTTransportStage - OCCURRENCE: MQTT connection dropped with pending DisconnectOperation" +) +class TestMQTTTransportStageOnConnectionDroppedWithPendingDisconnectOperation( + MQTTTransportStageTestConfigComplex +): + @pytest.fixture + def pending_connection_op(self): + return pipeline_ops_base.DisconnectOperation(callback=fake_callback) - # assert watchdog is running - assert op.watchdog_timer is mock_timer.return_value - assert op.watchdog_timer.start.call_count == 1 + @pytest.mark.it("Sends a DisconnectedEvent up the pipeline") + def test_disconnect_event_sent(self, stage, arbitrary_exception, pending_connection_op): + stage._pending_connection_op = pending_connection_op + assert stage.send_event_up.call_count == 0 - # Trigger connect completion - stage.transport.on_mqtt_connected_handler() + # Trigger disconnect + stage.transport.on_mqtt_connection_dropped_handler(arbitrary_exception) - # assert watchdog was cancelled - assert op.watchdog_timer is None - assert mock_timer.return_value.cancel.call_count == 1 + event = stage.send_event_up.call_args_list[0].args[0] + assert isinstance(event, pipeline_events_base.DisconnectedEvent) @pytest.mark.it( - "Does not cancels the connection watchdog if the pending operation is DisconnectOperation because there is no connection watchdog" + "Completes the pending DisconnectOperation successfully and removes its pending status" ) - def test_does_not_cancel_watchdog_on_pending_disconnect(self, mocker, stage, mock_timer): - # Set a pending disconnect operation - op = pipeline_ops_base.DisconnectOperation(callback=mocker.MagicMock()) - stage.run_op(op) - - # assert no timers are running - assert mock_timer.return_value.start.call_count == 0 - - # Trigger connect completion - stage.transport.on_mqtt_connected_handler() + def test_disconnect_op_completed(self, stage, arbitrary_exception, pending_connection_op): + stage._pending_connection_op = pending_connection_op + assert not pending_connection_op.completed + assert pending_connection_op.error is None - # assert no timers are still running - assert mock_timer.return_value.start.call_count == 0 - assert mock_timer.return_value.cancel.call_count == 0 + # Trigger disconnect + stage.transport.on_mqtt_connection_dropped_handler(arbitrary_exception) + assert stage._pending_connection_op is None + assert pending_connection_op.completed + assert pending_connection_op.error is None -@pytest.mark.describe("MQTTTransportStage - OCCURRENCE: MQTT connection failure") -class TestMQTTTransportStageOnConnectionFailure(MQTTTransportStageTestConfigComplex): - @pytest.mark.it("Does not send any events up the pipeline") + @pytest.mark.it("Applies connection-drop handling with a pending DisconnectOperation") @pytest.mark.parametrize( - "pending_connection_op", + "connection_retry", [ - pytest.param(None, id="No pending operation"), - pytest.param( - pipeline_ops_base.ConnectOperation(callback=fake_callback), - id="Pending ConnectOperation", - ), - pytest.param( - pipeline_ops_base.ReauthorizeConnectionOperation(callback=fake_callback), - id="Pending ReauthorizeConnectionOperation", - ), - pytest.param( - pipeline_ops_base.DisconnectOperation(callback=fake_callback), - id="Pending DisconnectOperation", - ), + pytest.param(False, id="Connection retry disabled"), + pytest.param(True, id="Connection retry enabled"), ], ) - def test_does_not_send_event(self, mocker, stage, pending_connection_op, arbitrary_exception): + def test_connection_drop_handling( + self, stage, arbitrary_exception, pending_connection_op, connection_retry + ): + stage.nucleus.pipeline_configuration.connection_retry = connection_retry stage._pending_connection_op = pending_connection_op - # Trigger connection failure with an arbitrary cause - stage.transport.on_mqtt_connection_failure_handler(arbitrary_exception) - - assert stage.send_event_up.call_count == 0 - - @pytest.mark.it( - "Completes a pending ConnectOperation unsuccessfully with the cause of connection failure as the error" - ) - def test_fails_pending_connect_op(self, mocker, stage, arbitrary_exception): - # Create a pending ConnectOperation - op = pipeline_ops_base.ConnectOperation(callback=mocker.MagicMock()) - stage.run_op(op) - assert not op.completed - assert stage._pending_connection_op is op - - # Trigger connection failure with an arbitrary cause - stage.transport.on_mqtt_connection_failure_handler(arbitrary_exception) - - assert op.completed - assert op.error is arbitrary_exception - assert stage._pending_connection_op is None - - @pytest.mark.it("Ignores a pending DisconnectOperation, and does not complete it") - def test_ignores_pending_disconnect_op(self, mocker, stage, arbitrary_exception): - # Create a pending DisconnectOperation - op = pipeline_ops_base.DisconnectOperation(callback=mocker.MagicMock()) - stage.run_op(op) - assert not op.completed - assert stage._pending_connection_op is op - - # Trigger connection failure with an arbitrary cause - stage.transport.on_mqtt_connection_failure_handler(arbitrary_exception) + stage.transport.on_mqtt_connection_dropped_handler(arbitrary_exception) + + if connection_retry: + assert ( + stage.transport._op_manager.complete_all_tracked_operations_as_cancelled.call_count + == 0 + ) + assert ( + stage.transport._op_manager.complete_non_resumable_operations_as_cancelled.call_count + == 1 + ) + else: + assert ( + stage.transport._op_manager.complete_all_tracked_operations_as_cancelled.call_count + == 1 + ) + assert ( + stage.transport._op_manager.complete_non_resumable_operations_as_cancelled.call_count + == 0 + ) - # Assert nothing changed about the operation - assert not op.completed - assert stage._pending_connection_op is op + assert stage.report_background_exception.call_count == 1 + background_exception = stage.report_background_exception.call_args.args[0] + assert isinstance(background_exception, transport_exceptions.ConnectionDroppedError) + assert background_exception.__cause__ is arbitrary_exception @pytest.mark.it( - "Triggers the swallowed exception handler (with error cause) when the connection failure is unexpected" + "Applies connection-drop handling before completing the pending DisconnectOperation" ) @pytest.mark.parametrize( - "pending_connection_op", + "connection_retry, cleanup_method_name", [ - pytest.param(None, id="No pending operation"), pytest.param( - pipeline_ops_base.DisconnectOperation(callback=fake_callback), - id="Pending DisconnectOperation", + False, + "complete_all_tracked_operations_as_cancelled", + id="Connection retry disabled", + ), + pytest.param( + True, + "complete_non_resumable_operations_as_cancelled", + id="Connection retry enabled", ), ], ) - def test_unexpected_connection_failure( - self, mocker, stage, arbitrary_exception, pending_connection_op + def test_connection_drop_handling_order( + self, + stage, + arbitrary_exception, + connection_retry, + cleanup_method_name, ): - # A connection failure is unexpected if there is not a pending Connect operation - # i.e. "Why did we get a connection failure? We weren't even trying to connect!" - mock_handler = mocker.patch.object(handle_exceptions, "swallow_unraised_exception") - stage._pending_connection_operation = pending_connection_op - - # Trigger connection failure with arbitrary cause - stage.transport.on_mqtt_connection_failure_handler(arbitrary_exception) - - # swallow exception handler has been called - assert mock_handler.call_count == 1 - assert mock_handler.call_args == mocker.call( - arbitrary_exception, log_msg=mocker.ANY, log_lvl="info" + call_order = [] + stage.nucleus.pipeline_configuration.connection_retry = connection_retry + stage.send_event_up.side_effect = lambda event: call_order.append(type(event)) + cleanup_method = getattr(stage.transport._op_manager, cleanup_method_name) + cleanup_method.side_effect = lambda: call_order.append(cleanup_method_name) + pending_connection_op = pipeline_ops_base.DisconnectOperation( + callback=lambda op, error: call_order.append(type(op)) ) - - @pytest.mark.it( - "Cancels the connection watchdog if the pending operation is a ConnectOperation" - ) - def test_cancels_watchdog_on_pending_connect( - self, mocker, stage, mock_timer, arbitrary_exception - ): - # Set a pending connect operation - op = pipeline_ops_base.ConnectOperation(callback=mocker.MagicMock()) - stage.run_op(op) - - # assert watchdog is running - assert op.watchdog_timer is mock_timer.return_value - assert op.watchdog_timer.start.call_count == 1 - - # Trigger connection failure with arbitrary cause - stage.transport.on_mqtt_connection_failure_handler(arbitrary_exception) - - # assert watchdog was cancelled - assert op.watchdog_timer is None - assert mock_timer.return_value.cancel.call_count == 1 - - @pytest.mark.it( - "Does not cancels the connection watchdog if the pending operation is DisconnectOperation" - ) - def test_does_not_cancel_watchdog_on_pending_disconnect( - self, mocker, stage, mock_timer, arbitrary_exception - ): - # Set a pending disconnect operation - op = pipeline_ops_base.DisconnectOperation(callback=mocker.MagicMock()) - stage.run_op(op) - - # assert no timers are running - assert mock_timer.return_value.start.call_count == 0 - - # Trigger connection failure with arbitrary cause - stage.transport.on_mqtt_connection_failure_handler(arbitrary_exception) - - # assert no timers are still running - assert mock_timer.return_value.start.call_count == 0 - assert mock_timer.return_value.cancel.call_count == 0 - - -@pytest.mark.describe("MQTTTransportStage - OCCURRENCE: MQTT disconnected (Expected)") -class TestMQTTTransportStageOnDisconnectedExpected(MQTTTransportStageTestConfigComplex): - @pytest.fixture(params=[False, True], ids=["No error cause", "With error cause"]) - def cause(self, request, arbitrary_exception): - if request.param: - return arbitrary_exception - else: - return None - - @pytest.fixture - def pending_connection_op(self): - return pipeline_ops_base.DisconnectOperation(callback=fake_callback) - - @pytest.mark.it("Sends a DisconnectedEvent up the pipeline") - def test_disconnect_event_sent(self, stage, cause, pending_connection_op): - stage._pending_connection_op = pending_connection_op - assert stage.send_event_up.call_count == 0 - - # Trigger disconnect - stage.transport.on_mqtt_disconnected_handler(cause) - - assert stage.send_event_up.call_count == 1 - event = stage.send_event_up.call_args[0][0] - assert isinstance(event, pipeline_events_base.DisconnectedEvent) - - @pytest.mark.it("Swallows the exception that caused the disconnect if the cause is specified") - def test_error_swallowed(self, mocker, stage, arbitrary_exception, pending_connection_op): - mock_swallow = mocker.patch.object(handle_exceptions, "swallow_unraised_exception") stage._pending_connection_op = pending_connection_op - # Trigger disconnect with arbitrary cause - stage.transport.on_mqtt_disconnected_handler(arbitrary_exception) + stage.transport.on_mqtt_connection_dropped_handler(arbitrary_exception) - # Exception swallower was called - assert mock_swallow.call_count == 1 - assert mock_swallow.call_args == mocker.call(arbitrary_exception, log_msg=mocker.ANY) - - @pytest.mark.it( - "Completes the pending DisconnectOperation successfully and removes its pending status" - ) - def test_disconnect_op_completed(self, mocker, stage, cause, pending_connection_op): - stage._pending_connection_op = pending_connection_op - assert not pending_connection_op.completed - assert pending_connection_op.error is None - - # Trigger disconnect - stage.transport.on_mqtt_disconnected_handler(cause) - - assert stage._pending_connection_op is None - assert pending_connection_op.completed - assert pending_connection_op.error is None + assert call_order == [ + pipeline_events_base.DisconnectedEvent, + cleanup_method_name, + pipeline_events_base.BackgroundExceptionEvent, + pipeline_ops_base.DisconnectOperation, + ] @pytest.mark.describe( - "MQTTTransportStage - OCCURRENCE: MQTT disconnected (Unexpected - pending ConnectionOperation)" + "MQTTTransportStage - OCCURRENCE: MQTT connection dropped with pending ConnectOperation" ) -class TestMQTTTransportStageOnDisconnectedUnexpectedWithPendingConnectOp( +class TestMQTTTransportStageOnConnectionDroppedWithPendingConnectOperation( MQTTTransportStageTestConfigComplex ): - @pytest.fixture(params=[False, True], ids=["No error cause", "With error cause"]) - def cause(self, request, arbitrary_exception): - if request.param: - return arbitrary_exception - else: - return None - @pytest.fixture def pending_connection_op(self): return pipeline_ops_base.ConnectOperation(callback=fake_callback) @pytest.mark.it("Sends a DisconnectedEvent up the pipeline") - def test_disconnect_event_sent(self, stage, cause, pending_connection_op): + def test_disconnect_event_sent(self, stage, arbitrary_exception, pending_connection_op): stage._pending_connection_op = pending_connection_op assert stage.send_event_up.call_count == 0 # Trigger disconnect - stage.transport.on_mqtt_disconnected_handler(cause) + stage.transport.on_mqtt_connection_dropped_handler(arbitrary_exception) - assert stage.send_event_up.call_count == 1 - event = stage.send_event_up.call_args[0][0] + event = stage.send_event_up.call_args_list[0].args[0] assert isinstance(event, pipeline_events_base.DisconnectedEvent) @pytest.mark.it( @@ -1114,287 +1063,64 @@ def test_op_completed_with_cause(self, stage, arbitrary_exception, pending_conne assert pending_connection_op.error is None # Trigger disconnect with arbitrary cause - stage.transport.on_mqtt_disconnected_handler(arbitrary_exception) + stage.transport.on_mqtt_connection_dropped_handler(arbitrary_exception) assert stage._pending_connection_op is None assert pending_connection_op.completed assert pending_connection_op.error is arbitrary_exception - @pytest.mark.it( - "Completes the pending ConnectOperation unsuccessfully with a ConnectionDroppedError, and removes its pending status, if no cause is provided for the disconnection" - ) - def test_op_completed_no_cause(self, stage, pending_connection_op): - stage._pending_connection_op = pending_connection_op - assert not pending_connection_op.completed - assert pending_connection_op.error is None - - # Trigger disconnect with no cause - stage.transport.on_mqtt_disconnected_handler() - - assert stage._pending_connection_op is None - assert pending_connection_op.completed - assert isinstance(pending_connection_op.error, transport_exceptions.ConnectionDroppedError) - - @pytest.mark.it("Cancels the connection watchdog") - def test_cancels_watchdog(self, mocker, stage, mock_timer, cause, pending_connection_op): - # Set a pending connect operation - stage.run_op(pending_connection_op) - - # assert watchdog is running - assert pending_connection_op.watchdog_timer is mock_timer.return_value - assert pending_connection_op.watchdog_timer.start.call_count == 1 - - # Trigger disconnect - stage.transport.on_mqtt_disconnected_handler(cause) - - # assert watchdog was cancelled - assert pending_connection_op.watchdog_timer is None - assert mock_timer.return_value.cancel.call_count == 1 - @pytest.mark.describe( - "MQTTTransportStage - OCCURRENCE: MQTT disconnected (Unexpected - no pending operation)" + "MQTTTransportStage - OCCURRENCE: MQTT connection dropped with no pending operation" ) -class TestMQTTTransportStageOnDisconnectedUnexpectedNoPendingConnectionOp( +class TestMQTTTransportStageOnConnectionDroppedWithNoPendingConnectionOperation( MQTTTransportStageTestConfigComplex ): - @pytest.fixture(params=[False, True], ids=["No error cause", "With error cause"]) - def cause(self, request, arbitrary_exception): - if request.param: - return arbitrary_exception - else: - return None - @pytest.mark.it( - "Cancels all in-flight operations in the transport, if connection retry has been disabled" + "Completes all tracked MQTT operations as cancelled if connection retry is disabled" ) - def test_inflight_no_retry(self, mocker, stage, cause): + def test_completes_tracked_operations_without_retry(self, mocker, stage, arbitrary_exception): stage.transport._op_manager = mocker.MagicMock() - mock_cancel = stage.transport._op_manager.cancel_all_operations + mock_cancel = stage.transport._op_manager.complete_all_tracked_operations_as_cancelled stage.nucleus.pipeline_configuration.connection_retry = False assert stage._pending_connection_op is None assert mock_cancel.call_count == 0 # Trigger disconnect - stage.transport.on_mqtt_disconnected_handler(cause) + stage.transport.on_mqtt_connection_dropped_handler(arbitrary_exception) assert mock_cancel.call_count == 1 assert mock_cancel.call_args == mocker.call() @pytest.mark.it( - "Does not cancel any in-flight operations in the transport if connection retry has been enabled" + "Preserves resumable publishes and cancels other MQTT operations if connection retry is enabled" ) - def test_inflight_unexpected_with_retry(self, mocker, stage, cause): + def test_cancels_non_resumable_operations_with_retry(self, mocker, stage, arbitrary_exception): stage.transport._op_manager = mocker.MagicMock() - mock_cancel = stage.transport._op_manager.cancel_all_operations + mock_cancel = stage.transport._op_manager.complete_all_tracked_operations_as_cancelled + mock_cancel_non_resumable = ( + stage.transport._op_manager.complete_non_resumable_operations_as_cancelled + ) stage.nucleus.pipeline_configuration.connection_retry = True assert stage._pending_connection_op is None assert mock_cancel.call_count == 0 + assert mock_cancel_non_resumable.call_count == 0 # Trigger disconnect - stage.transport.on_mqtt_disconnected_handler(cause) + stage.transport.on_mqtt_connection_dropped_handler(arbitrary_exception) assert mock_cancel.call_count == 0 + assert mock_cancel_non_resumable.call_args == mocker.call() @pytest.mark.it("Raises a ConnectionDroppedError as a background exception") - def test_background_exception_raised(self, stage, cause): + def test_background_exception_raised(self, stage, arbitrary_exception): assert stage._pending_connection_op is None assert stage.report_background_exception.call_count == 0 # Trigger disconnect - stage.transport.on_mqtt_disconnected_handler(cause) + stage.transport.on_mqtt_connection_dropped_handler(arbitrary_exception) assert stage.report_background_exception.call_count == 1 background_exception = stage.report_background_exception.call_args[0][0] assert isinstance(background_exception, transport_exceptions.ConnectionDroppedError) - assert background_exception.__cause__ is cause - - -disconnect_can_raise = [ - "disconnect_raises", - [ - pytest.param(True, id="mqtt_transport.disconnect raises an exception"), - pytest.param(False, id="mqtt_transport.disconnect does not raises an exception"), - ], -] - - -@pytest.mark.describe("MQTTTransportStage - OCCURRENCE: Connection watchdog expired") -class TestMQTTTransportStageWatchdogExpired(MQTTTransportStageTestConfigComplex): - @pytest.fixture(params=[pipeline_ops_base.ConnectOperation], ids=["Pending ConnectOperation"]) - def pending_op(self, request, mocker): - return request.param(callback=mocker.MagicMock()) - - @pytest.mark.it( - "Performs an MQTT disconnect via the MQTTTransport if the op that started the watchdog is still pending" - ) - def test_calls_disconnect(self, mocker, stage, pending_op, mock_timer): - stage.run_op(pending_op) - - watchdog_expiration = mock_timer.call_args[0][1] - watchdog_expiration() - - assert stage.transport.disconnect.call_count == 1 - - @pytest.mark.it( - "Does not perform an MQTT disconnect via the MQTTTransport if the op that started the watchdog is no longer pending" - ) - def test_does_not_call_disconnect_if_no_longer_pending( - self, mocker, stage, pending_op, mock_timer - ): - stage.run_op(pending_op) - stage._pending_connection_op = None - - watchdog_expiration = mock_timer.call_args[0][1] - watchdog_expiration() - - assert stage.transport.disconnect.call_count == 0 - - @pytest.mark.parametrize(*disconnect_can_raise) - @pytest.mark.it( - "Completes the op that started the watchdog with an OperationTimeout exception if that op is still pending" - ) - def test_completes_with_operation_cancelled( - self, mocker, stage, pending_op, mock_timer, disconnect_raises, arbitrary_exception - ): - if disconnect_raises: - stage.transport.disconnect = mocker.MagicMock(side_effect=arbitrary_exception) - - callback = pending_op.callback_stack[0] - - stage.run_op(pending_op) - - watchdog_expiration = mock_timer.call_args[0][1] - watchdog_expiration() - - assert callback.call_count == 1 - assert isinstance(callback.call_args[1]["error"], pipeline_exceptions.OperationTimeout) - - @pytest.mark.parametrize(*disconnect_can_raise) - @pytest.mark.it( - "Does not complete the op that started the watchdog with an OperationCancelled error if that op is no longer pending" - ) - def test_does_not_complete_op_if_no_longer_pending( - self, mocker, stage, pending_op, mock_timer, disconnect_raises, arbitrary_exception - ): - if disconnect_raises: - stage.transport.disconnect = mocker.MagicMock(side_effect=arbitrary_exception) - - callback = pending_op.callback_stack[0] - - stage.run_op(pending_op) - stage._pending_connection_op = None - - watchdog_expiration = mock_timer.call_args[0][1] - watchdog_expiration() - - assert callback.call_count == 0 - - @pytest.mark.parametrize(*disconnect_can_raise) - @pytest.mark.it( - "Sends a DisconnectedEvent if the op that started the watchdog is still pending and the pipeline is connected" - ) - def test_sends_disconnected_event_if_still_pending_and_connected( - self, - mocker, - stage, - pending_op, - mock_timer, - disconnect_raises, - arbitrary_exception, - pipeline_connected_mock, - ): - if disconnect_raises: - stage.transport.disconnect = mocker.MagicMock(side_effect=arbitrary_exception) - - pipeline_connected_mock.return_value = True - assert stage.nucleus.connected - stage.run_op(pending_op) - - watchdog_expiration = mock_timer.call_args[0][1] - watchdog_expiration() - - assert stage.send_event_up.call_count == 1 - assert isinstance( - stage.send_event_up.call_args[0][0], pipeline_events_base.DisconnectedEvent - ) - - @pytest.mark.parametrize(*disconnect_can_raise) - @pytest.mark.it( - "Does not send a DisconnectedEvent if the op that started the watchdog is still pending and the pipeline is not connected" - ) - def test_does_not_send_disconnected_event_if_still_pending_and_not_connected( - self, - mocker, - stage, - pending_op, - mock_timer, - disconnect_raises, - arbitrary_exception, - pipeline_connected_mock, - ): - if disconnect_raises: - stage.transport.disconnect = mocker.MagicMock(side_effect=arbitrary_exception) - - pipeline_connected_mock.return_value = False - assert not stage.nucleus.connected - stage.run_op(pending_op) - - watchdog_expiration = mock_timer.call_args[0][1] - watchdog_expiration() - - assert stage.send_event_up.call_count == 0 - - @pytest.mark.parametrize(*disconnect_can_raise) - @pytest.mark.it( - "Does not send a DisconnectedEvent if the op that started the watchdog is no longer pending and the pipeline is connected" - ) - def test_does_not_send_disconnected_event_if_no_longer_pending_and_connected( - self, - mocker, - stage, - pending_op, - mock_timer, - disconnect_raises, - arbitrary_exception, - pipeline_connected_mock, - ): - if disconnect_raises: - stage.transport.disconnect = mocker.MagicMock(side_effect=arbitrary_exception) - - pipeline_connected_mock.return_value = True - assert stage.nucleus.connected - stage.run_op(pending_op) - stage._pending_connection_op = None - - watchdog_expiration = mock_timer.call_args[0][1] - watchdog_expiration() - - assert stage.send_event_up.call_count == 0 - - @pytest.mark.parametrize(*disconnect_can_raise) - @pytest.mark.it( - "Does not send a DisconnectedEvent if the op that started the watchdog is no longer pending and the pipeline connected flag is False" - ) - def test_does_not_send_disconnected_event_if_no_longer_pending_and_not_connected( - self, - mocker, - stage, - pending_op, - mock_timer, - disconnect_raises, - arbitrary_exception, - pipeline_connected_mock, - ): - if disconnect_raises: - stage.transport.disconnect = mocker.MagicMock(side_effect=arbitrary_exception) - - pipeline_connected_mock.return_value = True - assert stage.nucleus.connected - stage.run_op(pending_op) - stage._pending_connection_op = None - - watchdog_expiration = mock_timer.call_args[0][1] - watchdog_expiration() - - assert stage.send_event_up.call_count == 0 + assert background_exception.__cause__ is arbitrary_exception diff --git a/tests/unit/common/test_mqtt_transport.py b/tests/unit/common/test_mqtt_transport.py index 41d58d904..68969cd32 100644 --- a/tests/unit/common/test_mqtt_transport.py +++ b/tests/unit/common/test_mqtt_transport.py @@ -4,11 +4,17 @@ # license information. # -------------------------------------------------------------------------- -from azure.iot.device.common.mqtt_transport import MQTTTransport, OperationManager +from azure.iot.device.common.mqtt_transport import ( + ConnectionState, + MQTTTransport, + OperationManager, + OperationType, +) from azure.iot.device.common.models.x509 import X509 from azure.iot.device.common import transport_exceptions as errors from azure.iot.device.common import ProxyOptions import paho.mqtt.client as mqtt +from paho.mqtt.packettypes import PacketTypes import ssl import copy import pytest @@ -32,116 +38,256 @@ fake_qos = 1 fake_mid = 52 fake_rc = 0 -fake_success_rc = 0 -fake_failed_rc = mqtt.MQTT_ERR_PROTOCOL -failed_connack_rc = mqtt.CONNACK_REFUSED_IDENTIFIER_REJECTED +successful_connack_reason_code = mqtt.convert_connack_rc_to_reason_code(mqtt.CONNACK_ACCEPTED) +failed_connack_reason_code = mqtt.convert_connack_rc_to_reason_code( + mqtt.CONNACK_REFUSED_IDENTIFIER_REJECTED +) +successful_disconnect_reason_code = mqtt.convert_disconnect_error_code_to_reason_code( + mqtt.MQTT_ERR_SUCCESS +) +failed_disconnect_reason_code = mqtt.convert_disconnect_error_code_to_reason_code( + mqtt.MQTT_ERR_CONN_LOST +) +protocol_error_disconnect_reason_code = mqtt.convert_disconnect_error_code_to_reason_code( + mqtt.MQTT_ERR_PROTOCOL +) +keep_alive_disconnect_reason_code = mqtt.convert_disconnect_error_code_to_reason_code( + mqtt.MQTT_ERR_KEEPALIVE +) fake_keepalive = 1234 -# mapping of Paho connack rc codes to Error object classes -connack_return_codes = [ +# Paho-normalized CONNACK reasons and their corresponding SDK exception types +paho_connack_reason_error_cases = [ { - "name": "CONNACK_REFUSED_PROTOCOL_VERSION", - "rc": mqtt.CONNACK_REFUSED_PROTOCOL_VERSION, + "reason_code": mqtt.convert_connack_rc_to_reason_code( + mqtt.CONNACK_REFUSED_PROTOCOL_VERSION + ), "error": errors.ProtocolClientError, }, { - "name": "CONNACK_REFUSED_IDENTIFIER_REJECTED", - "rc": mqtt.CONNACK_REFUSED_IDENTIFIER_REJECTED, + "reason_code": mqtt.convert_connack_rc_to_reason_code( + mqtt.CONNACK_REFUSED_IDENTIFIER_REJECTED + ), "error": errors.ProtocolClientError, }, { - "name": "CONNACK_REFUSED_SERVER_UNAVAILABLE", - "rc": mqtt.CONNACK_REFUSED_SERVER_UNAVAILABLE, + "reason_code": mqtt.convert_connack_rc_to_reason_code( + mqtt.CONNACK_REFUSED_SERVER_UNAVAILABLE + ), "error": errors.ConnectionFailedError, }, { - "name": "CONNACK_REFUSED_BAD_USERNAME_PASSWORD", - "rc": mqtt.CONNACK_REFUSED_BAD_USERNAME_PASSWORD, + "reason_code": mqtt.convert_connack_rc_to_reason_code( + mqtt.CONNACK_REFUSED_BAD_USERNAME_PASSWORD + ), "error": errors.UnauthorizedError, }, { - "name": "CONNACK_REFUSED_NOT_AUTHORIZED", - "rc": mqtt.CONNACK_REFUSED_NOT_AUTHORIZED, + "reason_code": mqtt.convert_connack_rc_to_reason_code(mqtt.CONNACK_REFUSED_NOT_AUTHORIZED), "error": errors.UnauthorizedError, }, ] +paho_disconnect_reason_error_cases = [ + { + "reason_code": failed_disconnect_reason_code, + "error": errors.ConnectionDroppedError, + }, + { + "reason_code": keep_alive_disconnect_reason_code, + "error": errors.ConnectionDroppedError, + }, +] + + +def trigger_on_connect(mqtt_client, reason_code=successful_connack_reason_code): + mqtt_client.on_connect( + client=mqtt_client, + userdata=None, + flags=mqtt.ConnectFlags(session_present=False), + reason_code=reason_code, + properties=mqtt.Properties(PacketTypes.CONNACK), + ) + + +def trigger_on_disconnect(mqtt_client, reason_code=successful_disconnect_reason_code): + mqtt_client.on_disconnect( + client=mqtt_client, + userdata=None, + disconnect_flags=mqtt.DisconnectFlags(is_disconnect_packet_from_server=False), + reason_code=reason_code, + properties=mqtt.Properties(PacketTypes.DISCONNECT), + ) + + +def trigger_on_subscribe(mqtt_client, mid, reason_codes=None): + if reason_codes is None: + reason_codes = [mqtt.ReasonCode(PacketTypes.SUBACK, identifier=fake_qos)] + mqtt_client.on_subscribe( + client=mqtt_client, + userdata=None, + mid=mid, + reason_codes=reason_codes, + properties=mqtt.Properties(PacketTypes.SUBACK), + ) + + +def trigger_on_unsubscribe(mqtt_client, mid): + mqtt_client.on_unsubscribe( + client=mqtt_client, + userdata=None, + mid=mid, + reason_codes=[], + properties=mqtt.Properties(PacketTypes.UNSUBACK), + ) + + +def register_publish(manager, mid, callback=None, qos=1): + manager.register_operation( + mid=mid, + callback=callback, + operation_type={ + 0: OperationType.PUBLISH_QOS_0, + 1: OperationType.PUBLISH_QOS_1, + 2: OperationType.PUBLISH_QOS_2, + }[qos], + ) + + +class CancellationCallbackLockProbe(object): + """Observe lifecycle lock state without risking a deadlocked test. + + Re-entering a lifecycle method could deadlock when the implementation is broken. Callback + assertions would also be swallowed by OperationManager, so this probe records observations + for assertions after the lifecycle method returns. + """ + + def __init__(self, lock): + self._lock = lock + self.call_count = 0 + self.cancelled = None + self.lock_was_held = None + + def __call__(self, cancelled=False): + self.call_count += 1 + self.cancelled = cancelled + self.lock_was_held = self._lock.locked() + + +def trigger_on_publish(mqtt_client, mid): + mqtt_client.on_publish( + client=mqtt_client, + userdata=None, + mid=mid, + reason_code=mqtt.ReasonCode(PacketTypes.PUBACK), + properties=mqtt.Properties(PacketTypes.PUBACK), + ) + -# mapping of Paho rc codes to Error object classes -operation_return_codes = [ - {"name": "MQTT_ERR_NOMEM", "rc": mqtt.MQTT_ERR_NOMEM, "error": errors.ConnectionDroppedError}, +# Paho library error codes and their corresponding SDK exception types +paho_error_code_cases = [ + { + "name": "MQTT_ERR_NOMEM", + "error_code": mqtt.MQTT_ERR_NOMEM, + "error": errors.ProtocolClientError, + }, { "name": "MQTT_ERR_PROTOCOL", - "rc": mqtt.MQTT_ERR_PROTOCOL, + "error_code": mqtt.MQTT_ERR_PROTOCOL, + "error": errors.ProtocolClientError, + }, + { + "name": "MQTT_ERR_INVAL", + "error_code": mqtt.MQTT_ERR_INVAL, "error": errors.ProtocolClientError, }, - {"name": "MQTT_ERR_INVAL", "rc": mqtt.MQTT_ERR_INVAL, "error": errors.ProtocolClientError}, - {"name": "MQTT_ERR_NO_CONN", "rc": mqtt.MQTT_ERR_NO_CONN, "error": errors.NoConnectionError}, + { + "name": "MQTT_ERR_NO_CONN", + "error_code": mqtt.MQTT_ERR_NO_CONN, + "error": errors.NoConnectionError, + }, { "name": "MQTT_ERR_CONN_REFUSED", - "rc": mqtt.MQTT_ERR_CONN_REFUSED, + "error_code": mqtt.MQTT_ERR_CONN_REFUSED, "error": errors.ConnectionFailedError, }, { "name": "MQTT_ERR_NOT_FOUND", - "rc": mqtt.MQTT_ERR_NOT_FOUND, + "error_code": mqtt.MQTT_ERR_NOT_FOUND, "error": errors.ConnectionFailedError, }, { "name": "MQTT_ERR_CONN_LOST", - "rc": mqtt.MQTT_ERR_CONN_LOST, + "error_code": mqtt.MQTT_ERR_CONN_LOST, "error": errors.ConnectionDroppedError, }, - {"name": "MQTT_ERR_TLS", "rc": mqtt.MQTT_ERR_TLS, "error": errors.UnauthorizedError}, + {"name": "MQTT_ERR_TLS", "error_code": mqtt.MQTT_ERR_TLS, "error": errors.UnauthorizedError}, { "name": "MQTT_ERR_PAYLOAD_SIZE", - "rc": mqtt.MQTT_ERR_PAYLOAD_SIZE, + "error_code": mqtt.MQTT_ERR_PAYLOAD_SIZE, "error": errors.ProtocolClientError, }, { "name": "MQTT_ERR_NOT_SUPPORTED", - "rc": mqtt.MQTT_ERR_NOT_SUPPORTED, + "error_code": mqtt.MQTT_ERR_NOT_SUPPORTED, "error": errors.ProtocolClientError, }, - {"name": "MQTT_ERR_AUTH", "rc": mqtt.MQTT_ERR_AUTH, "error": errors.UnauthorizedError}, + {"name": "MQTT_ERR_AUTH", "error_code": mqtt.MQTT_ERR_AUTH, "error": errors.UnauthorizedError}, { "name": "MQTT_ERR_ACL_DENIED", - "rc": mqtt.MQTT_ERR_ACL_DENIED, + "error_code": mqtt.MQTT_ERR_ACL_DENIED, "error": errors.UnauthorizedError, }, - {"name": "MQTT_ERR_UNKNOWN", "rc": mqtt.MQTT_ERR_UNKNOWN, "error": errors.ProtocolClientError}, - {"name": "MQTT_ERR_ERRNO", "rc": mqtt.MQTT_ERR_ERRNO, "error": errors.ProtocolClientError}, + { + "name": "MQTT_ERR_UNKNOWN", + "error_code": mqtt.MQTT_ERR_UNKNOWN, + "error": errors.ProtocolClientError, + }, + { + "name": "MQTT_ERR_ERRNO", + "error_code": mqtt.MQTT_ERR_ERRNO, + "error": errors.ProtocolClientError, + }, { "name": "MQTT_ERR_QUEUE_SIZE", - "rc": mqtt.MQTT_ERR_QUEUE_SIZE, + "error_code": mqtt.MQTT_ERR_QUEUE_SIZE, "error": errors.ProtocolClientError, }, { "name": "MQTT_ERR_KEEPALIVE", - "rc": mqtt.MQTT_ERR_KEEPALIVE, + "error_code": mqtt.MQTT_ERR_KEEPALIVE, "error": errors.ConnectionDroppedError, }, ] -# For disconnect, MQTT_ERR_NO_CONN is treated as success (socket already closed) -# so we exclude it from the error return codes for disconnect tests -disconnect_operation_return_codes = [ - x for x in operation_return_codes if x["rc"] != mqtt.MQTT_ERR_NO_CONN +# During disconnect, MQTT_ERR_NO_CONN means the socket is already closed and is successful. +disconnect_error_code_cases = [ + case for case in paho_error_code_cases if case["error_code"] != mqtt.MQTT_ERR_NO_CONN +] + +# For QoS 1 and QoS 2, Paho retains a publish that returns MQTT_ERR_NO_CONN. +publish_failure_code_cases = [ + case for case in paho_error_code_cases if case["error_code"] != mqtt.MQTT_ERR_NO_CONN ] @pytest.fixture -def mock_mqtt_client(mocker, fake_paho_thread): +def mock_mqtt_client(mocker): mock = mocker.patch.object(mqtt, "Client") mock_mqtt_client = mock.return_value mock_mqtt_client.subscribe = mocker.MagicMock(return_value=(fake_rc, fake_mid)) mock_mqtt_client.unsubscribe = mocker.MagicMock(return_value=(fake_rc, fake_mid)) - mock_mqtt_client.publish = mocker.MagicMock(return_value=(fake_rc, fake_mid)) + message_info = mqtt.MQTTMessageInfo(fake_mid) + message_info.rc = fake_rc + mock_mqtt_client.publish = mocker.MagicMock(return_value=message_info) mock_mqtt_client.connect.return_value = 0 mock_mqtt_client.reconnect.return_value = 0 mock_mqtt_client.disconnect.return_value = 0 - mock_mqtt_client._thread = fake_paho_thread + mock_mqtt_client.loop_start.side_effect = lambda: ( + trigger_on_connect(mock_mqtt_client) or mqtt.MQTT_ERR_SUCCESS + ) + mock_mqtt_client.loop_stop.return_value = 0 return mock_mqtt_client @@ -163,30 +309,6 @@ def collected_transport_weakref(mock_mqtt_client): return transport_weakref -@pytest.fixture -def fake_paho_thread(mocker): - thread = mocker.MagicMock(spec=threading.Thread) - thread.name = "_fake_paho_thread_" - return thread - - -@pytest.fixture -def mock_paho_thread_current(mocker, fake_paho_thread): - return mocker.patch.object(threading, "current_thread", return_value=fake_paho_thread) - - -@pytest.fixture -def fake_non_paho_thread(mocker): - thread = mocker.MagicMock(spec=threading.Thread) - thread.name = "_fake_non_paho_thread_" - return thread - - -@pytest.fixture -def mock_non_paho_thread_current(mocker, fake_non_paho_thread): - return mocker.patch.object(threading, "current_thread", return_value=fake_non_paho_thread) - - @pytest.mark.describe("MQTTTransport - Instantiation") class TestInstantiation(object): @pytest.fixture( @@ -220,10 +342,11 @@ def test_instantiates_mqtt_client(self, mocker): assert mock_mqtt_client_constructor.call_count == 1 assert mock_mqtt_client_constructor.call_args == mocker.call( - callback_api_version=mqtt.CallbackAPIVersion.VERSION1, + callback_api_version=mqtt.CallbackAPIVersion.VERSION2, client_id=fake_device_id, clean_session=False, protocol=mqtt.MQTTv311, + reconnect_on_failure=False, ) @pytest.mark.it( @@ -242,11 +365,12 @@ def test_configures_mqtt_websockets(self, mocker): assert mock_mqtt_client_constructor.call_count == 1 assert mock_mqtt_client_constructor.call_args == mocker.call( - callback_api_version=mqtt.CallbackAPIVersion.VERSION1, + callback_api_version=mqtt.CallbackAPIVersion.VERSION2, client_id=fake_device_id, clean_session=False, protocol=mqtt.MQTTv311, transport="websockets", + reconnect_on_failure=False, ) # Verify websockets options have been set @@ -392,8 +516,7 @@ def test_handler_callbacks_set_to_none(self, mocker): client_id=fake_device_id, hostname=fake_hostname, username=fake_username ) - assert transport.on_mqtt_connected_handler is None - assert transport.on_mqtt_disconnected_handler is None + assert transport.on_mqtt_connection_dropped_handler is None assert transport.on_mqtt_message_received_handler is None @pytest.mark.it("Initializes internal operation tracking structures") @@ -401,22 +524,19 @@ def test_operation_infrastructure_set_up(self, mocker): transport = MQTTTransport( client_id=fake_device_id, hostname=fake_hostname, username=fake_username ) - assert transport._op_manager._pending_operation_callbacks == {} + assert transport._op_manager._pending_operations == {} assert transport._op_manager._unknown_operation_completions == {} - @pytest.mark.it("Sets paho auto-reconnect interval to 2 hours") - def test_sets_reconnect_interval(self, mocker, transport, mock_mqtt_client): - MQTTTransport(client_id=fake_device_id, hostname=fake_hostname, username=fake_username) - - # called once by the mqtt_client constructor and once by mqtt_transport.py - assert mock_mqtt_client.reconnect_delay_set.call_count == 2 - assert mock_mqtt_client.reconnect_delay_set.call_args == mocker.call(120 * 60) + @pytest.mark.it("Does not configure Paho reconnect delay or manual acknowledgements") + def test_does_not_set_reconnect_interval(self, transport, mock_mqtt_client): + assert mock_mqtt_client.reconnect_delay_set.call_count == 0 + assert mock_mqtt_client.manual_ack_set.call_count == 0 @pytest.mark.describe("MQTTTransport - .shutdown()") class TestShutdown(object): - @pytest.mark.it("Force Disconnects Paho") - def test_disconnects(self, mocker, mock_mqtt_client, transport): + @pytest.mark.it("Disconnects Paho and stops its network loop") + def test_disconnects_and_stops_network_loop(self, mocker, mock_mqtt_client, transport): transport.shutdown() assert mock_mqtt_client.disconnect.call_count == 1 @@ -424,13 +544,101 @@ def test_disconnects(self, mocker, mock_mqtt_client, transport): assert mock_mqtt_client.loop_stop.call_count == 1 assert mock_mqtt_client.loop_stop.call_args == mocker.call() - @pytest.mark.it("Does NOT trigger the on_disconnect handler upon disconnect") - def test_does_not_trigger_handler(self, mocker, mock_mqtt_client, transport): - mock_disconnect_handler = mocker.MagicMock() - mock_mqtt_client.on_disconnect = mock_disconnect_handler + @pytest.mark.it("Serializes shutdown behind an in-progress connect") + def test_serializes_behind_connect( + self, mock_mqtt_client, transport, run_in_daemon_thread, poll_until + ): + mock_mqtt_client.loop_start.side_effect = None + mock_mqtt_client.loop_start.return_value = mqtt.MQTT_ERR_SUCCESS + connect_future = run_in_daemon_thread(transport.connect, fake_password, timeout=1) + poll_until(lambda: mock_mqtt_client.loop_start.call_count == 1, timeout=1) + + shutdown_future = run_in_daemon_thread(transport.shutdown) + poll_until(shutdown_future.running, timeout=1) + + assert mock_mqtt_client.disconnect.call_count == 0 + + trigger_on_connect(mock_mqtt_client) + connect_future.result(timeout=1) + shutdown_future.result(timeout=1) + + assert mock_mqtt_client.disconnect.call_count == 1 + + @pytest.mark.it("Serializes shutdown behind an in-progress disconnect") + def test_serializes_behind_disconnect( + self, mock_mqtt_client, transport, run_in_daemon_thread, poll_until + ): + disconnect_started = threading.Event() + release_disconnect = threading.Event() + + def blocking_disconnect(): + disconnect_started.set() + release_disconnect.wait() + return mqtt.MQTT_ERR_SUCCESS + + mock_mqtt_client.disconnect.side_effect = blocking_disconnect + disconnect_future = run_in_daemon_thread(transport.disconnect) + assert disconnect_started.wait(timeout=1) + + shutdown_future = run_in_daemon_thread(transport.shutdown) + poll_until(shutdown_future.running, timeout=1) + + try: + assert mock_mqtt_client.disconnect.call_count == 1 + finally: + release_disconnect.set() + + disconnect_future.result(timeout=1) + shutdown_future.result(timeout=1) + + assert mock_mqtt_client.disconnect.call_count == 2 + + @pytest.mark.it("Does NOT invoke Paho's on_disconnect callback during shutdown") + def test_does_not_invoke_paho_on_disconnect(self, mocker, mock_mqtt_client, transport): + paho_on_disconnect = mocker.MagicMock() + mock_mqtt_client.on_disconnect = paho_on_disconnect transport.shutdown() assert mock_mqtt_client.on_disconnect is None - assert mock_disconnect_handler.call_count == 0 + assert paho_on_disconnect.call_count == 0 + + @pytest.mark.it("Stops the network loop and allows any Exception from disconnect to propagate") + def test_stops_loop_if_disconnect_raises( + self, mock_mqtt_client, transport, arbitrary_exception + ): + mock_mqtt_client.disconnect.side_effect = arbitrary_exception + + with pytest.raises(type(arbitrary_exception)) as e_info: + transport.shutdown() + + assert e_info.value is arbitrary_exception + assert mock_mqtt_client.loop_stop.call_count == 1 + + @pytest.mark.it( + "Completes tracked operations as cancelled and allows any Exception from teardown to propagate" + ) + def test_completes_tracked_operations_if_teardown_raises( + self, mocker, mock_mqtt_client, transport, arbitrary_exception + ): + callback = mocker.MagicMock() + transport.subscribe(fake_topic, callback=callback) + mock_mqtt_client.disconnect.side_effect = arbitrary_exception + + with pytest.raises(type(arbitrary_exception)): + transport.shutdown() + + assert callback.call_count == 1 + assert callback.call_args == mocker.call(cancelled=True) + + @pytest.mark.it("Invokes cancellation callbacks after releasing the lifecycle lock") + def test_cancellation_callback_after_lifecycle_lock_release(self, mock_mqtt_client, transport): + callback_probe = CancellationCallbackLockProbe(transport._connection_lock) + transport.subscribe(fake_topic, callback=callback_probe) + + transport.shutdown() + + assert callback_probe.call_count == 1 + assert callback_probe.cancelled is True + assert callback_probe.lock_was_held is False class ArbitraryConnectException(Exception): @@ -439,6 +647,19 @@ class ArbitraryConnectException(Exception): @pytest.mark.describe("MQTTTransport - .connect()") class TestConnect(object): + @pytest.mark.it("Joins a previously started network loop before connecting") + def test_joins_prior_network_loop_before_connect(self, mocker, mock_mqtt_client, transport): + call_order = mocker.MagicMock() + call_order.attach_mock(mock_mqtt_client.loop_stop, "loop_stop") + call_order.attach_mock(mock_mqtt_client.connect, "connect") + + transport.connect(fake_password) + + assert call_order.mock_calls[:2] == [ + mocker.call.loop_stop(), + mocker.call.connect(host=fake_hostname, port=8883, keepalive=None), + ] + @pytest.mark.it("Uses the stored username and provided password for Paho credentials") def test_use_provided_password(self, mocker, mock_mqtt_client, transport): transport.connect(fake_password) @@ -581,20 +802,22 @@ def test_client_raises_base_exception( transport.connect(fake_password) assert e_info.value is arbitrary_base_exception - # NOTE: this test tests for all possible return codes, even ones that shouldn't be + # NOTE: this test tests all mapped Paho error codes, even ones that shouldn't be # possible on a connect operation. - @pytest.mark.it("Raises a custom Exception if Paho connect returns a failing rc code") + @pytest.mark.it("Raises a custom Exception if Paho connect returns an error code") @pytest.mark.parametrize( - "error_params", - operation_return_codes, - ids=["{}->{}".format(x["name"], x["error"].__name__) for x in operation_return_codes], + "error_case", + paho_error_code_cases, + ids=[ + "{}->{}".format(case["name"], case["error"].__name__) for case in paho_error_code_cases + ], ) - def test_client_returns_failing_rc_code( - self, mocker, mock_mqtt_client, transport, error_params - ): - mock_mqtt_client.connect.return_value = error_params["rc"] - with pytest.raises(error_params["error"]): + def test_client_returns_error_code(self, mocker, mock_mqtt_client, transport, error_case): + mock_mqtt_client.connect.return_value = error_case["error_code"] + with pytest.raises(error_case["error"]) as e_info: transport.connect(fake_password) + assert str(e_info.value) == mqtt.error_string(error_case["error_code"]) + assert mock_mqtt_client.disconnect.call_count == 1 @pytest.fixture( params=[ @@ -617,201 +840,566 @@ def test_client_returns_failing_rc_code( def connect_exception(self, request): return request.param - @pytest.mark.it("Calls _mqtt_client.disconnect if Paho raises an exception") - def test_calls_disconnect_on_exception( - self, mocker, mock_mqtt_client, transport, connect_exception - ): + @pytest.mark.it("Disconnects Paho and stops its network loop if connect raises an Exception") + def test_cleans_up_on_exception(self, mock_mqtt_client, transport, connect_exception): mock_mqtt_client.connect.side_effect = connect_exception with pytest.raises(Exception): transport.connect(fake_password) assert mock_mqtt_client.disconnect.call_count == 1 + assert mock_mqtt_client.loop_stop.call_count == 2 + assert transport._connection_lifecycle._state is ConnectionState.DISCONNECTED + assert transport._connection_lifecycle._error is None - @pytest.mark.it("Calls _mqtt_client.loop_stop if Paho raises an exception") - def test_calls_loop_stop_on_exception( - self, mocker, mock_mqtt_client, transport, connect_exception - ): - mock_mqtt_client.connect.side_effect = connect_exception - with pytest.raises(Exception): - transport.connect(fake_password) - assert mock_mqtt_client.loop_stop.call_count == 1 - - @pytest.mark.it( - "Sets Paho's _thread to None if Paho raises an exception while running in the Paho thread" + @pytest.mark.parametrize( + "connect_failure, expected_error", + [ + pytest.param("socket error", errors.ConnectionFailedError), + pytest.param("proxy auth error", errors.UnauthorizedError), + pytest.param("connect error code", errors.ProtocolClientError), + pytest.param("loop start error code", errors.ProtocolClientError), + ], ) - def test_sets_thread_to_none_on_exception_in_paho_thread( - self, mocker, mock_mqtt_client, transport, mock_paho_thread_current, connect_exception - ): - mock_mqtt_client.connect.side_effect = connect_exception - with pytest.raises(Exception): + @pytest.mark.parametrize( + "cleanup_failure", + [pytest.param("disconnect"), pytest.param("loop_stop")], + ) + @pytest.mark.it("Preserves a connect error if cleanup raises an Exception") + def test_connect_error_preserved_if_cleanup_raises( + self, + mock_mqtt_client, + transport, + arbitrary_exception, + connect_failure, + expected_error, + cleanup_failure, + ): + if connect_failure == "socket error": + mock_mqtt_client.connect.side_effect = socket.error() + elif connect_failure == "proxy auth error": + mock_mqtt_client.connect.side_effect = socks.SOCKS5AuthError( + "authentication failed", socket_err="authentication failed" + ) + elif connect_failure == "connect error code": + mock_mqtt_client.connect.return_value = mqtt.MQTT_ERR_INVAL + else: + mock_mqtt_client.loop_start.side_effect = None + mock_mqtt_client.loop_start.return_value = mqtt.MQTT_ERR_INVAL + + if cleanup_failure == "disconnect": + mock_mqtt_client.disconnect.side_effect = arbitrary_exception + else: + mock_mqtt_client.loop_stop.side_effect = [None, arbitrary_exception] + + with pytest.raises(expected_error) as e_info: transport.connect(fake_password) - assert mock_mqtt_client._thread is None + + assert type(e_info.value) is expected_error + assert mock_mqtt_client.on_disconnect is not None @pytest.mark.it( - "Does not sets Paho's _thread to None if Paho raises an exception running outside the Paho thread" + "Raises a ProtocolClientError and cleans up if Paho loop_start() returns an error code" ) - def test_does_not_set_thread_to_none_on_exception_not_in_paho_thread( - self, mocker, mock_mqtt_client, transport, mock_non_paho_thread_current, connect_exception - ): - mock_mqtt_client.connect.side_effect = connect_exception - with pytest.raises(Exception): + def test_loop_start_returns_error(self, mock_mqtt_client, transport): + mock_mqtt_client.loop_start.side_effect = None + mock_mqtt_client.loop_start.return_value = mqtt.MQTT_ERR_INVAL + + with pytest.raises(errors.ProtocolClientError): transport.connect(fake_password) - assert mock_mqtt_client._thread is not None + assert mock_mqtt_client.disconnect.call_count == 1 + assert mock_mqtt_client.loop_stop.call_count == 2 -@pytest.mark.describe("MQTTTransport - OCCURRENCE: Connect Completed") -class TestEventConnectComplete(object): @pytest.mark.it( - "Triggers on_mqtt_connected_handler event handler upon successful connect completion" + "Raises a ProtocolClientError and cleans up if Paho loop_start() raises an Exception" ) - def test_calls_event_handler_callback(self, mocker, mock_mqtt_client, transport): - callback = mocker.MagicMock() - transport.on_mqtt_connected_handler = callback + def test_loop_start_raises(self, mock_mqtt_client, transport, arbitrary_exception): + mock_mqtt_client.loop_start.side_effect = arbitrary_exception - # Manually trigger Paho on_connect event_handler - mock_mqtt_client.on_connect(client=mock_mqtt_client, userdata=None, flags=None, rc=fake_rc) + with pytest.raises(errors.ProtocolClientError) as e_info: + transport.connect(fake_password) - # Verify transport.on_mqtt_connected_handler was called - assert callback.call_count == 1 - assert callback.call_args == mocker.call() + assert e_info.value.__cause__ is arbitrary_exception + assert mock_mqtt_client.disconnect.call_count == 1 + assert mock_mqtt_client.loop_stop.call_count == 2 + assert mock_mqtt_client.on_disconnect is not None @pytest.mark.it( - "Stops Paho's network loop if the MQTTTransport was garbage collected before a successful connect completed" + "Raises a ProtocolClientError and replaces a Paho client left unusable by a network-thread start failure" ) - def test_stops_loop_after_gc(self, mocker, mock_mqtt_client, collected_transport_weakref): - mock_mqtt_client.on_connect( - client=mock_mqtt_client, userdata=None, flags=None, rc=fake_success_rc + def test_loop_start_thread_failure_replaces_client(self, mocker): + transport = MQTTTransport( + client_id=fake_device_id, + hostname=fake_hostname, + username=fake_username, + keep_alive=fake_keepalive, ) + failed_client = transport._mqtt_client + publish_callback = mocker.MagicMock() + transport.publish(fake_topic, fake_payload, qos=1, callback=publish_callback) + failed_client_socket, failed_server_socket = socket.socketpair() + mocker.patch.object(failed_client, "_create_socket", return_value=failed_client_socket) + start_error = RuntimeError("cannot start network thread") + mocker.patch.object(threading.Thread, "start", side_effect=start_error) + + try: + with pytest.raises(errors.ProtocolClientError) as e_info: + transport.connect(fake_password) + finally: + failed_server_socket.close() + + assert e_info.value.__cause__ is start_error + assert failed_client_socket.fileno() == -1 + assert transport._mqtt_client is not failed_client + assert transport._mqtt_client.on_connect is not None + assert transport._mqtt_client.on_disconnect is not None + assert publish_callback.call_count == 1 + assert publish_callback.call_args == mocker.call(cancelled=True) + assert transport._op_manager._pending_operations == {} + + @pytest.mark.it( + "Invokes cancellation callbacks after releasing the lifecycle lock when replacing a client" + ) + def test_loop_start_thread_failure_callback_after_lifecycle_lock_release( + self, mocker, mock_mqtt_client, transport + ): + callback_probe = CancellationCallbackLockProbe(transport._connection_lock) + transport.publish(fake_topic, fake_payload, qos=1, callback=callback_probe) + mock_mqtt_client.loop_start.side_effect = RuntimeError("cannot start network thread") + mocker.patch.object( + transport, + "_cleanup_failed_connect", + side_effect=RuntimeError("cannot clean up network thread"), + ) + mocker.patch.object(transport, "_create_mqtt_client", return_value=mocker.MagicMock()) - assert mock_mqtt_client.loop_stop.call_count == 1 - assert mock_mqtt_client.loop_stop.call_args == mocker.call() + with pytest.raises(errors.ProtocolClientError): + transport.connect(fake_password) - @pytest.mark.it( - "Skips on_mqtt_connected_handler event handler if set to 'None' upon successful connect completion" + assert callback_probe.call_count == 1 + assert callback_probe.cancelled is True + assert callback_probe.lock_was_held is False + + @pytest.mark.it("Waits for CONNACK before returning") + def test_waits_for_connack(self, mock_mqtt_client, transport, run_in_daemon_thread, poll_until): + mock_mqtt_client.loop_start.side_effect = None + mock_mqtt_client.loop_start.return_value = mqtt.MQTT_ERR_SUCCESS + + connect_future = run_in_daemon_thread(transport.connect, fake_password) + poll_until(lambda: mock_mqtt_client.loop_start.call_count == 1, timeout=1) + + assert not connect_future.done() + + trigger_on_connect(mock_mqtt_client) + connect_future.result(timeout=1) + + @pytest.mark.it("Serializes concurrent connect calls") + def test_serializes_connect_calls( + self, mock_mqtt_client, transport, run_in_daemon_thread, poll_until + ): + mock_mqtt_client.loop_start.side_effect = None + mock_mqtt_client.loop_start.return_value = mqtt.MQTT_ERR_SUCCESS + first_connect = run_in_daemon_thread(transport.connect, fake_password, timeout=1) + poll_until(lambda: mock_mqtt_client.loop_start.call_count == 1, timeout=1) + + second_connect = run_in_daemon_thread(transport.connect, fake_password, timeout=1) + poll_until(lambda: second_connect.running(), timeout=1) + + assert mock_mqtt_client.connect.call_count == 1 + + trigger_on_connect(mock_mqtt_client) + first_connect.result(timeout=1) + poll_until(lambda: mock_mqtt_client.loop_start.call_count == 2, timeout=1) + trigger_on_connect(mock_mqtt_client) + second_connect.result(timeout=1) + + @pytest.mark.it("Serializes connect behind an in-progress disconnect") + def test_serializes_behind_disconnect( + self, mock_mqtt_client, transport, run_in_daemon_thread, poll_until + ): + disconnect_started = threading.Event() + release_disconnect = threading.Event() + + def blocking_disconnect(): + disconnect_started.set() + release_disconnect.wait() + return mqtt.MQTT_ERR_SUCCESS + + mock_mqtt_client.disconnect.side_effect = blocking_disconnect + disconnect_future = run_in_daemon_thread(transport.disconnect) + assert disconnect_started.wait(timeout=1) + + connect_future = run_in_daemon_thread(transport.connect, fake_password) + poll_until(connect_future.running, timeout=1) + + try: + assert not connect_future.done() + assert mock_mqtt_client.connect.call_count == 0 + finally: + release_disconnect.set() + + disconnect_future.result(timeout=1) + connect_future.result(timeout=1) + + assert mock_mqtt_client.connect.call_count == 1 + + @pytest.mark.it("Raises the mapped error from a failed CONNACK") + @pytest.mark.parametrize( + "error_case", + paho_connack_reason_error_cases, + ids=[ + "{}->{}".format(case["reason_code"], case["error"].__name__) + for case in paho_connack_reason_error_cases + ], ) - def test_skips_none_event_handler_callback(self, mocker, mock_mqtt_client, transport): - assert transport.on_mqtt_connected_handler is None + def test_failed_connack_raises( + self, mock_mqtt_client, transport, run_in_daemon_thread, poll_until, error_case + ): + mock_mqtt_client.loop_start.side_effect = None + mock_mqtt_client.loop_start.return_value = mqtt.MQTT_ERR_SUCCESS + connect_future = run_in_daemon_thread(transport.connect, fake_password) + poll_until(lambda: mock_mqtt_client.loop_start.call_count == 1, timeout=1) - transport.connect(fake_password) + trigger_on_connect(mock_mqtt_client, reason_code=error_case["reason_code"]) - mock_mqtt_client.on_connect(client=mock_mqtt_client, userdata=None, flags=None, rc=fake_rc) + with pytest.raises(error_case["error"]) as e_info: + connect_future.result(timeout=1) - # No further asserts required - this is a test to show that it skips a callback. - # Not raising an exception == test passed + assert type(e_info.value) is error_case["error"] + assert mock_mqtt_client.disconnect.call_count == 1 + assert mock_mqtt_client.loop_stop.call_count == 2 - @pytest.mark.it("Recovers from Exception in on_mqtt_connected_handler event handler") - def test_event_handler_callback_raises_exception( - self, mocker, mock_mqtt_client, transport, arbitrary_exception + @pytest.mark.it("Raises ConnectionFailedError if the connection closes before CONNACK") + def test_disconnect_before_connack_raises( + self, mocker, mock_mqtt_client, transport, run_in_daemon_thread, poll_until ): - event_cb = mocker.MagicMock(side_effect=arbitrary_exception) - transport.on_mqtt_connected_handler = event_cb + mock_mqtt_client.loop_start.side_effect = None + mock_mqtt_client.loop_start.return_value = mqtt.MQTT_ERR_SUCCESS + connection_dropped_handler = mocker.MagicMock() + transport.on_mqtt_connection_dropped_handler = connection_dropped_handler + connect_future = run_in_daemon_thread(transport.connect, fake_password) + poll_until(lambda: mock_mqtt_client.loop_start.call_count == 1, timeout=1) - transport.connect(fake_password) - mock_mqtt_client.on_connect(client=mock_mqtt_client, userdata=None, flags=None, rc=fake_rc) + trigger_on_disconnect(mock_mqtt_client, reason_code=failed_disconnect_reason_code) - # Callback was called, but exception did not propagate - assert event_cb.call_count == 1 + with pytest.raises(errors.ConnectionFailedError): + connect_future.result(timeout=1) - @pytest.mark.it( - "Allows any BaseExceptions raised in on_mqtt_connected_handler event handler to propagate" + assert connection_dropped_handler.call_count == 0 + assert mock_mqtt_client.disconnect.call_count == 1 + assert mock_mqtt_client.loop_stop.call_count == 2 + + @pytest.mark.it("Reports ConnectionFailedError when Paho hides a protocol-version refusal") + def test_protocol_version_refusal_without_connack_callback( + self, mock_mqtt_client, transport, run_in_daemon_thread, poll_until + ): + mock_mqtt_client.loop_start.side_effect = None + mock_mqtt_client.loop_start.return_value = mqtt.MQTT_ERR_SUCCESS + connect_future = run_in_daemon_thread(transport.connect, fake_password) + poll_until(lambda: mock_mqtt_client.loop_start.call_count == 1, timeout=1) + + # Paho 2.1 skips on_connect for this refusal and collapses MQTT_ERR_PROTOCOL and + # MQTT_ERR_CONN_LOST to the same callback API v2 disconnect reason. + assert protocol_error_disconnect_reason_code == failed_disconnect_reason_code + trigger_on_disconnect(mock_mqtt_client, reason_code=protocol_error_disconnect_reason_code) + + with pytest.raises(errors.ConnectionFailedError) as e_info: + connect_future.result(timeout=1) + + assert type(e_info.value) is errors.ConnectionFailedError + + @pytest.mark.it("Raises ConnectionDroppedError if the connection drops before connect returns") + def test_disconnect_after_connack_before_return_raises( + self, mocker, mock_mqtt_client, transport + ): + connection_dropped_handler = mocker.MagicMock() + transport.on_mqtt_connection_dropped_handler = connection_dropped_handler + + def connect_then_disconnect(): + trigger_on_connect(mock_mqtt_client) + trigger_on_disconnect(mock_mqtt_client, reason_code=failed_disconnect_reason_code) + return mqtt.MQTT_ERR_SUCCESS + + mock_mqtt_client.loop_start.side_effect = connect_then_disconnect + + with pytest.raises(errors.ConnectionDroppedError): + transport.connect(fake_password) + + assert connection_dropped_handler.call_count == 0 + assert mock_mqtt_client.disconnect.call_count == 1 + assert mock_mqtt_client.loop_stop.call_count == 2 + + @pytest.mark.it("Preserves a rejected CONNACK if disconnection follows before connect returns") + def test_failed_connack_then_disconnect_preserves_connack_error( + self, mock_mqtt_client, transport + ): + def reject_then_disconnect(): + trigger_on_connect(mock_mqtt_client, reason_code=failed_connack_reason_code) + trigger_on_disconnect(mock_mqtt_client, reason_code=failed_disconnect_reason_code) + return mqtt.MQTT_ERR_SUCCESS + + mock_mqtt_client.loop_start.side_effect = reject_then_disconnect + + with pytest.raises(errors.ProtocolClientError): + transport.connect(fake_password) + + @pytest.mark.parametrize( + "reason_code", + [successful_connack_reason_code, failed_connack_reason_code], + ids=["Accepted CONNACK", "Rejected CONNACK"], ) - def test_event_handler_callback_raises_base_exception( - self, mocker, mock_mqtt_client, transport, arbitrary_base_exception + @pytest.mark.it("Preserves a pre-CONNACK disconnection if CONNACK follows") + def test_disconnect_then_connack_preserves_connection_failure( + self, mock_mqtt_client, transport, reason_code ): - event_cb = mocker.MagicMock(side_effect=arbitrary_base_exception) - transport.on_mqtt_connected_handler = event_cb + def disconnect_then_connack(): + trigger_on_disconnect(mock_mqtt_client, reason_code=failed_disconnect_reason_code) + trigger_on_connect(mock_mqtt_client, reason_code=reason_code) + return mqtt.MQTT_ERR_SUCCESS - transport.connect(fake_password) - with pytest.raises(arbitrary_base_exception.__class__) as e_info: - mock_mqtt_client.on_connect( - client=mock_mqtt_client, userdata=None, flags=None, rc=fake_rc - ) - assert e_info.value is arbitrary_base_exception + mock_mqtt_client.loop_start.side_effect = disconnect_then_connack + with pytest.raises(errors.ConnectionFailedError) as e_info: + transport.connect(fake_password) + + assert type(e_info.value) is errors.ConnectionFailedError -@pytest.mark.describe("MQTTTransport - OCCURRENCE: Connection Failure") -class TestEventConnectionFailure(object): @pytest.mark.parametrize( - "error_params", - connack_return_codes, - ids=["{}->{}".format(x["name"], x["error"].__name__) for x in connack_return_codes], + "terminal_outcome, expected_error", + [ + pytest.param("rejected_connack", errors.ProtocolClientError, id="Rejected CONNACK"), + pytest.param( + "disconnect_before_connack", + errors.ConnectionFailedError, + id="Disconnect before CONNACK", + ), + pytest.param( + "disconnect_after_connack", + errors.ConnectionDroppedError, + id="Disconnect after accepted CONNACK", + ), + ], ) - @pytest.mark.it( - "Triggers on_mqtt_connection_failure_handler event handler with custom Exception upon failed connect completion" + @pytest.mark.it("Preserves a terminal connection error if cleanup raises an Exception") + def test_terminal_error_preserved_if_cleanup_raises( + self, + mock_mqtt_client, + transport, + arbitrary_exception, + terminal_outcome, + expected_error, + ): + def fail_during_loop_start(): + if terminal_outcome == "rejected_connack": + trigger_on_connect(mock_mqtt_client, reason_code=failed_connack_reason_code) + elif terminal_outcome == "disconnect_before_connack": + trigger_on_disconnect(mock_mqtt_client, reason_code=failed_disconnect_reason_code) + else: + trigger_on_connect(mock_mqtt_client) + trigger_on_disconnect(mock_mqtt_client, reason_code=failed_disconnect_reason_code) + return mqtt.MQTT_ERR_SUCCESS + + mock_mqtt_client.loop_start.side_effect = fail_during_loop_start + mock_mqtt_client.disconnect.side_effect = arbitrary_exception + + with pytest.raises(expected_error) as e_info: + transport.connect(fake_password) + + assert type(e_info.value) is expected_error + + @pytest.mark.it("Raises ConnectionTimeoutError and cleans up if CONNACK times out") + def test_connack_timeout(self, mock_mqtt_client, transport): + mock_mqtt_client.loop_start.side_effect = None + mock_mqtt_client.loop_start.return_value = mqtt.MQTT_ERR_SUCCESS + + with pytest.raises(errors.ConnectionTimeoutError): + transport.connect(fake_password, timeout=0.01) + + assert mock_mqtt_client.disconnect.call_count == 1 + assert mock_mqtt_client.loop_stop.call_count == 2 + + @pytest.mark.it("Preserves pending publish tracking when a connection attempt times out") + def test_connack_timeout_preserves_publish_tracking(self, mocker, mock_mqtt_client, transport): + publish_callback = mocker.MagicMock() + transport.publish(fake_topic, fake_payload, callback=publish_callback) + mock_mqtt_client.loop_start.side_effect = None + mock_mqtt_client.loop_start.return_value = mqtt.MQTT_ERR_SUCCESS + + with pytest.raises(errors.ConnectionTimeoutError): + transport.connect(fake_password, timeout=0.01) + + assert publish_callback.call_count == 0 + + trigger_on_publish(mock_mqtt_client, fake_mid) + + assert publish_callback.call_count == 1 + assert publish_callback.call_args == mocker.call() + + @pytest.mark.it("Times out immediately with a zero timeout if no CONNACK was received") + def test_zero_timeout_without_connack(self, mock_mqtt_client, transport): + mock_mqtt_client.loop_start.side_effect = None + mock_mqtt_client.loop_start.return_value = mqtt.MQTT_ERR_SUCCESS + + with pytest.raises(errors.ConnectionTimeoutError): + transport.connect(fake_password, timeout=0) + + @pytest.mark.it("Accepts a CONNACK received before a zero timeout is evaluated") + def test_zero_timeout_with_connack(self, mock_mqtt_client, transport): + transport.connect(fake_password, timeout=0) + + @pytest.mark.parametrize( + "reason_code", + [successful_connack_reason_code, failed_connack_reason_code], + ids=["Accepted CONNACK", "Rejected CONNACK"], ) - def test_calls_event_handler_callback_with_failed_rc( - self, mocker, mock_mqtt_client, transport, error_params + @pytest.mark.it("Ignores a CONNACK received after the connection attempt times out") + def test_connack_after_timeout_is_ignored( + self, mocker, mock_mqtt_client, transport, reason_code ): - callback = mocker.MagicMock() - transport.on_mqtt_connection_failure_handler = callback + mock_mqtt_client.loop_start.side_effect = None + mock_mqtt_client.loop_start.return_value = mqtt.MQTT_ERR_SUCCESS + connection_dropped_handler = mocker.MagicMock() + transport.on_mqtt_connection_dropped_handler = connection_dropped_handler - # Initiate connect - transport.connect(fake_password) + def disconnect_after_timeout(): + trigger_on_connect(mock_mqtt_client, reason_code=reason_code) + return mqtt.MQTT_ERR_SUCCESS - # Manually trigger Paho on_connect event_handler - mock_mqtt_client.on_connect( - client=mock_mqtt_client, userdata=None, flags=None, rc=error_params["rc"] - ) + mock_mqtt_client.disconnect.side_effect = disconnect_after_timeout - # Verify transport.on_mqtt_connection_failure_handler was called - assert callback.call_count == 1 - assert isinstance(callback.call_args[0][0], error_params["error"]) + with pytest.raises(errors.ConnectionTimeoutError): + transport.connect(fake_password, timeout=0.01) + + trigger_on_disconnect(mock_mqtt_client, reason_code=failed_disconnect_reason_code) + + assert transport._connection_lifecycle._state is ConnectionState.DISCONNECTED + assert transport._connection_lifecycle._error is None + assert connection_dropped_handler.call_count == 0 @pytest.mark.it( - "Stops Paho's network loop if the MQTTTransport was garbage collected before a failed connect completed" + "Suppresses the Paho disconnect callback during timeout cleanup and restores it afterward" ) - def test_stops_loop_after_gc(self, mocker, mock_mqtt_client, collected_transport_weakref): - mock_mqtt_client.on_connect( - client=mock_mqtt_client, userdata=None, flags=None, rc=failed_connack_rc - ) + def test_connack_timeout_suppresses_cleanup_disconnect_callback( + self, mock_mqtt_client, transport + ): + mock_mqtt_client.loop_start.side_effect = None + mock_mqtt_client.loop_start.return_value = mqtt.MQTT_ERR_SUCCESS + paho_on_disconnect = mock_mqtt_client.on_disconnect - assert mock_mqtt_client.loop_stop.call_count == 1 - assert mock_mqtt_client.loop_stop.call_args == mocker.call() + def disconnect_while_checking_callback(): + assert mock_mqtt_client.on_disconnect is None + return mqtt.MQTT_ERR_SUCCESS - @pytest.mark.it( - "Skips on_mqtt_connection_failure_handler event handler if set to 'None' upon failed connect completion" + mock_mqtt_client.disconnect.side_effect = disconnect_while_checking_callback + + with pytest.raises(errors.ConnectionTimeoutError): + transport.connect(fake_password, timeout=0.01) + + assert mock_mqtt_client.on_disconnect is paho_on_disconnect + + @pytest.mark.parametrize( + "cleanup_failure", + ["disconnect", "loop_stop"], + ids=["Paho disconnect raises", "Paho loop_stop raises"], ) - def test_skips_none_event_handler_callback(self, mocker, mock_mqtt_client, transport): - assert transport.on_mqtt_connection_failure_handler is None + @pytest.mark.it("Preserves the timeout if timeout cleanup raises an Exception") + def test_connack_timeout_preserves_error_if_cleanup_raises( + self, + mock_mqtt_client, + transport, + arbitrary_exception, + cleanup_failure, + ): + mock_mqtt_client.loop_start.side_effect = None + mock_mqtt_client.loop_start.return_value = mqtt.MQTT_ERR_SUCCESS + if cleanup_failure == "disconnect": + mock_mqtt_client.disconnect.side_effect = arbitrary_exception + else: + mock_mqtt_client.loop_stop.side_effect = [None, arbitrary_exception] - transport.connect(fake_password) + with pytest.raises(errors.ConnectionTimeoutError): + transport.connect(fake_password, timeout=0.01) - mock_mqtt_client.on_connect( - client=mock_mqtt_client, userdata=None, flags=None, rc=failed_connack_rc + assert mock_mqtt_client.on_disconnect is not None + assert transport._connection_lifecycle._state is ConnectionState.DISCONNECTED + assert transport._connection_lifecycle._error is None + + @pytest.mark.it("Can connect successfully after a connection attempt times out") + def test_connect_after_timeout(self, mock_mqtt_client, transport): + mock_mqtt_client.loop_start.side_effect = None + mock_mqtt_client.loop_start.return_value = mqtt.MQTT_ERR_SUCCESS + + with pytest.raises(errors.ConnectionTimeoutError): + transport.connect(fake_password, timeout=0.01) + + mock_mqtt_client.loop_start.side_effect = lambda: ( + trigger_on_connect(mock_mqtt_client) or mqtt.MQTT_ERR_SUCCESS ) - # No further asserts required - this is a test to show that it skips a callback. - # Not raising an exception == test passed + transport.connect(fake_password) - @pytest.mark.it("Recovers from Exception in on_mqtt_connection_failure_handler event handler") - def test_event_handler_callback_raises_exception( - self, mocker, mock_mqtt_client, transport, arbitrary_exception - ): - event_cb = mocker.MagicMock(side_effect=arbitrary_exception) - transport.on_mqtt_connection_failure_handler = event_cb + assert mock_mqtt_client.connect.call_count == 2 + + @pytest.mark.it("Reuses connection lifecycle state across connections") + def test_reuses_connection_lifecycle(self, mock_mqtt_client, transport): + lifecycle = transport._connection_lifecycle transport.connect(fake_password) - mock_mqtt_client.on_connect( - client=mock_mqtt_client, userdata=None, flags=None, rc=failed_connack_rc + trigger_on_disconnect(mock_mqtt_client) + transport.connect(fake_password) + + assert transport._connection_lifecycle is lifecycle + + @pytest.mark.parametrize( + "reason_code", + [successful_connack_reason_code, failed_connack_reason_code], + ids=["Accepted CONNACK", "Rejected CONNACK"], + ) + @pytest.mark.it("Does not apply a prior attempt's late CONNACK to a retry") + def test_prior_connack_during_loop_join_does_not_complete_retry( + self, + mock_mqtt_client, + transport, + run_in_daemon_thread, + poll_until, + reason_code, + ): + mock_mqtt_client.loop_start.side_effect = None + mock_mqtt_client.loop_start.return_value = mqtt.MQTT_ERR_SUCCESS + + with pytest.raises(errors.ConnectionTimeoutError): + transport.connect(fake_password, timeout=0.01) + + prior_lifecycle = transport._connection_lifecycle + mock_mqtt_client.loop_stop.side_effect = lambda: trigger_on_connect( + mock_mqtt_client, reason_code=reason_code ) + retry_future = run_in_daemon_thread(transport.connect, fake_password, timeout=1) + poll_until(lambda: mock_mqtt_client.loop_start.call_count == 2, timeout=1) - # Callback was called, but exception did not propagate - assert event_cb.call_count == 1 + assert transport._connection_lifecycle is prior_lifecycle + assert not retry_future.done() + + trigger_on_connect(mock_mqtt_client) + retry_future.result(timeout=1) + +@pytest.mark.describe("MQTTTransport - OCCURRENCE: CONNACK after transport collection") +class TestConnackAfterTransportCollection(object): @pytest.mark.it( - "Allows any BaseExceptions raised in on_mqtt_connection_failure_handler event handler to propagate" + "Stops Paho's network loop if the MQTTTransport was garbage collected before CONNACK" ) - def test_event_handler_callback_raises_base_exception( - self, mocker, mock_mqtt_client, transport, arbitrary_base_exception + @pytest.mark.parametrize( + "reason_code", + [successful_connack_reason_code, failed_connack_reason_code], + ids=["Successful CONNACK", "Failed CONNACK"], + ) + def test_stops_loop_after_gc( + self, mocker, mock_mqtt_client, collected_transport_weakref, reason_code ): - event_cb = mocker.MagicMock(side_effect=arbitrary_base_exception) - transport.on_mqtt_connection_failure_handler = event_cb + trigger_on_connect(mock_mqtt_client, reason_code=reason_code) - transport.connect(fake_password) - with pytest.raises(arbitrary_base_exception.__class__) as e_info: - mock_mqtt_client.on_connect( - client=mock_mqtt_client, userdata=None, flags=None, rc=failed_connack_rc - ) - assert e_info.value is arbitrary_base_exception + assert mock_mqtt_client.loop_stop.call_count == 1 + assert mock_mqtt_client.loop_stop.call_args == mocker.call() @pytest.mark.describe("MQTTTransport - .disconnect()") @@ -823,6 +1411,26 @@ def test_calls_paho_disconnect(self, mocker, mock_mqtt_client, transport): assert mock_mqtt_client.disconnect.call_count == 1 assert mock_mqtt_client.disconnect.call_args == mocker.call() + @pytest.mark.it("Serializes disconnect behind an in-progress connect") + def test_serializes_behind_connect( + self, mock_mqtt_client, transport, run_in_daemon_thread, poll_until + ): + mock_mqtt_client.loop_start.side_effect = None + mock_mqtt_client.loop_start.return_value = mqtt.MQTT_ERR_SUCCESS + connect_future = run_in_daemon_thread(transport.connect, fake_password, timeout=1) + poll_until(lambda: mock_mqtt_client.loop_start.call_count == 1, timeout=1) + + disconnect_future = run_in_daemon_thread(transport.disconnect) + poll_until(lambda: disconnect_future.running(), timeout=1) + + assert mock_mqtt_client.disconnect.call_count == 0 + + trigger_on_connect(mock_mqtt_client) + connect_future.result(timeout=1) + disconnect_future.result(timeout=1) + + assert mock_mqtt_client.disconnect.call_count == 1 + @pytest.mark.it( "Raises a ProtocolClientError if Paho disconnect raises an unexpected Exception" ) @@ -843,24 +1451,47 @@ def test_client_raises_base_exception( transport.disconnect() assert e_info.value is arbitrary_base_exception - @pytest.mark.it("Raises a custom Exception if Paho disconnect returns a failing rc code") + @pytest.mark.it("Raises a custom Exception if Paho disconnect returns an error code") @pytest.mark.parametrize( - "error_params", - disconnect_operation_return_codes, + "error_case", + disconnect_error_code_cases, ids=[ - "{}->{}".format(x["name"], x["error"].__name__) - for x in disconnect_operation_return_codes + "{}->{}".format(case["name"], case["error"].__name__) + for case in disconnect_error_code_cases ], ) - def test_client_returns_failing_rc_code( - self, mocker, mock_mqtt_client, transport, error_params - ): - mock_mqtt_client.disconnect.return_value = error_params["rc"] - with pytest.raises(error_params["error"]): + def test_client_returns_error_code(self, mocker, mock_mqtt_client, transport, error_case): + mock_mqtt_client.disconnect.return_value = error_case["error_code"] + with pytest.raises(error_case["error"]) as e_info: transport.disconnect() + assert str(e_info.value) == mqtt.error_string(error_case["error_code"]) + + @pytest.mark.it("Treats MQTT_ERR_NO_CONN as a successful disconnect") + def test_no_connection_error_code(self, mock_mqtt_client, transport): + transport.connect(fake_password) + mock_mqtt_client.disconnect.return_value = mqtt.MQTT_ERR_NO_CONN + + transport.disconnect() + + assert transport._connection_lifecycle._state is ConnectionState.DISCONNECTED + + @pytest.mark.it( + "Completes tracked operations as cancelled after an already-completed disconnect" + ) + def test_no_connection_error_code_clears_inflight(self, mocker, mock_mqtt_client, transport): + callback = mocker.MagicMock() + transport.subscribe(topic=fake_topic, qos=fake_qos, callback=callback) + mock_mqtt_client.disconnect.return_value = mqtt.MQTT_ERR_NO_CONN + + transport.disconnect(clear_inflight=True) - @pytest.mark.it("Cancels all pending operations if the clear_inflight parameter is True") - def test_pending_op_cancellation(self, mocker, mock_mqtt_client, transport): + assert callback.call_count == 1 + assert callback.call_args == mocker.call(cancelled=True) + + @pytest.mark.it( + "Completes tracked operations as cancelled if the clear_inflight parameter is True" + ) + def test_clear_inflight_completes_tracked_operations(self, mocker, mock_mqtt_client, transport): # Set up a pending publish pub_callback = mocker.MagicMock(name="pub cb") pub_mid = "1" @@ -879,19 +1510,36 @@ def test_pending_op_cancellation(self, mocker, mock_mqtt_client, transport): assert pub_callback.call_count == 0 assert sub_callback.call_count == 0 - # Disconnect and clear pending ops + # Disconnect and clear tracked operations transport.disconnect(clear_inflight=True) - # Pending operations were cancelled + # Tracked operations were completed as cancelled assert pub_callback.call_count == 1 assert pub_callback.call_args == mocker.call(cancelled=True) assert sub_callback.call_count == 1 assert sub_callback.call_args == mocker.call(cancelled=True) @pytest.mark.it( - "Does not cancel any pending operations if the clear_inflight parameter is False" + "Invokes cancellation callbacks after releasing the lifecycle lock on hard disconnect" + ) + def test_clear_inflight_callback_after_lifecycle_lock_release( + self, mock_mqtt_client, transport + ): + callback_probe = CancellationCallbackLockProbe(transport._connection_lock) + transport.subscribe(fake_topic, callback=callback_probe) + + transport.disconnect(clear_inflight=True) + + assert callback_probe.call_count == 1 + assert callback_probe.cancelled is True + assert callback_probe.lock_was_held is False + + @pytest.mark.it( + "Preserves resumable publish tracking and cancels non-resumable operations if clear_inflight is False" ) - def test_no_pending_op_cancellation(self, mocker, mock_mqtt_client, transport): + def test_clear_inflight_false_preserves_resumable_publish_tracking( + self, mocker, mock_mqtt_client, transport + ): # Set up a pending publish pub_callback = mocker.MagicMock(name="pub cb") pub_mid = "1" @@ -913,14 +1561,42 @@ def test_no_pending_op_cancellation(self, mocker, mock_mqtt_client, transport): # Disconnect transport.disconnect(clear_inflight=False) - # No pending operations were cancelled + # Only resumable publish tracking remains pending assert pub_callback.call_count == 0 - assert sub_callback.call_count == 0 + assert sub_callback.call_args == mocker.call(cancelled=True) + assert list(transport._op_manager._pending_operations) == [pub_mid] + assert transport._op_manager._pending_operations[pub_mid].callback is pub_callback + assert ( + transport._op_manager._pending_operations[pub_mid].operation_type + is OperationType.PUBLISH_QOS_1 + ) + assert transport._op_manager._cancelled_operation_mids == {sub_mid} @pytest.mark.it( - "Does not cancel any pending operations if the clear_inflight parameter is not provided" + "Completes QoS 0 publish as cancelled after releasing the lifecycle lock if clear_inflight is False" ) - def test_default_no_pending_op_cancellation(self, mocker, mock_mqtt_client, transport): + def test_clear_inflight_false_cancels_qos_zero_publish(self, mock_mqtt_client, transport): + callback_probe = CancellationCallbackLockProbe(transport._connection_lock) + mid = "1" + message_info = mqtt.MQTTMessageInfo(mid) + message_info.rc = fake_rc + mock_mqtt_client.publish.return_value = message_info + transport.publish(topic=fake_topic, payload=fake_payload, qos=0, callback=callback_probe) + + transport.disconnect(clear_inflight=False) + + assert callback_probe.call_count == 1 + assert callback_probe.cancelled is True + assert callback_probe.lock_was_held is False + assert transport._op_manager._pending_operations == {} + assert transport._op_manager._cancelled_operation_mids == {mid} + + @pytest.mark.it( + "Preserves resumable publish tracking and cancels non-resumable operations by default" + ) + def test_default_preserves_resumable_publish_tracking( + self, mocker, mock_mqtt_client, transport + ): # Set up a pending publish pub_callback = mocker.MagicMock(name="pub cb") pub_mid = "1" @@ -942,9 +1618,16 @@ def test_default_no_pending_op_cancellation(self, mocker, mock_mqtt_client, tran # Disconnect transport.disconnect() - # No pending operations were cancelled + # Only resumable publish tracking remains pending assert pub_callback.call_count == 0 - assert sub_callback.call_count == 0 + assert sub_callback.call_args == mocker.call(cancelled=True) + assert list(transport._op_manager._pending_operations) == [pub_mid] + assert transport._op_manager._pending_operations[pub_mid].callback is pub_callback + assert ( + transport._op_manager._pending_operations[pub_mid].operation_type + is OperationType.PUBLISH_QOS_1 + ) + assert transport._op_manager._cancelled_operation_mids == {sub_mid} @pytest.mark.it("Stops MQTT Network Loop when disconnect does not raise an exception") def test_calls_loop_stop_on_success(self, mocker, mock_mqtt_client, transport): @@ -965,290 +1648,291 @@ def test_calls_loop_stop_on_exception( assert mock_mqtt_client.loop_stop.call_count == 1 assert mock_mqtt_client.loop_stop.call_args == mocker.call() - @pytest.mark.it( - "Sets Paho's _thread to None if disconnect does not raise an exception while running in the Paho thread" + +@pytest.mark.describe("MQTTTransport - OCCURRENCE: MQTT connection dropped") +class TestConnectionDropped(object): + @pytest.fixture( + params=[successful_disconnect_reason_code, failed_disconnect_reason_code], + ids=["success reason code", "failed reason code"], ) - def test_sets_thread_to_none_on_success_in_paho_thread( - self, mocker, mock_mqtt_client, transport, mock_paho_thread_current - ): - transport.disconnect() - assert mock_mqtt_client._thread is None + def reason_code_success_or_failure(self, request): + return request.param @pytest.mark.it( - "Sets Paho's _thread to None if disconnect raises an exception while running in the Paho thread" + "Synthesizes a ConnectionDroppedError if Paho reports a successful reason for a connection drop" ) - def test_sets_thread_to_none_on_exception_in_paho_thread( - self, mocker, mock_mqtt_client, transport, arbitrary_exception, mock_paho_thread_current - ): - mock_mqtt_client.disconnect.side_effect = arbitrary_exception + def test_success_reason_has_connection_dropped_error(self, mocker, mock_mqtt_client, transport): + connection_dropped_handler = mocker.MagicMock() + transport.on_mqtt_connection_dropped_handler = connection_dropped_handler + lifecycle = transport._connection_lifecycle - with pytest.raises(Exception): - transport.disconnect() - assert mock_mqtt_client._thread is None + transport.connect(fake_password) - @pytest.mark.it( - "Does not set Paho's _thread to None if disconnect does not raise an exception while running outside the Paho thread" - ) - def test_does_not_set_thread_to_none_on_success_in_non_paho_thread( - self, mocker, mock_mqtt_client, transport, mock_non_paho_thread_current - ): - transport.disconnect() - assert mock_mqtt_client._thread is not None + # Manually invoke Paho's on_disconnect callback. + trigger_on_disconnect(mock_mqtt_client) - @pytest.mark.it( - "Does not set Paho's _thread to None if disconnect raises an exception while running outside the Paho thread" + assert connection_dropped_handler.call_count == 1 + assert isinstance( + connection_dropped_handler.call_args.args[0], errors.ConnectionDroppedError + ) + assert ( + str(connection_dropped_handler.call_args.args[0]) + == "Network connection closed unexpectedly" + ) + assert transport._connection_lifecycle is lifecycle + assert transport._connection_lifecycle._state is ConnectionState.DISCONNECTED + + @pytest.mark.parametrize( + "error_case", + paho_disconnect_reason_error_cases, + ids=[ + "{}->{}".format(case["reason_code"], case["error"].__name__) + for case in paho_disconnect_reason_error_cases + ], ) - def test_does_not_set_thread_to_none_on_exception_in_non_paho_thread( - self, mocker, mock_mqtt_client, transport, arbitrary_exception, mock_non_paho_thread_current + @pytest.mark.it("Triggers on_mqtt_connection_dropped_handler with a ConnectionDroppedError") + def test_connection_dropped_handler_receives_failure( + self, mocker, mock_mqtt_client, transport, error_case ): - mock_mqtt_client.disconnect.side_effect = arbitrary_exception + connection_dropped_handler = mocker.MagicMock() + transport.on_mqtt_connection_dropped_handler = connection_dropped_handler - with pytest.raises(Exception): - transport.disconnect() - assert mock_mqtt_client._thread is not None + transport.connect(fake_password) + trigger_on_disconnect(mock_mqtt_client, reason_code=error_case["reason_code"]) -@pytest.mark.describe("MQTTTransport - OCCURRENCE: Disconnect Completed") -class TestEventDisconnectCompleted(object): - @pytest.fixture( - params=[fake_success_rc, fake_failed_rc], ids=["success rc code", "failed rc code"] - ) - def rc_success_or_failure(self, request): - return request.param + assert connection_dropped_handler.call_count == 1 + assert isinstance(connection_dropped_handler.call_args.args[0], error_case["error"]) + assert str(connection_dropped_handler.call_args.args[0]) == str(error_case["reason_code"]) - @pytest.mark.it( - "Triggers on_mqtt_disconnected_handler event handler upon disconnect completion" - ) - def test_calls_event_handler_callback_externally_driven( - self, mocker, mock_mqtt_client, transport + @pytest.mark.it("Does not report an explicit disconnect as a connection drop") + def test_explicit_disconnect_not_reported( + self, mocker, mock_mqtt_client, transport, reason_code_success_or_failure ): - callback = mocker.MagicMock() - transport.on_mqtt_disconnected_handler = callback + connection_dropped_handler = mocker.MagicMock() + transport.on_mqtt_connection_dropped_handler = connection_dropped_handler + lifecycle = transport._connection_lifecycle + transport.connect(fake_password) - # Initiate disconnect - transport.disconnect() + def disconnect_and_report_closure(): + trigger_on_disconnect(mock_mqtt_client, reason_code=reason_code_success_or_failure) + return mqtt.MQTT_ERR_SUCCESS - # Manually trigger Paho on_connect event_handler - mock_mqtt_client.on_disconnect(client=mock_mqtt_client, userdata=None, rc=fake_rc) + mock_mqtt_client.disconnect.side_effect = disconnect_and_report_closure - # Verify transport.on_mqtt_connected_handler was called - assert callback.call_count == 1 - assert callback.call_args == mocker.call(None) + transport.disconnect() - @pytest.mark.parametrize( - "error_params", - operation_return_codes, - ids=["{}->{}".format(x["name"], x["error"].__name__) for x in operation_return_codes], - ) - @pytest.mark.it( - "Triggers on_mqtt_disconnected_handler event handler with custom Exception when an error RC is returned upon disconnect completion." - ) - def test_calls_event_handler_callback_with_failure_user_driven( - self, mocker, mock_mqtt_client, transport, error_params - ): - callback = mocker.MagicMock() - transport.on_mqtt_disconnected_handler = callback + assert connection_dropped_handler.call_count == 0 + assert transport._connection_lifecycle is lifecycle + assert transport._connection_lifecycle._state is ConnectionState.DISCONNECTED + + @pytest.mark.it("Reports a connection drop that occurs before explicit disconnect begins") + def test_drop_before_explicit_disconnect(self, mocker, mock_mqtt_client, transport): + connection_dropped_handler = mocker.MagicMock() + transport.on_mqtt_connection_dropped_handler = connection_dropped_handler + transport.connect(fake_password) + trigger_on_disconnect(mock_mqtt_client, reason_code=failed_disconnect_reason_code) + mock_mqtt_client.disconnect.return_value = mqtt.MQTT_ERR_NO_CONN - # Initiate disconnect transport.disconnect() - # Manually trigger Paho on_disconnect event_handler - mock_mqtt_client.on_disconnect( - client=mock_mqtt_client, userdata=None, rc=error_params["rc"] + assert connection_dropped_handler.call_count == 1 + + @pytest.mark.it("Reports an already-classified drop before explicit disconnect returns") + def test_classified_drop_precedes_explicit_disconnect_return( + self, mocker, mock_mqtt_client, transport, run_in_daemon_thread + ): + call_order = [] + drop_classified = threading.Event() + release_drop_callback = threading.Event() + loop_stop_entered = threading.Event() + lifecycle = transport._connection_lifecycle + original_record_disconnection = lifecycle.record_disconnection + + def record_disconnection_then_pause(cause): + connection_dropped = original_record_disconnection(cause) + drop_classified.set() + assert release_drop_callback.wait(timeout=1) + return connection_dropped + + mocker.patch.object( + lifecycle, + "record_disconnection", + side_effect=record_disconnection_then_pause, ) + transport.on_mqtt_connection_dropped_handler = lambda cause: call_order.append( + "drop reported" + ) + transport.connect(fake_password) - # Verify transport.on_mqtt_disconnected_handler was called - assert callback.call_count == 1 - assert isinstance(callback.call_args[0][0], error_params["error"]) + drop_future = run_in_daemon_thread( + trigger_on_disconnect, + mock_mqtt_client, + failed_disconnect_reason_code, + ) + assert drop_classified.wait(timeout=1) - @pytest.mark.it( - "Skips on_mqtt_disconnected_handler event handler if set to 'None' upon disconnect completion" - ) - def test_skips_none_event_handler_callback(self, mocker, mock_mqtt_client, transport): - assert transport.on_mqtt_disconnected_handler is None + # Paho loop_stop() joins its network thread. Model that blocking contract so an + # explicit disconnect cannot return while the Paho callback is paused after + # classification but before reporting the drop. + mock_mqtt_client.disconnect.return_value = mqtt.MQTT_ERR_NO_CONN - transport.disconnect() + def join_drop_callback(): + loop_stop_entered.set() + return drop_future.result(timeout=1) - mock_mqtt_client.on_disconnect(client=mock_mqtt_client, userdata=None, rc=fake_rc) + mock_mqtt_client.loop_stop.side_effect = join_drop_callback - # No further asserts required - this is a test to show that it skips a callback. - # Not raising an exception == test passed + def disconnect_and_record_return(): + transport.disconnect() + call_order.append("explicit disconnect returned") - @pytest.mark.it("Recovers from Exception in on_mqtt_disconnected_handler event handler") - def test_event_handler_callback_raises_exception( - self, mocker, mock_mqtt_client, transport, arbitrary_exception - ): - event_cb = mocker.MagicMock(side_effect=arbitrary_exception) - transport.on_mqtt_disconnected_handler = event_cb + disconnect_future = run_in_daemon_thread(disconnect_and_record_return) + try: + assert loop_stop_entered.wait(timeout=1) + assert not disconnect_future.done() + assert call_order == [] + finally: + release_drop_callback.set() - transport.disconnect() - mock_mqtt_client.on_disconnect(client=mock_mqtt_client, userdata=None, rc=fake_rc) + drop_future.result(timeout=1) + disconnect_future.result(timeout=1) - # Callback was called, but exception did not propagate - assert event_cb.call_count == 1 + assert call_order == ["drop reported", "explicit disconnect returned"] - @pytest.mark.it( - "Allows any BaseExceptions raised in on_mqtt_disconnected_handler event handler to propagate" - ) - def test_event_handler_callback_raises_base_exception( - self, mocker, mock_mqtt_client, transport, arbitrary_base_exception + @pytest.mark.it("Reports one connection drop when Paho invokes on_disconnect more than once") + def test_reports_one_drop_for_duplicate_paho_callbacks( + self, mocker, mock_mqtt_client, transport ): - event_cb = mocker.MagicMock(side_effect=arbitrary_base_exception) - transport.on_mqtt_disconnected_handler = event_cb - - transport.disconnect() - with pytest.raises(arbitrary_base_exception.__class__) as e_info: - mock_mqtt_client.on_disconnect(client=mock_mqtt_client, userdata=None, rc=fake_rc) - assert e_info.value is arbitrary_base_exception + connection_dropped_handler = mocker.MagicMock() + transport.on_mqtt_connection_dropped_handler = connection_dropped_handler - @pytest.mark.it("Calls Paho's disconnect() method if cause is not None") - def test_calls_disconnect_with_cause(self, mock_mqtt_client, transport): - mock_mqtt_client.on_disconnect(client=mock_mqtt_client, userdata=None, rc=fake_failed_rc) - assert mock_mqtt_client.disconnect.call_count == 1 + transport.connect(fake_password) + trigger_on_disconnect(mock_mqtt_client, reason_code=failed_disconnect_reason_code) + trigger_on_disconnect(mock_mqtt_client, reason_code=failed_disconnect_reason_code) - @pytest.mark.it("Does not call Paho's disconnect() method if cause is None") - def test_doesnt_call_disconnect_without_cause(self, mock_mqtt_client, transport): - mock_mqtt_client.on_disconnect(client=mock_mqtt_client, userdata=None, rc=fake_success_rc) - assert mock_mqtt_client.disconnect.call_count == 0 + assert connection_dropped_handler.call_count == 1 - @pytest.mark.it("Calls Paho's loop_stop() if cause is not None") - def test_calls_loop_stop(self, mock_mqtt_client, transport): - mock_mqtt_client.on_disconnect(client=mock_mqtt_client, userdata=None, rc=fake_failed_rc) - assert mock_mqtt_client.loop_stop.call_count == 1 + transport.connect(fake_password) + trigger_on_disconnect(mock_mqtt_client, reason_code=failed_disconnect_reason_code) - @pytest.mark.it("Does not calls Paho's loop_stop() if cause is None") - def test_does_not_call_loop_stop(self, mock_mqtt_client, transport): - mock_mqtt_client.on_disconnect(client=mock_mqtt_client, userdata=None, rc=fake_success_rc) - assert mock_mqtt_client.loop_stop.call_count == 0 + assert connection_dropped_handler.call_count == 2 - @pytest.mark.it( - "Sets Paho's _thread to None if cause is not None while running in the Paho thread" - ) - def test_sets_thread_to_none_on_failure_in_paho_thread( - self, mock_mqtt_client, transport, mock_paho_thread_current + @pytest.mark.it("Skips on_mqtt_connection_dropped_handler if it is not configured") + def test_skips_unconfigured_connection_dropped_handler( + self, mocker, mock_mqtt_client, transport ): - mock_mqtt_client.on_disconnect(client=mock_mqtt_client, userdata=None, rc=fake_failed_rc) - assert mock_mqtt_client._thread is None + assert transport.on_mqtt_connection_dropped_handler is None - @pytest.mark.it( - "Does not set Paho's _thread to None if cause is not None while running outside the paho thread" - ) - def test_sets_thread_to_none_on_failure_in_non_paho_thread( - self, mock_mqtt_client, transport, mock_non_paho_thread_current - ): - mock_mqtt_client.on_disconnect(client=mock_mqtt_client, userdata=None, rc=fake_failed_rc) - assert mock_mqtt_client._thread is not None + transport.connect(fake_password) - @pytest.mark.it( - "Does not sets Paho's _thread to None if cause is None while running in the Paho thread" - ) - def test_does_not_set_thread_to_none_on_success_in_paho_thread( - self, mock_mqtt_client, transport, mock_paho_thread_current - ): - mock_mqtt_client.on_disconnect(client=mock_mqtt_client, userdata=None, rc=fake_success_rc) - assert mock_mqtt_client._thread is not None + trigger_on_disconnect(mock_mqtt_client) - @pytest.mark.it( - "Does not sets Paho's _thread to None if cause is None while running outside the Paho thread" - ) - def test_does_not_set_thread_to_none_on_success_in_non_paho_thread( - self, mock_mqtt_client, transport, mock_non_paho_thread_current - ): - mock_mqtt_client.on_disconnect(client=mock_mqtt_client, userdata=None, rc=fake_success_rc) - assert mock_mqtt_client._thread is not None + # No further asserts required - this is a test to show that it skips a callback. + # Not raising an exception == test passed - @pytest.mark.it("Allows any Exception raised by Paho's disconnect() to propagate") - def test_disconnect_raises_exception( - self, mock_mqtt_client, transport, mocker, arbitrary_exception + @pytest.mark.it("Recovers from Exception in on_mqtt_connection_dropped_handler") + def test_connection_dropped_handler_raises_exception( + self, mocker, mock_mqtt_client, transport, arbitrary_exception ): - mock_mqtt_client.disconnect = mocker.MagicMock(side_effect=arbitrary_exception) - with pytest.raises(type(arbitrary_exception)) as e_info: - mock_mqtt_client.on_disconnect( - client=mock_mqtt_client, userdata=None, rc=fake_failed_rc - ) - assert e_info.value is arbitrary_exception + connection_dropped_handler = mocker.MagicMock(side_effect=arbitrary_exception) + transport.on_mqtt_connection_dropped_handler = connection_dropped_handler + + transport.connect(fake_password) + trigger_on_disconnect(mock_mqtt_client) - @pytest.mark.it("Allows any BaseException raised by Paho's disconnect() to propagate") - def test_disconnect_raises_base_exception( - self, mock_mqtt_client, transport, mocker, arbitrary_base_exception - ): - mock_mqtt_client.disconnect = mocker.MagicMock(side_effect=arbitrary_base_exception) - with pytest.raises(type(arbitrary_base_exception)) as e_info: - mock_mqtt_client.on_disconnect( - client=mock_mqtt_client, userdata=None, rc=fake_failed_rc - ) - assert e_info.value is arbitrary_base_exception + # Callback was called, but exception did not propagate + assert connection_dropped_handler.call_count == 1 - @pytest.mark.it("Allows any Exception raised by Paho's loop_stop() to propagate") - def test_loop_stop_raises_exception( - self, mock_mqtt_client, transport, mocker, arbitrary_exception + @pytest.mark.it( + "Allows any BaseExceptions raised in on_mqtt_connection_dropped_handler to propagate" + ) + def test_connection_dropped_handler_raises_base_exception( + self, mocker, mock_mqtt_client, transport, arbitrary_base_exception ): - mock_mqtt_client.loop_stop = mocker.MagicMock(side_effect=arbitrary_exception) - with pytest.raises(type(arbitrary_exception)) as e_info: - mock_mqtt_client.on_disconnect( - client=mock_mqtt_client, userdata=None, rc=fake_failed_rc - ) - assert e_info.value is arbitrary_exception + connection_dropped_handler = mocker.MagicMock(side_effect=arbitrary_base_exception) + transport.on_mqtt_connection_dropped_handler = connection_dropped_handler - @pytest.mark.it("Allows any BaseException raised by Paho's loop_stop() to propagate") - def test_loop_stop_raises_base_exception( - self, mock_mqtt_client, transport, mocker, arbitrary_base_exception - ): - mock_mqtt_client.loop_stop = mocker.MagicMock(side_effect=arbitrary_base_exception) - with pytest.raises(type(arbitrary_base_exception)) as e_info: - mock_mqtt_client.on_disconnect( - client=mock_mqtt_client, userdata=None, rc=fake_failed_rc - ) + transport.connect(fake_password) + with pytest.raises(arbitrary_base_exception.__class__) as e_info: + trigger_on_disconnect(mock_mqtt_client) assert e_info.value is arbitrary_base_exception + @pytest.mark.it("Does not call Paho's disconnect() method after a connection drop") + def test_doesnt_call_disconnect(self, mock_mqtt_client, transport): + transport.connect(fake_password) + trigger_on_disconnect(mock_mqtt_client) + assert mock_mqtt_client.disconnect.call_count == 0 + + @pytest.mark.it("Does not call Paho's loop_stop() after a connection drop") + def test_does_not_call_loop_stop(self, mock_mqtt_client, transport): + transport.connect(fake_password) + mock_mqtt_client.loop_stop.reset_mock() + trigger_on_disconnect(mock_mqtt_client) + assert mock_mqtt_client.loop_stop.call_count == 0 + + @pytest.mark.it("Does not stop or reconnect Paho after a connection drop") + def test_does_not_stop_or_reconnect_paho_after_failure(self, mock_mqtt_client, transport): + transport.connect(fake_password) + mock_mqtt_client.loop_stop.reset_mock() + trigger_on_disconnect(mock_mqtt_client, reason_code=failed_disconnect_reason_code) + + assert mock_mqtt_client.disconnect.call_count == 0 + assert mock_mqtt_client.loop_stop.call_count == 0 + assert mock_mqtt_client.reconnect.call_count == 0 + @pytest.mark.it( - "Does not raise any exceptions if the MQTTTransport object was garbage collected before the disconnect completed" + "Does not raise if MQTTTransport is collected before Paho invokes on_disconnect" ) def test_no_exception_after_gc( - self, mock_mqtt_client, collected_transport_weakref, rc_success_or_failure + self, mock_mqtt_client, collected_transport_weakref, reason_code_success_or_failure ): assert mock_mqtt_client.on_disconnect - mock_mqtt_client.on_disconnect(mock_mqtt_client, None, rc_success_or_failure) + trigger_on_disconnect(mock_mqtt_client, reason_code=reason_code_success_or_failure) # lack of exception is success @pytest.mark.it( - "Calls Paho's loop_stop() if the MQTTTransport object was garbage collected before the disconnect completed" + "Calls Paho's loop_stop() if MQTTTransport is collected before Paho invokes on_disconnect" ) def test_calls_loop_stop_after_gc( - self, collected_transport_weakref, mock_mqtt_client, rc_success_or_failure, mocker + self, + collected_transport_weakref, + mock_mqtt_client, + reason_code_success_or_failure, + mocker, ): assert mock_mqtt_client.loop_stop.call_count == 0 - mock_mqtt_client.on_disconnect(mock_mqtt_client, None, rc_success_or_failure) + trigger_on_disconnect(mock_mqtt_client, reason_code=reason_code_success_or_failure) assert mock_mqtt_client.loop_stop.call_count == 1 assert mock_mqtt_client.loop_stop.call_args == mocker.call() @pytest.mark.it( - "Allows any Exception raised by Paho's loop_stop() to propagate if the MQTTTransport object was garbage collected before the disconnect completed" + "Allows any Exception from Paho's loop_stop() to propagate after MQTTTransport collection" ) def test_raises_exception_after_gc( self, collected_transport_weakref, mock_mqtt_client, - rc_success_or_failure, + reason_code_success_or_failure, arbitrary_exception, ): mock_mqtt_client.loop_stop.side_effect = arbitrary_exception with pytest.raises(type(arbitrary_exception)): - mock_mqtt_client.on_disconnect(mock_mqtt_client, None, rc_success_or_failure) + trigger_on_disconnect(mock_mqtt_client, reason_code=reason_code_success_or_failure) @pytest.mark.it( - "Allows any BaseException raised by Paho's loop_stop() to propagate if the MQTTTransport object was garbage collected before the disconnect completed" + "Allows any BaseException from Paho's loop_stop() to propagate after MQTTTransport collection" ) def test_raises_base_exception_after_gc( self, collected_transport_weakref, mock_mqtt_client, - rc_success_or_failure, + reason_code_success_or_failure, arbitrary_base_exception, ): mock_mqtt_client.loop_stop.side_effect = arbitrary_base_exception with pytest.raises(type(arbitrary_base_exception)): - mock_mqtt_client.on_disconnect(mock_mqtt_client, None, rc_success_or_failure) + trigger_on_disconnect(mock_mqtt_client, reason_code=reason_code_success_or_failure) @pytest.mark.describe("MQTTTransport - .subscribe()") @@ -1264,6 +1948,16 @@ def test_calls_paho_subscribe(self, mocker, mock_mqtt_client, transport, qos): assert mock_mqtt_client.subscribe.call_count == 1 assert mock_mqtt_client.subscribe.call_args == mocker.call(fake_topic, qos=qos) + @pytest.mark.it("Tracks the operation as a SUBSCRIBE") + def test_tracks_subscribe_operation_type(self, mocker, transport): + callback = mocker.MagicMock() + + transport.subscribe(fake_topic, callback=callback) + + pending_operation = transport._op_manager._pending_operations[fake_mid] + assert pending_operation.operation_type is OperationType.SUBSCRIBE + assert pending_operation.callback is callback + @pytest.mark.it("Raises ValueError on invalid QoS") @pytest.mark.parametrize("qos", [pytest.param(-1, id="QoS < 0"), pytest.param(3, id="QoS > 2")]) def test_raises_value_error_invalid_qos(self, qos): @@ -1285,8 +1979,16 @@ def test_raises_value_error_invalid_topic(self, topic): transport.subscribe(topic, qos=fake_qos) @pytest.mark.it("Triggers callback upon subscribe completion") + @pytest.mark.parametrize( + "suback_return_code", + [ + pytest.param(0x00, id="Maximum QoS 0"), + pytest.param(0x01, id="Maximum QoS 1"), + pytest.param(0x02, id="Maximum QoS 2"), + ], + ) def test_triggers_callback_upon_paho_on_subscribe_event( - self, mocker, mock_mqtt_client, transport + self, mocker, mock_mqtt_client, transport, suback_return_code ): callback = mocker.MagicMock() mock_mqtt_client.subscribe.return_value = (fake_rc, fake_mid) @@ -1298,20 +2000,30 @@ def test_triggers_callback_upon_paho_on_subscribe_event( assert callback.call_count == 0 # Manually trigger Paho on_subscribe event handler - mock_mqtt_client.on_subscribe( - client=mock_mqtt_client, userdata=None, mid=fake_mid, granted_qos=fake_qos - ) + granted_qos = mqtt.ReasonCode(PacketTypes.SUBACK, identifier=suback_return_code) + trigger_on_subscribe(mock_mqtt_client, mid=fake_mid, reason_codes=[granted_qos]) # Check callback has now been called assert callback.call_count == 1 + assert callback.call_args == mocker.call() + + @pytest.mark.it("Completes a subscription with ProtocolClientError if any reason fails") + def test_failed_suback(self, mocker, mock_mqtt_client, transport): + callback = mocker.MagicMock() + transport.subscribe(topic=fake_topic, qos=fake_qos, callback=callback) + granted = mqtt.ReasonCode(PacketTypes.SUBACK, identifier=1) + rejected = mqtt.ReasonCode(PacketTypes.SUBACK, identifier=128) + + trigger_on_subscribe(mock_mqtt_client, mid=fake_mid, reason_codes=[granted, rejected]) + + assert callback.call_count == 1 + assert isinstance(callback.call_args.kwargs["error"], errors.ProtocolClientError) @pytest.mark.it( "Stops Paho's network loop if the MQTTTransport was garbage collected before subscribe completed" ) def test_stops_loop_after_gc(self, mocker, mock_mqtt_client, collected_transport_weakref): - mock_mqtt_client.on_subscribe( - client=mock_mqtt_client, userdata=None, mid=fake_mid, granted_qos=fake_qos - ) + trigger_on_subscribe(mock_mqtt_client, mid=fake_mid) assert mock_mqtt_client.loop_stop.call_count == 1 assert mock_mqtt_client.loop_stop.call_args == mocker.call() @@ -1327,9 +2039,7 @@ def test_triggers_callback_when_paho_on_subscribe_event_called_early( def trigger_early_on_subscribe(topic, qos): # Trigger on_subscribe before returning mid - mock_mqtt_client.on_subscribe( - client=mock_mqtt_client, userdata=None, mid=fake_mid, granted_qos=fake_qos - ) + trigger_on_subscribe(mock_mqtt_client, mid=fake_mid) # Check callback not yet called assert callback.call_count == 0 @@ -1344,6 +2054,46 @@ def trigger_early_on_subscribe(topic, qos): # Check callback has now been called assert callback.call_count == 1 + @pytest.mark.it( + "Triggers callback when a cancelled MID is reused and Paho completes before subscribe returns" + ) + def test_triggers_callback_when_cancelled_mid_reused_and_completed_early( + self, mocker, mock_mqtt_client, transport + ): + transport.subscribe(topic=fake_topic, qos=fake_qos) + transport._op_manager.complete_all_tracked_operations_as_cancelled() + callback = mocker.MagicMock() + + def trigger_early_on_subscribe(topic, qos): + trigger_on_subscribe(mock_mqtt_client, mid=fake_mid) + assert callback.call_count == 0 + return (fake_rc, fake_mid) + + mock_mqtt_client.subscribe.side_effect = trigger_early_on_subscribe + + transport.subscribe(topic=fake_topic, qos=fake_qos, callback=callback) + + assert callback.call_args == mocker.call() + + @pytest.mark.it( + "Completes a rejected subscription when the SUBACK arrives before subscribe returns" + ) + def test_failed_suback_received_early(self, mocker, mock_mqtt_client, transport): + callback = mocker.MagicMock() + rejected = mqtt.ReasonCode(PacketTypes.SUBACK, identifier=128) + + def trigger_early_on_subscribe(topic, qos): + trigger_on_subscribe(mock_mqtt_client, mid=fake_mid, reason_codes=[rejected]) + assert callback.call_count == 0 + return (fake_rc, fake_mid) + + mock_mqtt_client.subscribe.side_effect = trigger_early_on_subscribe + + transport.subscribe(topic=fake_topic, qos=fake_qos, callback=callback) + + assert callback.call_count == 1 + assert isinstance(callback.call_args.kwargs["error"], errors.ProtocolClientError) + @pytest.mark.it("Skips callback that is set to 'None' upon subscribe completion") def test_none_callback_upon_paho_on_subscribe_event(self, mocker, mock_mqtt_client, transport): callback = None @@ -1353,9 +2103,7 @@ def test_none_callback_upon_paho_on_subscribe_event(self, mocker, mock_mqtt_clie transport.subscribe(topic=fake_topic, qos=fake_qos, callback=callback) # Manually trigger Paho on_subscribe event handler - mock_mqtt_client.on_subscribe( - client=mock_mqtt_client, userdata=None, mid=fake_mid, granted_qos=fake_qos - ) + trigger_on_subscribe(mock_mqtt_client, mid=fake_mid) # No assertions necessary - not raising an exception => success @@ -1370,9 +2118,7 @@ def test_none_callback_when_paho_on_subscribe_event_called_early( def trigger_early_on_subscribe(topic, qos): # Trigger on_subscribe before returning mid - mock_mqtt_client.on_subscribe( - client=mock_mqtt_client, userdata=None, mid=fake_mid, granted_qos=fake_qos - ) + trigger_on_subscribe(mock_mqtt_client, mid=fake_mid) return (fake_rc, fake_mid) @@ -1408,23 +2154,17 @@ def test_multiple_callbacks(self, mocker, mock_mqtt_client, transport): assert callback3.call_count == 0 # Manually trigger Paho on_subscribe event handler (2 -> 3 -> 1) - mock_mqtt_client.on_subscribe( - client=mock_mqtt_client, userdata=None, mid=mid2, granted_qos=fake_qos - ) + trigger_on_subscribe(mock_mqtt_client, mid=mid2) assert callback1.call_count == 0 assert callback2.call_count == 1 assert callback3.call_count == 0 - mock_mqtt_client.on_subscribe( - client=mock_mqtt_client, userdata=None, mid=mid3, granted_qos=fake_qos - ) + trigger_on_subscribe(mock_mqtt_client, mid=mid3) assert callback1.call_count == 0 assert callback2.call_count == 1 assert callback3.call_count == 1 - mock_mqtt_client.on_subscribe( - client=mock_mqtt_client, userdata=None, mid=mid1, granted_qos=fake_qos - ) + trigger_on_subscribe(mock_mqtt_client, mid=mid1) assert callback1.call_count == 1 assert callback2.call_count == 1 assert callback3.call_count == 1 @@ -1437,9 +2177,7 @@ def test_callback_raises_exception( mock_mqtt_client.subscribe.return_value = (fake_rc, fake_mid) transport.subscribe(topic=fake_topic, qos=fake_qos, callback=callback) - mock_mqtt_client.on_subscribe( - client=mock_mqtt_client, userdata=None, mid=fake_mid, granted_qos=fake_qos - ) + trigger_on_subscribe(mock_mqtt_client, mid=fake_mid) # Callback was called, but exception did not propagate assert callback.call_count == 1 @@ -1453,9 +2191,7 @@ def test_callback_raises_base_exception( transport.subscribe(topic=fake_topic, qos=fake_qos, callback=callback) with pytest.raises(arbitrary_base_exception.__class__) as e_info: - mock_mqtt_client.on_subscribe( - client=mock_mqtt_client, userdata=None, mid=fake_mid, granted_qos=fake_qos - ) + trigger_on_subscribe(mock_mqtt_client, mid=fake_mid) assert e_info.value is arbitrary_base_exception @pytest.mark.it("Recovers from Exception in callback when Paho event handler triggered early") @@ -1465,9 +2201,7 @@ def test_callback_raises_exception_when_paho_on_subscribe_triggered_early( callback = mocker.MagicMock(side_effect=arbitrary_exception) def trigger_early_on_subscribe(topic, qos): - mock_mqtt_client.on_subscribe( - client=mock_mqtt_client, userdata=None, mid=fake_mid, granted_qos=fake_qos - ) + trigger_on_subscribe(mock_mqtt_client, mid=fake_mid) # Should not have yet called callback assert callback.call_count == 0 @@ -1491,9 +2225,7 @@ def test_callback_raises_base_exception_when_paho_on_subscribe_triggered_early( callback = mocker.MagicMock(side_effect=arbitrary_base_exception) def trigger_early_on_subscribe(topic, qos): - mock_mqtt_client.on_subscribe( - client=mock_mqtt_client, userdata=None, mid=fake_mid, granted_qos=fake_qos - ) + trigger_on_subscribe(mock_mqtt_client, mid=fake_mid) # Should not have yet called callback assert callback.call_count == 0 @@ -1525,20 +2257,21 @@ def test_client_raises_base_exception( transport.subscribe(topic=fake_topic, qos=fake_qos, callback=None) assert e_info.value is arbitrary_base_exception - # NOTE: this test tests for all possible return codes, even ones that shouldn't be + # NOTE: this test tests all mapped Paho error codes, even ones that shouldn't be # possible on a subscribe operation. - @pytest.mark.it("Raises a custom Exception if Paho subscribe returns a failing rc code") + @pytest.mark.it("Raises a custom Exception if Paho subscribe returns an error code") @pytest.mark.parametrize( - "error_params", - operation_return_codes, - ids=["{}->{}".format(x["name"], x["error"].__name__) for x in operation_return_codes], + "error_case", + paho_error_code_cases, + ids=[ + "{}->{}".format(case["name"], case["error"].__name__) for case in paho_error_code_cases + ], ) - def test_client_returns_failing_rc_code( - self, mocker, mock_mqtt_client, transport, error_params - ): - mock_mqtt_client.subscribe.return_value = (error_params["rc"], 0) - with pytest.raises(error_params["error"]): + def test_client_returns_error_code(self, mocker, mock_mqtt_client, transport, error_case): + mock_mqtt_client.subscribe.return_value = (error_case["error_code"], 0) + with pytest.raises(error_case["error"]) as e_info: transport.subscribe(topic=fake_topic, qos=fake_qos, callback=None) + assert str(e_info.value) == mqtt.error_string(error_case["error_code"]) @pytest.mark.describe("MQTTTransport - .unsubscribe()") @@ -1550,6 +2283,16 @@ def test_calls_paho_unsubscribe(self, mocker, mock_mqtt_client, transport): assert mock_mqtt_client.unsubscribe.call_count == 1 assert mock_mqtt_client.unsubscribe.call_args == mocker.call(fake_topic) + @pytest.mark.it("Tracks the operation as an UNSUBSCRIBE") + def test_tracks_unsubscribe_operation_type(self, mocker, transport): + callback = mocker.MagicMock() + + transport.unsubscribe(fake_topic, callback=callback) + + pending_operation = transport._op_manager._pending_operations[fake_mid] + assert pending_operation.operation_type is OperationType.UNSUBSCRIBE + assert pending_operation.callback is callback + @pytest.mark.it("Raises ValueError on invalid topic string") @pytest.mark.parametrize("topic", [pytest.param(None), pytest.param("", id="Empty string")]) def test_raises_value_error_invalid_topic(self, topic): @@ -1574,7 +2317,7 @@ def test_triggers_callback_upon_paho_on_unsubscribe_event( assert callback.call_count == 0 # Manually trigger Paho on_unsubscribe event handler - mock_mqtt_client.on_unsubscribe(client=mock_mqtt_client, userdata=None, mid=fake_mid) + trigger_on_unsubscribe(mock_mqtt_client, mid=fake_mid) # Check callback has now been called assert callback.call_count == 1 @@ -1583,7 +2326,7 @@ def test_triggers_callback_upon_paho_on_unsubscribe_event( "Stops Paho's network loop if the MQTTTransport was garbage collected before unsubscribe completed" ) def test_stops_loop_after_gc(self, mocker, mock_mqtt_client, collected_transport_weakref): - mock_mqtt_client.on_unsubscribe(client=mock_mqtt_client, userdata=None, mid=fake_mid) + trigger_on_unsubscribe(mock_mqtt_client, mid=fake_mid) assert mock_mqtt_client.loop_stop.call_count == 1 assert mock_mqtt_client.loop_stop.call_args == mocker.call() @@ -1599,7 +2342,7 @@ def test_triggers_callback_when_paho_on_unsubscribe_event_called_early( def trigger_early_on_unsubscribe(topic): # Trigger on_unsubscribe before returning mid - mock_mqtt_client.on_unsubscribe(client=mock_mqtt_client, userdata=None, mid=fake_mid) + trigger_on_unsubscribe(mock_mqtt_client, mid=fake_mid) # Check callback not yet called assert callback.call_count == 0 @@ -1614,6 +2357,27 @@ def trigger_early_on_unsubscribe(topic): # Check callback has now been called assert callback.call_count == 1 + @pytest.mark.it( + "Triggers callback when a cancelled MID is reused and Paho completes before unsubscribe returns" + ) + def test_triggers_callback_when_cancelled_mid_reused_and_completed_early( + self, mocker, mock_mqtt_client, transport + ): + transport.unsubscribe(topic=fake_topic) + transport._op_manager.complete_all_tracked_operations_as_cancelled() + callback = mocker.MagicMock() + + def trigger_early_on_unsubscribe(topic): + trigger_on_unsubscribe(mock_mqtt_client, mid=fake_mid) + assert callback.call_count == 0 + return (fake_rc, fake_mid) + + mock_mqtt_client.unsubscribe.side_effect = trigger_early_on_unsubscribe + + transport.unsubscribe(topic=fake_topic, callback=callback) + + assert callback.call_args == mocker.call() + @pytest.mark.it("Skips callback that is set to 'None' upon unsubscribe completion") def test_none_callback_upon_paho_on_unsubscribe_event( self, mocker, mock_mqtt_client, transport @@ -1625,7 +2389,7 @@ def test_none_callback_upon_paho_on_unsubscribe_event( transport.unsubscribe(topic=fake_topic, callback=callback) # Manually trigger Paho on_unsubscribe event handler - mock_mqtt_client.on_unsubscribe(client=mock_mqtt_client, userdata=None, mid=fake_mid) + trigger_on_unsubscribe(mock_mqtt_client, mid=fake_mid) # No assertions necessary - not raising an exception => success @@ -1640,7 +2404,7 @@ def test_none_callback_when_paho_on_unsubscribe_event_called_early( def trigger_early_on_unsubscribe(topic): # Trigger on_unsubscribe before returning mid - mock_mqtt_client.on_unsubscribe(client=mock_mqtt_client, userdata=None, mid=fake_mid) + trigger_on_unsubscribe(mock_mqtt_client, mid=fake_mid) return (fake_rc, fake_mid) @@ -1680,17 +2444,17 @@ def test_multiple_callbacks(self, mocker, mock_mqtt_client, transport): assert callback3.call_count == 0 # Manually trigger Paho on_unsubscribe event handler (2 -> 3 -> 1) - mock_mqtt_client.on_unsubscribe(client=mock_mqtt_client, userdata=None, mid=mid2) + trigger_on_unsubscribe(mock_mqtt_client, mid=mid2) assert callback1.call_count == 0 assert callback2.call_count == 1 assert callback3.call_count == 0 - mock_mqtt_client.on_unsubscribe(client=mock_mqtt_client, userdata=None, mid=mid3) + trigger_on_unsubscribe(mock_mqtt_client, mid=mid3) assert callback1.call_count == 0 assert callback2.call_count == 1 assert callback3.call_count == 1 - mock_mqtt_client.on_unsubscribe(client=mock_mqtt_client, userdata=None, mid=mid1) + trigger_on_unsubscribe(mock_mqtt_client, mid=mid1) assert callback1.call_count == 1 assert callback2.call_count == 1 assert callback3.call_count == 1 @@ -1703,7 +2467,7 @@ def test_callback_raises_exception( mock_mqtt_client.unsubscribe.return_value = (fake_rc, fake_mid) transport.unsubscribe(topic=fake_topic, callback=callback) - mock_mqtt_client.on_unsubscribe(client=mock_mqtt_client, userdata=None, mid=fake_mid) + trigger_on_unsubscribe(mock_mqtt_client, mid=fake_mid) # Callback was called, but exception did not propagate assert callback.call_count == 1 @@ -1717,7 +2481,7 @@ def test_callback_raises_base_exception( transport.unsubscribe(topic=fake_topic, callback=callback) with pytest.raises(arbitrary_base_exception.__class__) as e_info: - mock_mqtt_client.on_unsubscribe(client=mock_mqtt_client, userdata=None, mid=fake_mid) + trigger_on_unsubscribe(mock_mqtt_client, mid=fake_mid) assert e_info.value is arbitrary_base_exception @pytest.mark.it("Recovers from Exception in callback when Paho event handler triggered early") @@ -1727,7 +2491,7 @@ def test_callback_raises_exception_when_paho_on_unsubscribe_triggered_early( callback = mocker.MagicMock(side_effect=arbitrary_exception) def trigger_early_on_unsubscribe(topic): - mock_mqtt_client.on_unsubscribe(client=mock_mqtt_client, userdata=None, mid=fake_mid) + trigger_on_unsubscribe(mock_mqtt_client, mid=fake_mid) # Should not have yet called callback assert callback.call_count == 0 @@ -1751,7 +2515,7 @@ def test_callback_raises_base_exception_when_paho_on_unsubscribe_triggered_early callback = mocker.MagicMock(side_effect=arbitrary_base_exception) def trigger_early_on_unsubscribe(topic): - mock_mqtt_client.on_unsubscribe(client=mock_mqtt_client, userdata=None, mid=fake_mid) + trigger_on_unsubscribe(mock_mqtt_client, mid=fake_mid) # Should not have yet called callback assert callback.call_count == 0 @@ -1785,20 +2549,21 @@ def test_client_raises_base_exception( transport.unsubscribe(topic=fake_topic, callback=None) assert e_info.value is arbitrary_base_exception - # NOTE: this test tests for all possible return codes, even ones that shouldn't be + # NOTE: this test tests all mapped Paho error codes, even ones that shouldn't be # possible on an unsubscribe operation. - @pytest.mark.it("Raises a custom Exception if Paho unsubscribe returns a failing rc code") + @pytest.mark.it("Raises a custom Exception if Paho unsubscribe returns an error code") @pytest.mark.parametrize( - "error_params", - operation_return_codes, - ids=["{}->{}".format(x["name"], x["error"].__name__) for x in operation_return_codes], + "error_case", + paho_error_code_cases, + ids=[ + "{}->{}".format(case["name"], case["error"].__name__) for case in paho_error_code_cases + ], ) - def test_client_returns_failing_rc_code( - self, mocker, mock_mqtt_client, transport, error_params - ): - mock_mqtt_client.unsubscribe.return_value = (error_params["rc"], 0) - with pytest.raises(error_params["error"]): + def test_client_returns_error_code(self, mocker, mock_mqtt_client, transport, error_case): + mock_mqtt_client.unsubscribe.return_value = (error_case["error_code"], 0) + with pytest.raises(error_case["error"]) as e_info: transport.unsubscribe(topic=fake_topic, callback=None) + assert str(e_info.value) == mqtt.error_string(error_case["error_code"]) @pytest.mark.describe("MQTTTransport - .publish()") @@ -1822,6 +2587,24 @@ def test_calls_paho_publish(self, mocker, mock_mqtt_client, transport, qos): topic=fake_topic, payload=fake_payload, qos=qos ) + @pytest.mark.it("Tracks the operation with its QoS-specific PUBLISH type") + @pytest.mark.parametrize( + "qos, expected_operation_type", + [ + pytest.param(0, OperationType.PUBLISH_QOS_0, id="QoS 0"), + pytest.param(1, OperationType.PUBLISH_QOS_1, id="QoS 1"), + pytest.param(2, OperationType.PUBLISH_QOS_2, id="QoS 2"), + ], + ) + def test_tracks_publish_operation_type(self, mocker, transport, qos, expected_operation_type): + callback = mocker.MagicMock() + + transport.publish(fake_topic, fake_payload, qos=qos, callback=callback) + + pending_operation = transport._op_manager._pending_operations[fake_mid] + assert pending_operation.operation_type is expected_operation_type + assert pending_operation.callback is callback + @pytest.mark.it("Raises ValueError on invalid QoS") @pytest.mark.parametrize("qos", [pytest.param(-1, id="QoS < 0"), pytest.param(3, id="Qos > 2")]) def test_raises_value_error_invalid_qos(self, qos): @@ -1890,7 +2673,7 @@ def test_triggers_callback_upon_paho_on_publish_event( assert callback.call_count == 0 # Manually trigger Paho on_publish event handler - mock_mqtt_client.on_publish(client=mock_mqtt_client, userdata=None, mid=message_info.mid) + trigger_on_publish(mock_mqtt_client, mid=message_info.mid) # Check callback has now been called assert callback.call_count == 1 @@ -1899,7 +2682,7 @@ def test_triggers_callback_upon_paho_on_publish_event( "Stops Paho's network loop if the MQTTTransport was garbage collected before publish completed" ) def test_stops_loop_after_gc(self, mocker, mock_mqtt_client, collected_transport_weakref): - mock_mqtt_client.on_publish(client=mock_mqtt_client, userdata=None, mid=fake_mid) + trigger_on_publish(mock_mqtt_client, mid=fake_mid) assert mock_mqtt_client.loop_stop.call_count == 1 assert mock_mqtt_client.loop_stop.call_args == mocker.call() @@ -1915,9 +2698,7 @@ def test_triggers_callback_when_paho_on_publish_event_called_early( def trigger_early_on_publish(topic, payload, qos): # Trigger on_publish before returning message_info - mock_mqtt_client.on_publish( - client=mock_mqtt_client, userdata=None, mid=message_info.mid - ) + trigger_on_publish(mock_mqtt_client, mid=message_info.mid) # Check callback not yet called assert callback.call_count == 0 @@ -1932,6 +2713,27 @@ def trigger_early_on_publish(topic, payload, qos): # Check callback has now been called assert callback.call_count == 1 + @pytest.mark.it( + "Triggers callback when a cancelled MID is reused and Paho completes before publish returns" + ) + def test_triggers_callback_when_cancelled_mid_reused_and_completed_early( + self, mocker, mock_mqtt_client, transport, message_info + ): + transport.publish(topic=fake_topic, payload=fake_payload) + transport._op_manager.complete_all_tracked_operations_as_cancelled() + callback = mocker.MagicMock() + + def trigger_early_on_publish(topic, payload, qos): + trigger_on_publish(mock_mqtt_client, mid=message_info.mid) + assert callback.call_count == 0 + return message_info + + mock_mqtt_client.publish.side_effect = trigger_early_on_publish + + transport.publish(topic=fake_topic, payload=fake_payload, callback=callback) + + assert callback.call_args == mocker.call() + @pytest.mark.it("Skips callback that is set to 'None' upon publish completion") def test_none_callback_upon_paho_on_publish_event( self, mocker, mock_mqtt_client, transport, message_info @@ -1943,7 +2745,7 @@ def test_none_callback_upon_paho_on_publish_event( transport.publish(topic=fake_topic, payload=fake_payload, callback=callback) # Manually trigger Paho on_publish event handler - mock_mqtt_client.on_publish(client=mock_mqtt_client, userdata=None, mid=message_info.mid) + trigger_on_publish(mock_mqtt_client, mid=message_info.mid) # No assertions necessary - not raising an exception => success @@ -1958,9 +2760,7 @@ def test_none_callback_when_paho_on_publish_event_called_early( def trigger_early_on_publish(topic, payload, qos): # Trigger on_publish before returning message_info - mock_mqtt_client.on_publish( - client=mock_mqtt_client, userdata=None, mid=message_info.mid - ) + trigger_on_publish(mock_mqtt_client, mid=message_info.mid) return message_info @@ -2000,17 +2800,17 @@ def test_multiple_callbacks(self, mocker, mock_mqtt_client, transport): assert callback3.call_count == 0 # Manually trigger Paho on_publish event handler (2 -> 3 -> 1) - mock_mqtt_client.on_publish(client=mock_mqtt_client, userdata=None, mid=mid2) + trigger_on_publish(mock_mqtt_client, mid=mid2) assert callback1.call_count == 0 assert callback2.call_count == 1 assert callback3.call_count == 0 - mock_mqtt_client.on_publish(client=mock_mqtt_client, userdata=None, mid=mid3) + trigger_on_publish(mock_mqtt_client, mid=mid3) assert callback1.call_count == 0 assert callback2.call_count == 1 assert callback3.call_count == 1 - mock_mqtt_client.on_publish(client=mock_mqtt_client, userdata=None, mid=mid1) + trigger_on_publish(mock_mqtt_client, mid=mid1) assert callback1.call_count == 1 assert callback2.call_count == 1 assert callback3.call_count == 1 @@ -2023,7 +2823,7 @@ def test_callback_raises_exception( mock_mqtt_client.publish.return_value = message_info transport.publish(topic=fake_topic, payload=fake_payload, callback=callback) - mock_mqtt_client.on_publish(client=mock_mqtt_client, userdata=None, mid=message_info.mid) + trigger_on_publish(mock_mqtt_client, mid=message_info.mid) # Callback was called, but exception did not propagate assert callback.call_count == 1 @@ -2037,9 +2837,7 @@ def test_callback_raises_base_exception( transport.publish(topic=fake_topic, payload=fake_payload, callback=callback) with pytest.raises(arbitrary_base_exception.__class__) as e_info: - mock_mqtt_client.on_publish( - client=mock_mqtt_client, userdata=None, mid=message_info.mid - ) + trigger_on_publish(mock_mqtt_client, mid=message_info.mid) assert e_info.value is arbitrary_base_exception @pytest.mark.it("Recovers from Exception in callback when Paho event handler triggered early") @@ -2049,9 +2847,7 @@ def test_callback_raises_exception_when_paho_on_publish_triggered_early( callback = mocker.MagicMock(side_effect=arbitrary_exception) def trigger_early_on_publish(topic, payload, qos): - mock_mqtt_client.on_publish( - client=mock_mqtt_client, userdata=None, mid=message_info.mid - ) + trigger_on_publish(mock_mqtt_client, mid=message_info.mid) # Should not have yet called callback assert callback.call_count == 0 @@ -2075,9 +2871,7 @@ def test_callback_raises_base_exception_when_paho_on_publish_triggered_early( callback = mocker.MagicMock(side_effect=arbitrary_base_exception) def trigger_early_on_publish(topic, payload, qos): - mock_mqtt_client.on_publish( - client=mock_mqtt_client, userdata=None, mid=message_info.mid - ) + trigger_on_publish(mock_mqtt_client, mid=message_info.mid) # Should not have yet called callback assert callback.call_count == 0 @@ -2109,20 +2903,52 @@ def test_client_raises_base_exception( transport.publish(topic=fake_topic, payload=fake_payload, callback=None) assert e_info.value is arbitrary_base_exception - # NOTE: this test tests for all possible return codes, even ones that shouldn't be + @pytest.mark.it("Completes a QoS publish retained after Paho reports no connection") + @pytest.mark.parametrize("qos", [pytest.param(1, id="QoS 1"), pytest.param(2, id="QoS 2")]) + def test_no_connection_qos_publish_completes_later( + self, mocker, mock_mqtt_client, transport, qos + ): + callback = mocker.MagicMock() + message_info = mqtt.MQTTMessageInfo(fake_mid) + message_info.rc = mqtt.MQTT_ERR_NO_CONN + mock_mqtt_client.publish.return_value = message_info + + transport.publish(fake_topic, fake_payload, qos=qos, callback=callback) + + assert callback.call_count == 0 + trigger_on_publish(mock_mqtt_client, mid=fake_mid) + assert callback.call_count == 1 + assert callback.call_args == mocker.call() + + @pytest.mark.it("Raises NoConnectionError for a disconnected QoS 0 publish") + def test_no_connection_qos_zero(self, mock_mqtt_client, transport): + message_info = mqtt.MQTTMessageInfo(fake_mid) + message_info.rc = mqtt.MQTT_ERR_NO_CONN + mock_mqtt_client.publish.return_value = message_info + + with pytest.raises(errors.NoConnectionError): + transport.publish(fake_topic, fake_payload, qos=0) + + # NOTE: this test tests all mapped Paho error codes, even ones that shouldn't be # possible on a publish operation. - @pytest.mark.it("Raises a custom Exception if Paho publish returns a failing rc code") + @pytest.mark.it("Raises a custom Exception if MQTTMessageInfo contains a failure code") @pytest.mark.parametrize( - "error_params", - operation_return_codes, - ids=["{}->{}".format(x["name"], x["error"].__name__) for x in operation_return_codes], + "error_case", + publish_failure_code_cases, + ids=[ + "{}->{}".format(case["name"], case["error"].__name__) + for case in publish_failure_code_cases + ], ) - def test_client_returns_failing_rc_code( - self, mocker, mock_mqtt_client, transport, error_params + def test_message_info_contains_failure_code( + self, mocker, mock_mqtt_client, transport, error_case ): - mock_mqtt_client.publish.return_value = (error_params["rc"], 0) - with pytest.raises(error_params["error"]): + message_info = mqtt.MQTTMessageInfo(0) + message_info.rc = error_case["error_code"] + mock_mqtt_client.publish.return_value = message_info + with pytest.raises(error_case["error"]) as e_info: transport.publish(topic=fake_topic, payload=fake_payload, callback=None) + assert str(e_info.value) == mqtt.error_string(error_case["error_code"]) @pytest.mark.describe("MQTTTransport - OCCURRENCE: Message Received") @@ -2159,6 +2985,26 @@ def test_stops_loop_after_gc( assert mock_mqtt_client.loop_stop.call_count == 1 assert mock_mqtt_client.loop_stop.call_args == mocker.call() + @pytest.mark.it( + "Stops Paho's network loop and allows any Exception from disconnect after GC to propagate" + ) + def test_stops_loop_after_gc_if_disconnect_raises( + self, + mock_mqtt_client, + collected_transport_weakref, + message, + arbitrary_exception, + ): + mock_mqtt_client.disconnect.side_effect = arbitrary_exception + + with pytest.raises(type(arbitrary_exception)) as e_info: + mock_mqtt_client.on_message( + client=mock_mqtt_client, userdata=None, mqtt_message=message + ) + + assert e_info.value is arbitrary_exception + assert mock_mqtt_client.loop_stop.call_count == 1 + @pytest.mark.it( "Skips on_mqtt_message_received_handler event handler if set to 'None' upon receiving message" ) @@ -2231,20 +3077,18 @@ def test_multiple_callbacks_multiple_ops(self, mocker, mock_mqtt_client, transpo assert callback2.call_count == 0 assert callback3.call_count == 0 - # Manually trigger Paho on_unsubscribe event handler (2 -> 3 -> 1) - mock_mqtt_client.on_publish(client=mock_mqtt_client, userdata=None, mid=mid2) + # Complete the operations out of order (2 -> 3 -> 1) + trigger_on_publish(mock_mqtt_client, mid=mid2) assert callback1.call_count == 0 assert callback2.call_count == 1 assert callback3.call_count == 0 - mock_mqtt_client.on_unsubscribe(client=mock_mqtt_client, userdata=None, mid=mid3) + trigger_on_unsubscribe(mock_mqtt_client, mid=mid3) assert callback1.call_count == 0 assert callback2.call_count == 1 assert callback3.call_count == 1 - mock_mqtt_client.on_subscribe( - client=mock_mqtt_client, userdata=None, mid=mid1, granted_qos=fake_qos - ) + trigger_on_subscribe(mock_mqtt_client, mid=mid1) assert callback1.call_count == 1 assert callback2.call_count == 1 assert callback3.call_count == 1 @@ -2255,12 +3099,51 @@ class TestOperationManager(object): @pytest.mark.it("Instantiates with no operation tracking information") def test_instantiates_empty(self): manager = OperationManager() - assert len(manager._pending_operation_callbacks) == 0 + assert len(manager._pending_operations) == 0 assert len(manager._unknown_operation_completions) == 0 + assert len(manager._cancelled_operation_mids) == 0 + + +@pytest.mark.describe("OperationManager - .operation_context()") +class TestOperationManagerOperationContext(object): + @pytest.mark.it("Claims an early completion when a cancelled MID is reused") + def test_claims_early_completion_for_reused_mid(self, mocker): + manager = OperationManager() + mid = 1 + callback = mocker.MagicMock() + register_publish(manager, mid) + manager.complete_all_tracked_operations_as_cancelled() + + with manager.operation_context(): + manager.complete_operation(mid) + assert manager._cancelled_operation_mids == {mid} + assert manager._unknown_operation_completions == {mid: None} + + register_publish(manager, mid, callback=callback) + + assert callback.call_args == mocker.call() + assert manager._cancelled_operation_mids == set() + assert manager._unknown_operation_completions == {} + assert manager._pending_operations == {} + + @pytest.mark.it("Discards an unclaimed late completion for a cancelled MID") + def test_discards_unclaimed_cancelled_completion(self): + manager = OperationManager() + mid = 1 + register_publish(manager, mid) + manager.complete_all_tracked_operations_as_cancelled() + + with manager.operation_context(): + manager.complete_operation(mid) + assert manager._cancelled_operation_mids == {mid} + assert manager._unknown_operation_completions == {mid: None} + + assert manager._cancelled_operation_mids == set() + assert manager._unknown_operation_completions == {} -@pytest.mark.describe("OperationManager - .establish_operation()") -class TestOperationManagerEstablishOperation(object): +@pytest.mark.describe("OperationManager - .register_operation()") +class TestOperationManagerRegisterOperation(object): @pytest.fixture(params=[True, False]) def optional_callback(self, mocker, request): if request.param: @@ -2274,46 +3157,76 @@ def optional_callback(self, mocker, request): [pytest.param(True, id="With callback"), pytest.param(False, id="No callback")], indirect=True, ) - def test_no_early_completion(self, optional_callback): + def test_no_unknown_completion(self, optional_callback): manager = OperationManager() mid = 1 - manager.establish_operation(mid, optional_callback) + register_publish(manager, mid, optional_callback) - assert len(manager._pending_operation_callbacks) == 1 - assert manager._pending_operation_callbacks[mid] is optional_callback + assert len(manager._pending_operations) == 1 + assert manager._pending_operations[mid].operation_type is OperationType.PUBLISH_QOS_1 + assert manager._pending_operations[mid].callback is optional_callback - @pytest.mark.it( - "Resolves operation tracking when MID corresponds to a previous unknown completion" - ) + @pytest.mark.it("Allows a cancelled MID without a late completion to be reused") + def test_cancelled_mid_reused_without_late_completion(self, mocker): + manager = OperationManager() + mid = 1 + reused_mid_callback = mocker.MagicMock() + + register_publish(manager, mid) + manager.complete_all_tracked_operations_as_cancelled() + register_publish(manager, mid, callback=reused_mid_callback) + + assert reused_mid_callback.call_count == 0 + + manager.complete_operation(mid) + assert reused_mid_callback.call_args == mocker.call() + + @pytest.mark.it("Resolves operation tracking when the response arrived before registration") def test_early_completion(self): manager = OperationManager() mid = 1 - # Cause early completion of an unknown operation + # Record a completion before the operation is registered manager.complete_operation(mid) assert len(manager._unknown_operation_completions) == 1 - assert manager._unknown_operation_completions[mid] + assert manager._unknown_operation_completions[mid] is None - # Establish operation that was already completed - manager.establish_operation(mid) + # Register operation that was already completed + register_publish(manager, mid) assert len(manager._unknown_operation_completions) == 0 @pytest.mark.it( - "Triggers the callback if provided when MID corresponds to a previous unknown completion" + "Invokes the callback if provided when the response arrived before registration" ) def test_early_completion_with_callback(self, mocker): manager = OperationManager() mid = 1 cb_mock = mocker.MagicMock() - # Cause early completion of an unknown operation + # Record a completion before the operation is registered manager.complete_operation(mid) - # Establish operation that was already completed - manager.establish_operation(mid, cb_mock) + # Register operation that was already completed + register_publish(manager, mid, cb_mock) assert cb_mock.call_count == 1 + assert cb_mock.call_args == mocker.call() + + @pytest.mark.it("Preserves an error when the completion arrives before registration") + def test_early_completion_with_error(self, mocker): + manager = OperationManager() + mid = 1 + callback = mocker.MagicMock() + error = errors.ProtocolClientError("subscription rejected") + + manager.complete_operation(mid, error=error) + manager.register_operation( + mid=mid, callback=callback, operation_type=OperationType.SUBSCRIBE + ) + + assert callback.call_count == 1 + assert callback.call_args == mocker.call(error=error) @pytest.mark.it("Recovers from Exception thrown in callback") def test_callback_raises_exception(self, mocker, arbitrary_exception): @@ -2321,11 +3234,11 @@ def test_callback_raises_exception(self, mocker, arbitrary_exception): mid = 1 cb_mock = mocker.MagicMock(side_effect=arbitrary_exception) - # Cause early completion of an unknown operation + # Record a completion before the operation is registered manager.complete_operation(mid) - # Establish operation that was already completed - manager.establish_operation(mid, cb_mock) + # Register operation that was already completed + register_publish(manager, mid, cb_mock) # Callback was called, but exception did not propagate assert cb_mock.call_count == 1 @@ -2336,21 +3249,21 @@ def test_callback_raises_base_exception(self, mocker, arbitrary_base_exception): mid = 1 cb_mock = mocker.MagicMock(side_effect=arbitrary_base_exception) - # Cause early completion of an unknown operation + # Record a completion before the operation is registered manager.complete_operation(mid) - # Establish operation that was already completed + # Register operation that was already completed with pytest.raises(arbitrary_base_exception.__class__) as e_info: - manager.establish_operation(mid, cb_mock) + register_publish(manager, mid, cb_mock) assert e_info.value is arbitrary_base_exception - @pytest.mark.it("Does not trigger the callback until after thread lock has been released") + @pytest.mark.it("Does not invoke the callback until after thread lock has been released") def test_callback_called_after_lock_release(self, mocker): manager = OperationManager() mid = 1 cb_mock = mocker.MagicMock() - # Cause early completion of an unknown operation + # Record a completion before the operation is registered manager.complete_operation(mid) # Set up mock tracking @@ -2372,8 +3285,8 @@ def stop_tracking_mocks(*args): lock_spy.__enter__.side_effect = track_mocks lock_spy.__exit__.side_effect = stop_tracking_mocks - # Establish operation that was already completed - manager.establish_operation(mid, cb_mock) + # Register operation that was already completed + register_publish(manager, mid, cb_mock) # Callback WAS called, but... assert cb_mock.call_count == 1 @@ -2384,39 +3297,54 @@ def stop_tracking_mocks(*args): @pytest.mark.describe("OperationManager - .complete_operation()") class TestOperationManagerCompleteOperation(object): - @pytest.mark.it("Resolves a operation tracking when MID corresponds to a pending operation") + @pytest.mark.it("Resolves operation tracking when MID corresponds to a pending operation") def test_complete_pending_operation(self): manager = OperationManager() mid = 1 - # Establish a pending operation - manager.establish_operation(mid) - assert len(manager._pending_operation_callbacks) == 1 + # Register a pending operation + register_publish(manager, mid) + assert len(manager._pending_operations) == 1 # Complete pending operation manager.complete_operation(mid) - assert len(manager._pending_operation_callbacks) == 0 + assert len(manager._pending_operations) == 0 - @pytest.mark.it("Triggers callback for a pending operation when resolving") + @pytest.mark.it("Invokes callback for a pending operation when resolving") def test_complete_pending_operation_callback(self, mocker): manager = OperationManager() mid = 1 cb_mock = mocker.MagicMock() - manager.establish_operation(mid, cb_mock) + register_publish(manager, mid, cb_mock) assert cb_mock.call_count == 0 manager.complete_operation(mid) assert cb_mock.call_count == 1 assert cb_mock.call_args == mocker.call() + @pytest.mark.it("Invokes callback with an error for a failed pending operation") + def test_complete_pending_operation_callback_with_error(self, mocker): + manager = OperationManager() + mid = 1 + callback = mocker.MagicMock() + error = errors.ProtocolClientError("subscription rejected") + + manager.register_operation( + mid=mid, callback=callback, operation_type=OperationType.SUBSCRIBE + ) + manager.complete_operation(mid, error=error) + + assert callback.call_count == 1 + assert callback.call_args == mocker.call(error=error) + @pytest.mark.it("Recovers from Exception thrown in callback") def test_callback_raises_exception(self, mocker, arbitrary_exception): manager = OperationManager() mid = 1 cb_mock = mocker.MagicMock(side_effect=arbitrary_exception) - manager.establish_operation(mid, cb_mock) + register_publish(manager, mid, cb_mock) assert cb_mock.call_count == 0 manager.complete_operation(mid) @@ -2429,32 +3357,48 @@ def test_callback_raises_base_exception(self, mocker, arbitrary_base_exception): mid = 1 cb_mock = mocker.MagicMock(side_effect=arbitrary_base_exception) - manager.establish_operation(mid, cb_mock) + register_publish(manager, mid, cb_mock) assert cb_mock.call_count == 0 with pytest.raises(arbitrary_base_exception.__class__) as e_info: manager.complete_operation(mid) assert e_info.value is arbitrary_base_exception - @pytest.mark.it( - "Begins tracking an unknown completion if MID does not correspond to a pending operation" - ) - def test_early_completion(self): + @pytest.mark.it("Retains a completion if MID does not correspond to a pending operation") + def test_unknown_completion(self): manager = OperationManager() mid = 1 manager.complete_operation(mid) assert len(manager._unknown_operation_completions) == 1 - assert manager._unknown_operation_completions[mid] + assert manager._unknown_operation_completions[mid] is None + + @pytest.mark.it("Discards a late completion for a cancelled MID") + def test_late_completion_for_cancelled_mid(self, mocker): + manager = OperationManager() + mid = 1 + cancelled_callback = mocker.MagicMock() + reused_mid_callback = mocker.MagicMock() - @pytest.mark.it("Does not trigger the callback until after thread lock has been released") + register_publish(manager, mid, callback=cancelled_callback) + manager.complete_all_tracked_operations_as_cancelled() + manager.complete_operation(mid) + register_publish(manager, mid, callback=reused_mid_callback) + + assert cancelled_callback.call_args == mocker.call(cancelled=True) + assert reused_mid_callback.call_count == 0 + + manager.complete_operation(mid) + assert reused_mid_callback.call_args == mocker.call() + + @pytest.mark.it("Does not invoke the callback until after thread lock has been released") def test_callback_called_after_lock_release(self, mocker): manager = OperationManager() mid = 1 cb_mock = mocker.MagicMock() # Set up an operation and save the callback - manager.establish_operation(mid, cb_mock) + register_publish(manager, mid, cb_mock) # Set up mock tracking lock_spy = mocker.spy(manager, "_lock") @@ -2486,21 +3430,105 @@ def stop_tracking_mocks(*args): assert mocker.call.cb() not in calls_during_lock -@pytest.mark.describe("OperationManager - .cancel_all_operations()") -class TestOperationManagerCancelAllOperations(object): - @pytest.mark.it("Removes all MID tracking for all pending operations") - def test_remove_pending_ops(self): +@pytest.mark.describe("OperationManager - .complete_non_resumable_operations_as_cancelled()") +class TestOperationManagerCompleteNonResumableOperationsAsCancelled(object): + @pytest.mark.it("Preserves resumable publishes and cancels non-resumable operations") + def test_completes_non_resumable_operations(self, mocker): + manager = OperationManager() + qos_one_publish_callback = mocker.MagicMock() + qos_two_publish_callback = mocker.MagicMock() + qos_zero_publish_callback = mocker.MagicMock() + subscribe_callback = mocker.MagicMock() + unsubscribe_callback = mocker.MagicMock() + register_publish(manager, mid=1, callback=qos_one_publish_callback, qos=1) + register_publish(manager, mid=2, callback=qos_two_publish_callback, qos=2) + register_publish(manager, mid=3, callback=qos_zero_publish_callback, qos=0) + manager.register_operation( + mid=4, callback=subscribe_callback, operation_type=OperationType.SUBSCRIBE + ) + manager.register_operation( + mid=5, callback=unsubscribe_callback, operation_type=OperationType.UNSUBSCRIBE + ) + + manager.complete_non_resumable_operations_as_cancelled() + + assert list(manager._pending_operations) == [1, 2] + assert manager._pending_operations[1].operation_type is OperationType.PUBLISH_QOS_1 + assert manager._pending_operations[1].callback is qos_one_publish_callback + assert manager._pending_operations[2].operation_type is OperationType.PUBLISH_QOS_2 + assert manager._pending_operations[2].callback is qos_two_publish_callback + assert manager._cancelled_operation_mids == {3, 4, 5} + assert qos_one_publish_callback.call_count == 0 + assert qos_two_publish_callback.call_count == 0 + assert qos_zero_publish_callback.call_args == mocker.call(cancelled=True) + assert subscribe_callback.call_args == mocker.call(cancelled=True) + assert unsubscribe_callback.call_args == mocker.call(cancelled=True) + + @pytest.mark.it("Discards a late completion for a cancelled non-resumable operation") + def test_discards_late_completion(self, mocker): + manager = OperationManager() + callback = mocker.MagicMock() + manager.register_operation(mid=1, callback=callback, operation_type=OperationType.SUBSCRIBE) + manager.complete_non_resumable_operations_as_cancelled() + + assert callback.call_args == mocker.call(cancelled=True) + + manager.complete_operation(mid=1) + + assert callback.call_count == 1 + assert manager._cancelled_operation_mids == set() + assert manager._unknown_operation_completions == {} + + @pytest.mark.it("Discards a late completion for a cancelled QoS 0 publish") + def test_discards_late_qos_zero_completion(self, mocker): + manager = OperationManager() + callback = mocker.MagicMock() + register_publish(manager, mid=1, callback=callback, qos=0) + manager.complete_non_resumable_operations_as_cancelled() + + assert callback.call_args == mocker.call(cancelled=True) + + manager.complete_operation(mid=1) + + assert callback.call_count == 1 + assert manager._cancelled_operation_mids == set() + assert manager._unknown_operation_completions == {} + + +@pytest.mark.describe("OperationManager - .connection_context()") +class TestOperationManagerConnectionContext(object): + @pytest.mark.it("Defers cancellation callbacks without deferring tracking cleanup") + def test_defers_callbacks_only(self, mocker): + manager = OperationManager() + callback = mocker.MagicMock() + register_publish(manager, mid=1, callback=callback) + + with manager.connection_context(): + manager.complete_all_tracked_operations_as_cancelled() + + assert manager._pending_operations == {} + assert manager._cancelled_operation_mids == {1} + assert callback.call_count == 0 + + assert callback.call_args == mocker.call(cancelled=True) + + +@pytest.mark.describe("OperationManager - .complete_all_tracked_operations_as_cancelled()") +class TestOperationManagerCompleteAllTrackedOperationsAsCancelled(object): + @pytest.mark.it("Removes pending callbacks and retains their MIDs as cancelled") + def test_cancel_pending_ops(self): manager = OperationManager() - # Establish pending operations - manager.establish_operation(mid=1) - manager.establish_operation(mid=2) - manager.establish_operation(mid=3) - assert len(manager._pending_operation_callbacks) == 3 + # Register pending operations + register_publish(manager, mid=1) + manager.register_operation(mid=2, callback=None, operation_type=OperationType.SUBSCRIBE) + manager.register_operation(mid=3, callback=None, operation_type=OperationType.UNSUBSCRIBE) + assert len(manager._pending_operations) == 3 - # Cancel operations - manager.cancel_all_operations() - assert len(manager._pending_operation_callbacks) == 0 + # Complete tracked operations as cancelled + manager.complete_all_tracked_operations_as_cancelled() + assert len(manager._pending_operations) == 0 + assert manager._cancelled_operation_mids == {1, 2, 3} @pytest.mark.it("Removes all MID tracking for unknown operation completions") def test_remove_unknown_completions(self): @@ -2512,25 +3540,25 @@ def test_remove_unknown_completions(self): manager.complete_operation(mid=2345) assert len(manager._unknown_operation_completions) == 3 - # Cancel operations - manager.cancel_all_operations() + # Complete tracked operations as cancelled + manager.complete_all_tracked_operations_as_cancelled() assert len(manager._unknown_operation_completions) == 0 - @pytest.mark.it("Triggers callbacks (if present) with cancel flag for each pending operation") + @pytest.mark.it("Invokes callbacks with cancelled=True for each tracked operation") def test_op_callback_completion(self, mocker): manager = OperationManager() - # Establish pending operations + # Register pending operations cb_mock1 = mocker.MagicMock() - manager.establish_operation(mid=1, callback=cb_mock1) + register_publish(manager, mid=1, callback=cb_mock1) cb_mock2 = mocker.MagicMock() - manager.establish_operation(mid=2, callback=cb_mock2) - manager.establish_operation(mid=3, callback=None) + manager.register_operation(mid=2, callback=cb_mock2, operation_type=OperationType.SUBSCRIBE) + manager.register_operation(mid=3, callback=None, operation_type=OperationType.UNSUBSCRIBE) assert cb_mock1.call_count == 0 assert cb_mock2.call_count == 0 - # Cancel operations - manager.cancel_all_operations() + # Complete tracked operations as cancelled + manager.complete_all_tracked_operations_as_cancelled() assert cb_mock1.call_count == 1 assert cb_mock1.call_args == mocker.call(cancelled=True) assert cb_mock2.call_count == 1 @@ -2540,13 +3568,13 @@ def test_op_callback_completion(self, mocker): def test_callback_raises_exception(self, mocker, arbitrary_exception): manager = OperationManager() - # Establish pending operation + # Register pending operation cb_mock = mocker.MagicMock(side_effect=arbitrary_exception) - manager.establish_operation(mid=1, callback=cb_mock) + register_publish(manager, mid=1, callback=cb_mock) assert cb_mock.call_count == 0 - # Cancel operations - manager.cancel_all_operations() + # Complete tracked operations as cancelled + manager.complete_all_tracked_operations_as_cancelled() # Callback was called but exception did not propagate assert cb_mock.call_count == 1 @@ -2555,25 +3583,25 @@ def test_callback_raises_exception(self, mocker, arbitrary_exception): def test_callback_raises_base_exception(self, mocker, arbitrary_base_exception): manager = OperationManager() - # Establish pending operation + # Register pending operation cb_mock = mocker.MagicMock(side_effect=arbitrary_base_exception) - manager.establish_operation(mid=1, callback=cb_mock) + register_publish(manager, mid=1, callback=cb_mock) assert cb_mock.call_count == 0 - # When cancelling operations, Base Exception propagates + # When completing operations, Base Exception propagates with pytest.raises(arbitrary_base_exception.__class__) as e_info: - manager.cancel_all_operations() + manager.complete_all_tracked_operations_as_cancelled() assert e_info.value is arbitrary_base_exception - @pytest.mark.it("Does not trigger callbacks until after thread lock has been released") + @pytest.mark.it("Does not invoke callbacks until after thread lock has been released") def test_callback_called_after_lock_release(self, mocker): manager = OperationManager() cb_mock1 = mocker.MagicMock() cb_mock2 = mocker.MagicMock() # Set up operations and save the callback - manager.establish_operation(mid=1, callback=cb_mock1) - manager.establish_operation(mid=2, callback=cb_mock2) + register_publish(manager, mid=1, callback=cb_mock1) + manager.register_operation(mid=2, callback=cb_mock2, operation_type=OperationType.SUBSCRIBE) # Set up mock tracking lock_spy = mocker.spy(manager, "_lock") @@ -2595,8 +3623,8 @@ def stop_tracking_mocks(*args): lock_spy.__enter__.side_effect = track_mocks lock_spy.__exit__.side_effect = stop_tracking_mocks - # Cancel operations - manager.cancel_all_operations() + # Complete tracked operations as cancelled + manager.complete_all_tracked_operations_as_cancelled() # Callbacks WERE called, but... assert cb_mock1.call_count == 1 diff --git a/uv.lock b/uv.lock index c1523b134..f4f462ea9 100644 --- a/uv.lock +++ b/uv.lock @@ -91,7 +91,7 @@ test = [ requires-dist = [ { name = "deprecation", specifier = ">=2.1.0,<3.0.0" }, { name = "janus", specifier = ">=2.0.0,<3.0.0" }, - { name = "paho-mqtt", specifier = ">=2.0.0,<3.0.0" }, + { name = "paho-mqtt", specifier = ">=2.1.0,<3.0.0" }, { name = "pysocks" }, { name = "requests", specifier = ">=2.32.3,<3.0.0" }, { name = "requests-unixsocket", specifier = ">=0.4.1" },