Repository: adafruit/Adafruit_CircuitPython_MiniMQTT (language: python)

Issue:
Errors with MQTT CircuitPython 9 Publish
We have an implementation of MiniMQTT that has been quite stable in CircuitPython 8 but started to display multiple issues when we tested in CircuitPython 9 (9.0.3 and below) on multiple ESP32-S3 boards.

The code base sends some API requests over HTTP GET/POST using the `Adafruit_CircuitPython_Requests` library (with no issues) and then continues on to connect to the MQTT server over SSL to send sensor observations every few seconds. We tried updating to use `adafruit_connection_manager` for `socket_pool` and `ssl_context` but this does not seem to fix the issue. We also tried to run a `reconnect()` on errors as described in #194 but no luck. It is interesting because sometimes the code runs fine for an hour or more but then most often it causes errors within the first few minutes if not immediately.

We tried various bundles both with `*.py` source code, `*.mpy` bundle pre-built and self-built `*.mpy` variations with no luck and similar observations. The bundle dates we have tried out are : `20240402`, `20240307` and `20240224`.

The most common errors seen (but not necessarily together) are :
```
OSError(11,)
OSError(104,)
MemoryError()
MMQTTException('Repeated connect failures',)
```

Less Common Errors (maybe stemming from the common errors above ? ) :
```
OSError(128,)
<WatchDogTimeout>
gaierror(-2, 'Name or service not known')
```

Sample Code :

```python
import wifi
import adafruit_minimqtt.adafruit_minimqtt as MQTT

radio = wifi.radio
pool = adafruit_connection_manager.get_radio_socketpool(radio)
ssl_context = adafruit_connection_manager.get_radio_ssl_context(radio)

mqtt = MQTT.MQTT(
    broker=...,
    port=...,
    is_ssl=True,
    socket_pool=pool,
    ssl_context=ssl_context,
    username=...,
    password=...,
)

try:
	mqtt.publish(topic, message)
except:
	mqtt.reconnect()
```

Are there any additional debugging steps that we can try out or are there other things we should consider when upgrading to CP9? Thank you!


Discussion / hints:
There is a core CircuitPython issue open that may match what you're seeing: https://github.com/adafruit/circuitpython/issues/9123. Feel free to contribute to that issue.

Are you using `CIRCUITPY_WIFI_SSID` and `CIRCUITPY_WIFI_PASSWORD`, which will enable the web workflow and cause a connection in advance?

Clearly there is some new problem, and we have to track it down. There's no reason you can't stay on 8.x.x for now.
Thanks for the quick response and pointing us to this issue Dan. 

For our implementation. We are not using the web workflow but `wifi.radio.connect( ssid=...,    password=... )` instead. Will add this to the list to try.
I have a weather related script running on an Adafruit ESP32-S3 Feather with 9.0 that publishes to AdafruitIO.  It's been just as stable as 8.x but I'm probably catching and ignoring or resetting the board for every possible error now.  It's more work but it is possible to catch most of those errors with a lot of try/except's and as a last resort do a supervisor.reload.

