Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix future exception on disconnect #25

Conversation

MartinHjelmare
Copy link
Contributor

@MartinHjelmare MartinHjelmare commented Nov 27, 2020

I'm testing this library to use with Home Assistant. I've written a client based on the advanced example in the readme.

It seems we're setting an exception on the self._disconnected asyncio.Future used in _on_disconnect, even though the future is never awaited if there's an exception in the connect flow.

Without this fix, when the connection is refused, I'm seeing:

2020-11-27 01:20:49,138 INFO (MainThread) [openzwavemqtt] Starting client.
2020-11-27 01:20:49,138 DEBUG (MainThread) [asyncio] Using selector: EpollSelector
2020-11-27 01:20:49,147 DEBUG (ThreadPoolExecutor-0_0) [paho.mqtt.client] Sending CONNECT (u0, p0, wr0, wq0, wf0, c1, k60) client_id=b'6SHLIyeUtZENAYM3p4xxFi'
2020-11-27 01:20:49,150 DEBUG (MainThread) [paho.mqtt.client] Received CONNACK (0, 5)
2020-11-27 01:20:49,150 ERROR (MainThread) [openzwavemqtt] MQTT error: [code:5] Connection refused: Not authorised. Reconnecting in 2 seconds
2020-11-27 01:20:51,153 ERROR (MainThread) [asyncio] Future exception was never retrieved
future: <Future finished exception=MqttCodeError('Unexpected disconnect')>
asyncio_mqtt.error.MqttCodeError: [code:5] Unexpected disconnect
2020-11-27 01:20:51,159 DEBUG (ThreadPoolExecutor-0_0) [paho.mqtt.client] Sending CONNECT (u0, p0, wr0, wq0, wf0, c1, k60) client_id=b'6SHLIyeUtZENAYM3p4xxFi'
2020-11-27 01:20:51,161 DEBUG (MainThread) [paho.mqtt.client] Received CONNACK (0, 5)
2020-11-27 01:20:51,162 ERROR (MainThread) [openzwavemqtt] MQTT error: [code:5] Connection refused: Not authorised. Reconnecting in 4 seconds
2020-11-27 01:20:55,296 DEBUG (ThreadPoolExecutor-0_0) [paho.mqtt.client] Sending CONNECT (u0, p0, wr0, wq0, wf0, c1, k60) client_id=b'6SHLIyeUtZENAYM3p4xxFi'
2020-11-27 01:20:55,301 DEBUG (MainThread) [paho.mqtt.client] Received CONNACK (0, 5)
2020-11-27 01:20:55,302 ERROR (MainThread) [openzwavemqtt] MQTT error: [code:5] Connection refused: Not authorised. Reconnecting in 8 seconds
^C2020-11-27 01:20:55,976 INFO (MainThread) [openzwavemqtt] Exiting client.
2020-11-27 01:20:55,982 ERROR (MainThread) [asyncio] Future exception was never retrieved
future: <Future finished exception=MqttCodeError('Unexpected disconnect')>
asyncio_mqtt.error.MqttCodeError: [code:5] Unexpected disconnect
2020-11-27 01:20:55,983 ERROR (MainThread) [asyncio] Future exception was never retrieved
future: <Future finished exception=MqttCodeError('Unexpected disconnect')>
asyncio_mqtt.error.MqttCodeError: [code:5] Unexpected disconnect

With this fix:

2020-11-27 01:20:30,700 INFO (MainThread) [openzwavemqtt] Starting client.
2020-11-27 01:20:30,700 DEBUG (MainThread) [asyncio] Using selector: EpollSelector
2020-11-27 01:20:30,709 DEBUG (ThreadPoolExecutor-0_0) [paho.mqtt.client] Sending CONNECT (u0, p0, wr0, wq0, wf0, c1, k60) client_id=b'1c0Jnihm0DyAfyK4ETcfXp'
2020-11-27 01:20:30,712 DEBUG (MainThread) [paho.mqtt.client] Received CONNACK (0, 5)
2020-11-27 01:20:30,713 ERROR (MainThread) [openzwavemqtt] MQTT error: [code:5] Connection refused: Not authorised. Reconnecting in 2 seconds
2020-11-27 01:20:32,722 DEBUG (ThreadPoolExecutor-0_0) [paho.mqtt.client] Sending CONNECT (u0, p0, wr0, wq0, wf0, c1, k60) client_id=b'1c0Jnihm0DyAfyK4ETcfXp'
2020-11-27 01:20:32,725 DEBUG (MainThread) [paho.mqtt.client] Received CONNACK (0, 5)
2020-11-27 01:20:32,725 ERROR (MainThread) [openzwavemqtt] MQTT error: [code:5] Connection refused: Not authorised. Reconnecting in 4 seconds
2020-11-27 01:20:36,731 DEBUG (ThreadPoolExecutor-0_0) [paho.mqtt.client] Sending CONNECT (u0, p0, wr0, wq0, wf0, c1, k60) client_id=b'1c0Jnihm0DyAfyK4ETcfXp'
2020-11-27 01:20:36,734 DEBUG (MainThread) [paho.mqtt.client] Received CONNACK (0, 5)
2020-11-27 01:20:36,735 ERROR (MainThread) [openzwavemqtt] MQTT error: [code:5] Connection refused: Not authorised. Reconnecting in 8 seconds
^C2020-11-27 01:20:37,383 INFO (MainThread) [openzwavemqtt] Exiting client.

@@ -320,7 +320,7 @@ async def __aexit__(self, exc_type, exc, tb):
# Early out if already disconnected...
if self._disconnected.done():
disc_exc = self._disconnected.exception()
if disc_exc is None:
if disc_exc is not None:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change isn't required to resolve the issue, but it seems like a typo so I included it. Let me know if you want that as a separate PR/discussion.

@frederikaalund
Copy link
Collaborator

Hi Martin. Thanks for the pull request. Let me have a look. :)

Great catch! Both of them, that is. :)) I thought I got rid of all those annoying "Future exception was never retrieved" messages. Guess I was wrong.

Looks good to me. 👍


I'm testing this library to use with Home Assistant. I've written a client based on the advanced example in the readme.

Glad to hear that people use asyncio-mqtt in practice. Let me know if you run into other bugs in the implementation or quirks in the API design itself. I'm planning a 1.0.0 release at the end of the year.

@frederikaalund frederikaalund merged commit 9830e5a into empicano:master Nov 27, 2020
@MartinHjelmare MartinHjelmare deleted the fix-exception-connection-on-disconnect branch November 27, 2020 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants