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

client: don't hang indefinitive when the connection break #153

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lynxis
Copy link

@lynxis lynxis commented Oct 28, 2023

When the connection closes all client task should terminate. But following example would hang even when the broker closes the connection:

mqcli = MQTTClient(config={'auto_reconnect': False})
await mqcli.connect('mqtt://localhost/')
await mqcli.subscribe([('some/topic', QOS_2),])
message = await mqcli.deliver_message()

The problem lies in the task.set_exception(). It should terminate all pending task. But the problem is Tasks doesn't support set_exception() and this call raise itself an exception.
As the docs state:
"asyncio.Task inherits from Future all of its APIs except Future.set_result() and Future.set_exception()."

However after implementing it. message will return None without any exception.

When the connection closes all client task should terminate.
But following example would hang even when the broker closes the connection:

```
mqcli = MQTTClient(config={'auto_reconnect': False})
await mqcli.connect('mqtt://localhost/')
await mqcli.subscribe([('some/topic', QOS_2),])
message = await mqcli.deliver_message()
```

The problem lies in the task.set_exception(). It should terminate all
pending task. But the problem is Tasks doesn't support set_exception()
and this call raise itself an exception.
As the docs state:
"asyncio.Task inherits from Future all of its APIs except Future.set_result() and Future.set_exception()."

However after implementing it. message will return None without
any exception.
@lynxis
Copy link
Author

lynxis commented Oct 28, 2023

This PR only handles the non-auto-reconnecting case.

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.

1 participant