paho-mqtt-2.0.0 client issue

[liyinan0501]

Bug Description

When running the script with paho-mqtt 2.0.0 and initializing the Client(), an AttributeError occurs: 'Client' object has no attribute '_sock'. However, when changing to paho-mqtt 1.6.1, everything works fine.

Environment

• Python version: 3.8
• Library version: 2.0.0
• Operating system (including version): Ubuntu 20.04.6
• MQTT server (name, version, configuration, hosting details): 2.0.18

Logs

Exception ignored in: <function Client.del at 0x7f23e91e2040>
Traceback (most recent call last):
File "/usr/local/lib/python3.8/site-packages/paho/mqtt/client.py", line 874, in del
self._reset_sockets()
File "/usr/local/lib/python3.8/site-packages/paho/mqtt/client.py", line 1133, in _reset_sockets
self._sock_close()
File "/usr/local/lib/python3.8/site-packages/paho/mqtt/client.py", line 1119, in _sock_close
if not self._sock:
AttributeError: 'Client' object has no attribute '_sock'

[jimmusson]

I have this exact problem with raspberry pi Bullseye 32 bit.

[PierreF]

I believe you didn't provided the full traceback. Please report full traceback when submitting bug report.

I believe something similar to the following is missing:

TypeError: Client.__init__() missing 1 required positional argument: 'callback_api_version'

Please read https://eclipse.dev/paho/files/paho.mqtt.python/html/migrations.html

[matthuisman]

mqttc = mqtt.Client(mqtt.CallbackAPIVersion.VERSION1) fixed for me :)

Problem was that the AttributeError: 'Client' object has no attribute '_sock' was coming out in console and the
TypeError: Client.init() missing 1 required positional argument: 'callback_api_version' was gobbled up elsewhere

[ddevassy]

I also got into the same error.

Is it a good idea to provide a default value for API_VERSION for avoiding the AttributeError? Any thoughts?

[MattBrittan]

@PierreF this is going to come up a lot (there are already two questions on stack overflow). I wonder if it's worth adding something at the start of the readme, and perhaps a pinned issue?

Most tutorials just say to run pip3 install paho-mqtt so will get version 2, meaning that the tutorial code (e.g.) will throw this error (until the tutorial is updated). This is likely to confuse a lot of new users (I understand that it's unavoidable, but I believe it's worth making the solution highly visible). I've reached out to the authors of the tutorials that topped my search results and suggested they update their tutorials to use pip install paho-mqtt<2.0.0 for the time being.