It might be helpful to see how to [catch the errors including the -2 gaierror](https://github.com/DJDevon3/My_Circuit_Python_Projects/blob/3e31fa76699a81dc7a9887def97a0c82efef2ffe/Boards/espressif/Adafruit%20Feather%20ESP32-S3/3.5%20TFT%20Featherwing/Feather%20Weather%20MQTT%20Touch/code.py#L1148). 
[Feather Weather MQTT Touch](https://github.com/DJDevon3/My_Circuit_Python_Projects/blob/main/Boards/espressif/Adafruit%20Feather%20ESP32-S3/3.5%20TFT%20Featherwing/Feather%20Weather%20MQTT%20Touch/code.py)

In my experience I would get gaierrors in both 8.x and 9.0.  Only real way to make it stable is to catch all the errors.  I've been working on my script for about 2+ years though and use it every day sitting in front of my pc.  I've had plenty of time to refine it in the long term.

A lot of the errors come down to wifi disconnections mid-script or DNS lookup failures, or socket errors, the list goes on the point is there are a lot of errors and you have to catch them.  A lot of the time.sleep's in my error handling are the approximate amount of time it takes for my wifi to reconnect if it goes down or other reasons.  Catching errors and your wifi might have different behavior so you have to customize the way you handle errors for your script.  There is no silver bullet solution.

I'm running 9.0 from March 19th, 2024, so about a month now total.  My ability to publish with miniMQTT to AdafruitIO has been 99.9% solid.  I can show my adafruitio dashboard as proof but just take my word for it.  It is possible to get things stabilized with error handling.
thank you @DJDevon3 for sharing your project and experience. I will compare our code and see where we can make some improvements.
I'm also facing issues when I try to connect an ESP32 running CircuitPython 9.0.4 to a MQTT broker. This is my code:

```python
import os
import time
import ssl
import json
import alarm
import board
import socketpool
import wifi
import adafruit_minimqtt.adafruit_minimqtt as MQTT
import adafruit_bme680
import adafruit_logging as logging

PUBLISH_DELAY = 60
MQTT_TOPIC = "grenhouse"
USE_DEEP_SLEEP = True

# Connect to the Sensor
i2c = board.I2C()
sensor = adafruit_bme680.Adafruit_BME680_I2C(i2c)

wifi.radio.connect(
    os.getenv("CIRCUITPY_WIFI_SSID"), os.getenv("CIRCUITPY_WIFI_PASSWORD")
)
print(f"Connected to {os.getenv('CIRCUITPY_WIFI_SSID')}")
print(f"My IP address: {wifi.radio.ipv4_address}")

# Create a socket pool
pool = socketpool.SocketPool(wifi.radio)

# Set up a MiniMQTT Client
mqtt_client = MQTT.MQTT(
    broker=os.getenv("MQTT_BROKER"),
    port=os.getenv("MQTT_PORT"),
    username=os.getenv("MQTT_USERNAME"),
    password=os.getenv("MQTT_PASSWORD"),
    socket_pool=pool,
    ssl_context=ssl.create_default_context(),
)


mqtt_client.logger = logging.getLogger()

while not mqtt_client.is_connected():
    print(f"Attempting to connect to {mqtt_client.broker}")
    try:
        mqtt_client.connect()
    except Exception as exp:
        print(exp)
        time.sleep(5)

while True:
    temperature = sensor.temperature
    humidity = sensor.humidity
    pressure = sensor.pressure
    gas = sensor.gas

    output = {
        "temperature": temperature,
        "humidity": humidity,
        "pressure": pressure,
        "gas": gas,
    }

    print("Publishing to %s" % MQTT_TOPIC)
    mqtt_client.publish(MQTT_TOPIC, json.dumps(output))

    if USE_DEEP_SLEEP:
        mqtt_client.disconnect()
        pause = alarm.time.TimeAlarm(monotonic_time=time.monotonic() + PUBLISH_DELAY)
        alarm.exit_and_deep_sleep_until_alarms(pause)
    else:
        last_update = time.monotonic()
        while time.monotonic() < last_update + PUBLISH_DELAY:
            mqtt_client.loop()
```

This is the log:

```
Connected to WIFI
My IP address: 192.168.1.79
Attempting to connect to 192.168.1.91
69.789: WARNING - Socket error when connecting: Socket already connected to mqtt://192.168.1.91:1883
69.800: WARNING - Socket error when connecting: Socket already connected to mqtt://192.168.1.91:1883
69.812: WARNING - Socket error when connecting: Socket already connected to mqtt://192.168.1.91:1883
69.825: WARNING - Socket error when connecting: Socket already connected to mqtt://192.168.1.91:1883
Repeated connect failures
```

The logs do not say which line of the `_connect()` function raised the exception with message "Socket already connected".

How could I debug this further?
Since you are setting the logger, I would set it to debug so you can see what it's logging.
> Since you are setting the logger, I would set it to debug so you can see what it's logging.

That got me on track. Thanks

My connection was unauthorised. I used the same credentials that I used for another MQTT client. I had to add a new user to HomeAssistant in order to log values with my ESP32 device. Some details about this in [this other HA discussion](https://community.home-assistant.io/t/mqtt-refusing-connection/437138/4)

Without using a logger, I would have expected that an unauthorised access would be reported as an error by MiniMQTT and not just as "Repeated connect failures"
Well, glad to get that solved. I can look at auth and see if there's something that can be fixed there.

Fix patch:
diff --git a/adafruit_minimqtt/adafruit_minimqtt.py b/adafruit_minimqtt/adafruit_minimqtt.py
index 622c6e6..22b1f80 100644
--- a/adafruit_minimqtt/adafruit_minimqtt.py
+++ b/adafruit_minimqtt/adafruit_minimqtt.py
@@ -72,12 +72,18 @@
 MQTT_PKT_TYPE_MASK = const(0xF0)
 
 
+CONNACK_ERROR_INCORRECT_PROTOCOL = const(0x01)
+CONNACK_ERROR_ID_REJECTED = const(0x02)
+CONNACK_ERROR_SERVER_UNAVAILABLE = const(0x03)
+CONNACK_ERROR_INCORECT_USERNAME_PASSWORD = const(0x04)
+CONNACK_ERROR_UNAUTHORIZED = const(0x05)
+
 CONNACK_ERRORS = {
-    const(0x01): "Connection Refused - Incorrect Protocol Version",
-    const(0x02): "Connection Refused - ID Rejected",
-    const(0x03): "Connection Refused - Server unavailable",
-    const(0x04): "Connection Refused - Incorrect username/password",
-    const(0x05): "Connection Refused - Unauthorized",
+    CONNACK_ERROR_INCORRECT_PROTOCOL: "Connection Refused - Incorrect Protocol Version",
+    CONNACK_ERROR_ID_REJECTED: "Connection Refused - ID Rejected",
+    CONNACK_ERROR_SERVER_UNAVAILABLE: "Connection Refused - Server unavailable",
+    CONNACK_ERROR_INCORECT_USERNAME_PASSWORD: "Connection Refused - Incorrect username/password",
+    CONNACK_ERROR_UNAUTHORIZED: "Connection Refused - Unauthorized",
 }
 
 _default_sock = None  # pylint: disable=invalid-name
@@ -87,6 +93,10 @@
 class MMQTTException(Exception):
     """MiniMQTT Exception class."""
 
+    def __init__(self, error, code=None):
+        super().__init__(error, code)
+        self.code = code
+
 
 class NullLogger:
     """Fake logger class that does not do anything"""
@@ -428,17 +438,24 @@ def connect(
                 self.logger.warning(f"Socket error when connecting: {e}")
                 backoff = False
             except MMQTTException as e:
-                last_exception = e
+                self._close_socket()
                 self.logger.info(f"MMQT error: {e}")
+                if e.code in [
+                    CONNACK_ERROR_INCORECT_USERNAME_PASSWORD,
+                    CONNACK_ERROR_UNAUTHORIZED,
+                ]:
+                    # No sense trying these again, re-raise
+                    raise
+                last_exception = e
                 backoff = True
 
         if self._reconnect_attempts_max > 1:
             exc_msg = "Repeated connect failures"
         else:
             exc_msg = "Connect failure"
+
         if last_exception:
             raise MMQTTException(exc_msg) from last_exception
-
         raise MMQTTException(exc_msg)
 
     # pylint: disable=too-many-branches, too-many-statements, too-many-locals
@@ -535,7 +552,7 @@ def _connect(
                 rc = self._sock_exact_recv(3)
                 assert rc[0] == 0x02
                 if rc[2] != 0x00:
-                    raise MMQTTException(CONNACK_ERRORS[rc[2]])
+                    raise MMQTTException(CONNACK_ERRORS[rc[2]], code=rc[2])
                 self._is_connected = True
                 result = rc[0] & 1
                 if self.on_connect is not None:
@@ -549,6 +566,12 @@ def _connect(
                         f"No data received from broker for {self._recv_timeout} seconds."
                     )
 
+    def _close_socket(self):
+        if self._sock:
+            self.logger.debug("Closing socket")
+            self._connection_manager.close_socket(self._sock)
+            self._sock = None
+
     # pylint: disable=no-self-use
     def _encode_remaining_length(
         self, fixed_header: bytearray, remaining_length: int
@@ -577,8 +600,7 @@ def disconnect(self) -> None:
             self._sock.send(MQTT_DISCONNECT)
         except RuntimeError as e:
             self.logger.warning(f"Unable to send DISCONNECT packet: {e}")
-        self.logger.debug("Closing socket")
-        self._connection_manager.close_socket(self._sock)
+        self._close_socket()
         self._is_connected = False
         self._subscribed_topics = []
         self._last_msg_sent_timestamp = 0


Test patch:
diff --git a/tests/test_backoff.py b/tests/test_backoff.py
index e26d07a..44d2da9 100644
--- a/tests/test_backoff.py
+++ b/tests/test_backoff.py
@@ -18,18 +18,24 @@ class TestExpBackOff:
     """basic exponential back-off test"""
 
     connect_times = []
+    raise_exception = None
 
     # pylint: disable=unused-argument
     def fake_connect(self, arg):
         """connect() replacement that records the call times and always raises OSError"""
         self.connect_times.append(time.monotonic())
-        raise OSError("this connect failed")
+        raise self.raise_exception
 
     def test_failing_connect(self) -> None:
         """test that exponential back-off is used when connect() always raises OSError"""
         # use RFC 1918 address to avoid dealing with IPv6 in the call list below
         host = "172.40.0.3"
         port = 1883
+        self.connect_times = []
+        error_code = MQTT.CONNACK_ERROR_SERVER_UNAVAILABLE
+        self.raise_exception = MQTT.MMQTTException(
+            MQTT.CONNACK_ERRORS[error_code], code=error_code
+        )
 
         with patch.object(socket.socket, "connect") as mock_method:
             mock_method.side_effect = self.fake_connect
@@ -45,6 +51,7 @@ def test_failing_connect(self) -> None:
             print("connecting")
             with pytest.raises(MQTT.MMQTTException) as context:
                 mqtt_client.connect()
+                assert mqtt_client._sock is None
             assert "Repeated connect failures" in str(context)
 
             mock_method.assert_called()
@@ -54,3 +61,34 @@ def test_failing_connect(self) -> None:
             print(f"connect() call times: {self.connect_times}")
             for i in range(1, connect_retries):
                 assert self.connect_times[i] >= 2**i
+
+    def test_unauthorized(self) -> None:
+        """test that exponential back-off is used when connect() always raises OSError"""
+        # use RFC 1918 address to avoid dealing with IPv6 in the call list below
+        host = "172.40.0.3"
+        port = 1883
+        self.connect_times = []
+        error_code = MQTT.CONNACK_ERROR_UNAUTHORIZED
+        self.raise_exception = MQTT.MMQTTException(
+            MQTT.CONNACK_ERRORS[error_code], code=error_code
+        )
+
+        with patch.object(socket.socket, "connect") as mock_method:
+            mock_method.side_effect = self.fake_connect
+
+            connect_retries = 3
+            mqtt_client = MQTT.MQTT(
+                broker=host,
+                port=port,
+                socket_pool=socket,
+                ssl_context=ssl.create_default_context(),
+                connect_retries=connect_retries,
+            )
+            print("connecting")
+            with pytest.raises(MQTT.MMQTTException) as context:
+                mqtt_client.connect()
+                assert mqtt_client._sock is None
+            assert "Connection Refused - Unauthorized" in str(context)
+
+            mock_method.assert_called()
+            assert len(self.connect_times) == 1