Socket not closed if first connect fails/wrong authentication

[kevinkk525]

I took a careful look at the connect function which raises an error if the first connection to the broker fails, which could be due to not being able to connect to wifi, broker offline or wrong authentication. This means that there is a possibility that the socket remains open. Since I have to build a loop around the first client.connect as the broker might just be down at the moment my device (re)starts, I would have to close the socket myself using:

while True:
    try:
        await client.connect()
    except OSError:
        client.close()
        await asyncio.sleep(5)

This is of course acceptable but a bit unexpected because if I can't connect, I don't expect to close the client because it is not connected. You document in the README that client.connect() raises an OSError if the connection fails but do not mention that client.close() might be needed and the example code doesn't cover the possibility that client.connect() raises an OSError.

A different approach would be to change the client.connect() method to:

    async def connect(self):
        if not self._has_connected:
            await self.wifi_connect()  # On 1st call, caller handles error
            # Note this blocks if DNS lookup occurs. Do it once to prevent
            # blocking during later internet outage:
            self._addr = socket.getaddrinfo(self.server, self.port)[0][-1]
        self._in_connect = True  # Disable low level ._isconnected check
        clean = self._clean if self._has_connected else self._clean_init
        try:
            await self._connect(clean)
        except Exception:
            self.close()
            raise

This way the socket is always cleaned up and the user doesn't need to worry about it.

I understand that this might be a minor issue because it will only be a problem if the restart of a device happens during an outage of the broker or if the device has wrong credentials (and did not previously connect successfully).

[peterhinch]

Thank you, that looks good. I will push an update shortly.