On a linked note the error says see migrations.md for details but that file is not obvious in the repo (the file being [migrations.rst](https://github.com/eclipse/paho.mqtt.python/blob/master/docs/migrations.rst)).

[PierreF]

Is it a good idea to provide a default value for API_VERSION for avoiding the AttributeError? Any thoughts?

I think of this... but it means each time the default change it will cause a (potentially rather silent) breaking change.

On a linked note the error says see migrations.md for details but that file is not obvious in the repo [...]

yeah... I moved the file and forget to update that link.

I'll also try to see if we could avoid the 2nd error (the delete of _sock) to make clearer where is the actual error.

[matthuisman]

yes, if the first real cause was the only error - be much quicker for users to resolve :)

could the default not just be V1?

[MattBrittan]

could the default not just be V1?

This is an option, however I wonder if doing so might just prolong the pain...

With the current solution (deliberately breaking old code) a lot of users are going to hit this message in the near future, learn about the V2 release, and update their code (and, hopefully, tutorials etc). We can ensure the solution is highly visible and attempt to get popular tutorials updated.

If the library defaults to V1 it's likely that a lot of users will not really notice the V2 release. Whilst this would result in less immediate pain, it seems likely to result in ongoing confusion as users copy/paste incompatible code (e.g. copying portions of examples included in this repo into their V1 code bases). This will result in somewhat confusing runtime errors, and it's likely to be more difficult to find solutions (the current message is pretty clear, and the stack overflow question has 300+ views in under a day).

This was always going to be a painful change; but one I was really happy to see Pierre make (the V1 API was confusing; particularly the way the API differed depending upon whether you were connected via V3 or V5!).

[fuomag9]

def connect(data):
    client = mqtt.Client.__init__(CallbackAPIVersion.VERSION2)
    client.username_pw_set(MQTT_USERNAME, MQTT_PASSWORD)
    client.connect(MQTT_BROKER_HOST, MQTT_BROKER_PORT, 60)

Running this gives me:

ERROR:root:Error notifying new subdomains: Client.__init__() missing 1 required positional argument: 'callback_api_version'

I'm using 2.0.0 version

root@d7bdbdd0a329:/app# pip freeze
certifi==2024.2.2
charset-normalizer==3.3.2
filelock==3.13.1
greenlet==3.0.3
idna==3.6
paho-mqtt==2.0.0
psycopg2-binary==2.9.9
requests==2.31.0
requests-file==2.0.0
SQLAlchemy==2.0.28
tldextract==5.1.2
typing_extensions==4.10.0
urllib3==2.2.1

[PierreF]

@fuomag9
I don't understand the way you use Python classes, this seems a wrong usage of Python to me. Could you open another issue with a justification on why paho should support such usage of Python and why it's a paho bug ?

Your error seems perfectly normal to me, since you do not provide the require argument callback_api_version to __init__, you only provide the self argument with a wrong value type (paho expect an instance of Client class).

But I don't understand why you don't use standard way to create a instance ?

client = mqtt.Client(CallbackAPIVersion.VERSION2)

This point don't is not related to this issue, it's another issue. Please open a new issue if the problem persist.

[jimfunk]

As someone who just ran into this, I would like to add my 2 cents.

My problem with the approach taken is that it makes supporting older and newer versions more difficult and it's a bit ugly. For example, now I have to use the library like this:

import paho.mqtt
import paho.mqtt.client as mqtt

USE_VERSION2_CALLBACKS = not paho.mqtt.__version__.startswith("1.")

...

        if USE_VERSION2_CALLBACKS:
            self.mqtt = mqtt.Client(mqtt.CallbackAPIVersion.VERSION2, client_id=mqtt_client_id)
        else:
            self.mqtt = mqtt.Client(client_id=mqtt_client_id)

This is pretty awkward. IMO and actually encourages people to just use VERSION1 and not update to the new ones. I have 6 or 7 apps I need to update to look like this now.

I would have appreciated this approach:

1. Default to the version 2 callback API, or dispense with the notion of a callback API version
2. Add code to the callback property setters to inspect the callback and figure out how many arguments are expected. If the arguments match the old one signature, emit a deprecation warning but allow it to be used for now.

That way the older arguments can be completely deprecated at a later time, but that gives people time to adjust.

As it stands right now, everybody has to make a change right now as it's broken. The point of deprecation warnings is to give people a window to make the changes so that applications do not break. The approach taken defeats this purpose.

If the approach I described above was taken, it would have been a lot more smooth, especially since adjusting the callbacks is not exactly difficult. Why would I really want to make 2 changes at different times when only one is needed?

For example, this is all I could have needed in an app I updated just now to support both:

-     def connect_callback(self, client, userdata, flags, rc)
+     def connect_callback(self, client, userdata, flags, reason_code, properties=None)
...
-     def on_publish(self, client, userdata, mid):
+     def on_publish(self, client, userdata, mid, reason_codes=None, properties=None):

If the approach seems reasonable to you I would be happy to make a PR.

[PierreF]

#831 already do some step to reduce the breaking changes.

On your suggestion, I do have few remark:

• since the callback signature detection might not be able to known the version (most obvious case: def on_connect(*args)), I think we need to be clear what is the behavior of the detection.
• To avoid changes I believe the behavior should be: if signature is the one of callback v2(*) then use version2, else we fallback to version1.
• That probably means the default will be VERSION1 (with possible automatic upgrade to VERSION2). Should we then still call it CallbackAPIVersion.VERSION1 ? Or should it be CallbackAPIVersion.COMPATIBILITY_VERSION ?

(*): I'm not really sure how we could detect a v2 callback, this seems tricky to define. I believe the followings are version 2:

def connect_callback(self, client, userdata, flags, reason_code, properties=None):
def connect_callback(self, client, userdata, flags, reason_code, properties):
def connect_callback(self, client, my_data_names, flags, reason_code, properties):

Note that the difference between the following two is very tiny:

def connect_callback(self, client, userdata, flags, rc, properties=None)   # I believe this one was suggestion in VERSION 1 for MQTT 5 support
def connect_callback(self, client, userdata, flags, reason_code, properties=None):

If I understand right your use-case, it's on a application where a 3rd party library (or just something not easy to upgrade) create the paho client, but you can upgrade the callback ?
In that case, the version detection might always make sense, and we will probably kept it forever, just that detection version might evolve (today v1 and v2, later v2 and v3...).

So you suggestion seems a good idea, but I think we will need an option to skip version detection and choose a specific version.

[jimfunk]

@PierreF When I made my suggestion I didn't realize that MQTT5 changed the values of the reason codes. That is unfortunate and certainly makes the heuristic a bit more difficult.

However, it seems the existence of the properties field as a positional or keyword arg may indicate that it was intended for either the 1.x release with MQTT5 support or the 2.x release, in which case the ReasonCode instance would probably be appropriate. If the properties field was not present at all, there would be no ambiguity.

However, I would suspect that most instances of the connect callbacks are either ignoring rc/reason_code or merely logging it.

But yeah, thanks to the reason code change, it can't actually be perfect, and an override would probably be warranted.

I still think the notion of a callback API version in this manner may make it more complicated than it needs to be. However, it might make more sense if that was defined explicitly via a namespace, for example:

# V1 explicit
client.callback.v1.connect = 

# V1 compatibility
client.on_connect = ...

# V2
client.callback.v2.connect = 

And then for each time it needs to perform a callback it could check for a v2 callback, then a v1 callback. That would be a much bigger change however and look less ergonomic.

[mschlenstedt]

Well, a lot of existing projects are broken now. How should a user without programming skills should handle this?! These changes are the badest idea ever - sorry to be honest.

[uwedisch]

Well, a lot of existing projects are broken now. How should a user without programming skills should handle this?! These changes are the badest idea ever - sorry to be honest.

For example notus-scanner and ospd-openvas, both of GVM, are broken.

That is bad coding style. Changes must maintain "legacy" intefaces, i.e. the V1 must work without any knowledge of V2.

[uwedisch]

If the library defaults to V1 it's likely that a lot of users will not really notice the V2 release.

I think, it's more important for the whole community to have stable code than beeing always informed about new features. A new feature is worthless if something is broken. And for sure, you have many ways of informing users of new attributes ...

[mschlenstedt]

To be honest an end-user is not interested in any APIs, versions, super-dupa-new features. It should just work and they want to have their values in their MQTT brokers. And that is what V1 did now for years. Maybe V2 is really a big step forward, but only programmers will be interested in that.

All other just want to have a running system.

Another broken project now: flyte/mqtt-io#348 The authors now discuss to change their whole (!!!) code basis just to be comptatible with paho-mqtt V2. What a waste of limited ressources...

[uwedisch]

Well, a lot of existing projects are broken now. How should a user without programming skills should handle this?! These changes are the badest idea ever - sorry to be honest.

For example notus-scanner and ospd-openvas, both of GVM, are broken.

That is bad coding style. Changes must maintain "legacy" intefaces, i.e. the V1 must work without any knowledge of V2.

In that situation GVM is part of Kali Linux using the pre-packaged Python libraries.

[uwedisch]

Did I see that correctly?
V2 was introduced some weeks ago and since that V1 isn't working anymore without change.
It is part of being patient with each other to make such a serious change a year or so in advance.

[OptimusGREEN]

2.1.0 fixes this for me