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

on_publish called before on_connect #521

Closed
tompropst opened this issue Oct 16, 2020 · 4 comments
Closed

on_publish called before on_connect #521

tompropst opened this issue Oct 16, 2020 · 4 comments

Comments

@tompropst
Copy link

I am trying to detect when a connection fails by checking the CONNACK in the on_connect callback. In the case of a failure, I'm calling disconnect in the on_connect callback to break the network loop. In order to also break the network loop when a connection is successful (e.g. in a case where I just want to publish and exit), I also call disconnect in the on_publish callback.

When the connection fails (specifically, connection refused due to authentication failure), the on_publish callback executes first and I am unable to catch the connection failure so all indications are that everything worked.

import logging
import paho.mqtt.client as mqtt

logger = logging.getLogger(__name__)

def on_connect(client, userdata, flags, rc):
    logger.info("Connection result: " + mqtt.connack_string(rc))
    if rc != mqtt.CONNACK_ACCEPTED:
        client.disconnect()

def on_disconnect(client, userdata, rc):
    logger.info("Disconnecting")

def on_publish(client, userdata, mid):
    logger.info("Published message: " + str(mid))
    client.disconnect()

mqtt_client = mqtt.Client()
mqtt_client.enable_logger(logger)
mqtt_client.on_connect = on_connect
mqtt_client.on_disconnect = on_disconnect
mqtt_client.on_publish = on_publish
mqtt_client.connect("some.broker.url", 1883)
mqtt_client.publish("mytopic", "hello")
mqtt_client.loop_forever()

As mentioned in issue #374, on_publish happens as soon as the packets are sent which here, appears to happen before the CONNACK is received. The lack of reliable connection status and error notification seems to be a common problem. A few options would be very helpful.

  1. Allow connection attempts to cease on some errors like a connection refusal or authentication error. These are not likely to change with repeated attempts.
  2. Make connect truly synchronous instead of calling connect_async in all cases.
  3. Maintain a connection status property to make detecting and correcting connection issues more straightforward.
    • Another approach would be to add an on_connection_error callback.

If any of this is amenable to the maintainers, I'm happy to go through the Eclipse registration and submit pull requests.

@ralight
Copy link
Contributor

ralight commented Oct 27, 2020

on_publish is tricky with QoS 0 messages, because we can only detect when the message has been passed to the OS, not when they have been accepted by the broker. So it will almost always fire before the broker can send a connack.

I always recommend that actions go in the on_connect callback where possible, which would catch this, but it does seem that the expectation of synchronous connect() is that the connect/connack flow has occurred. This might be worth a wider discussion on the Paho mailing list, but I think something needs to be done.

@MattBrittan
Copy link
Contributor

Hi @ralight; just noticed your comment on this issue (from a while ago!). I've recently commented on a few related issues (#454, #475, and a few questions on stack overflow) and can see other open issues with the same root cause.

As you say above it looks like there is some confusion about the connection state when connect returns (back to issue #4 where your answer is technically correct but perhaps also a little misleading) and the need for a network loop to complete the connection process.

I'm happy to discuss on the mailing list; but given that this is a python specific question I was not sure if that was the best place. If you are in agreement I'll propose some updates to the documentation in an attempt to clarify this (I would think that changing the behaviour of connect() to wait for the CONNACK would be a breaking change so was not going to propose that). Apologies for not responding to your request for help on the mailing list a while back - my python skills are somewhat limited.

@ralight
Copy link
Contributor

ralight commented Oct 26, 2021

Hi @MattBrittan,

The idea of taking it to the mailing list would be to get a wider audience than is likely to see this. I agree with your points though - changing behaviour now is probably a bad idea and so documentation and examples are the way to help.

If you want to suggest some updates I'd be very happy to see them.

MattBrittan added a commit to ChIoT-Tech/paho.mqtt.python that referenced this issue Oct 29, 2021
@MattBrittan
Copy link
Contributor

Closing this because #615 has now been accepted which, hopefully, clarifies things a bit. If you feel more is needed then please feel free to reopen it.

The MQTT protocol allows messages to be sent prior to a CONNACK being received; we could prevent that from happening but it's something that some users might want to do...

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

No branches or pull requests

3 participants