connect() always returns 0, even if connection fails.

acetylen commented 5 years ago

Summary

The paho.mqtt.client.Client.connect() function call always returns zero, even on failed connections.

If the intention is to use the on_connect and on_disconnectcallbacks, then surely the connect method should not return anything? This behavior is needlessly confusing.

Software

paho-mqtt version: 1.4.0
python version: 3.7.1
mosquitto version: 1.4.15

Test setup

Locally running instance of mosquitto
Test script mqtt_tester.py

Mosquitto configuration:

$ cat mosquitto.conf
password_file mosquitto.passwd
allow_anonymous false

$ mosquitto_passwd -b mosquitto.passwd testuser test

Contents of mqtt_tester.py:

import paho.mqtt.client as mqtt

client = mqtt.Client("mqtt_tester")
client.username_pw_set("testuser", "test")

rc = client.connect("127.0.0.1", 1883)
print("connect() return code:", rc)

client.publish("TEST", "test")

try:
    client.loop_forever()
except KeyboardInterrupt:
    client.disconnect()
    exit(0)

Method

Start mosquitto -v -c mosquitto.conf
In another terminal, start mosquitto_sub -t "#" -v -u testuser -P test
in a third terminal, start python3 mqtt_tester.py

Test

Case 1: correct credentials

mosquitto -v -c mosquitto.conf output:

mosquitto version 1.4.15 (build date Wed, 13 Feb 2019 00:27:01 +0000) starting
Config loaded from mosquitto.conf.
Opening ipv4 listen socket on port 1883.
Opening ipv6 listen socket on port 1883.

New connection from 127.0.0.1 on port 1883.
New client connected from 127.0.0.1 as mosqsub (c1, k60, u'testuser').
Sending CONNACK to mosqsub (0, 0)
Received SUBSCRIBE from mosqsub
    # (QoS 0)
mosqsub 0 #
Sending SUBACK to mosqsub

New connection from 127.0.0.1 on port 1883.
New client connected from 127.0.0.1 as mqtt_tester (c1, k60, u'testuser').
Sending CONNACK to mqtt_tester (0, 0)

Received PUBLISH from mqtt_tester (d0, q0, r0, m0, 'TEST', ... (4 bytes))
Sending PUBLISH to mosqsub (d0, q0, r0, m0, 'TEST', ... (4 bytes))

Received DISCONNECT from mqtt_tester
Client mqtt_tester disconnected.

Socket error on client mosqsub, disconnecting.

python3 mqtt_tester.py output:

connect() return code: 0

mosquitto_sub -t "#" -v -u testuser -P test output:

TEST test

Case 2: incorrect credentials

for this case, line 5 of mqtt_tester.py was changed from

client.username_pw_set("testuser", "test")

to

client.username_pw_set("testuser", "wrong")

No other changes were made.

mosquitto -v -c mosquitto.conf output:

mosquitto version 1.4.15 (build date Wed, 13 Feb 2019 00:27:01 +0000) starting
Config loaded from mosquitto.conf.
Opening ipv4 listen socket on port 1883.
Opening ipv6 listen socket on port 1883.

New connection from 127.0.0.1 on port 1883.
New client connected from 127.0.0.1 as mosqsub (c1, k60, u'testuser').
Sending CONNACK to mosqsub (0, 0)
Received SUBSCRIBE from mosqsub
    # (QoS 0)
mosqsub 0 #
Sending SUBACK to mosqsub

New connection from 127.0.0.1 on port 1883.
Sending CONNACK to 127.0.0.1 (0, 5)
Socket error on client <unknown>, disconnecting.

Socket error on client mosqsub, disconnecting.

mosquitto_sub -t "#" -v -u testuser -P test output:

python3 mqtt_tester.py output:

connect() return code: 0

vrst37 commented 5 years ago

From my understanding the code is supposed to return 0.

The intention is definitely to use the on_connect and on_disconnect callbacks.

I think the behaviour might be residues from ancient times of this library (i.e. to maintain compatibility and not break too many things).

@PierreF @ralight can any one you confirm so we can close this issue?

acetylen commented 5 years ago

My problem is specifically that I'm dealing with a system that runs a non-standard broker, that doesn't signal failure properly. If the client provides bad credentials, the broker holds the connection open until the other side times out. This makes it hard to use the callbacks as none of them fire, and it also makes it hard to use connect as it returns immediately. With is library there's basically no way to know failure state.

MattBrittan commented 9 months ago

that doesn't signal failure properly.

So, if I'm understanding you correctly, the broker does not return a CONNACK or drop the connection?

The only real option the spec offers in this situation is a timeout:

If the Client does not receive a CONNACK Packet from the Server within a reasonable amount of time, the Client SHOULD close the Network Connection. A "reasonable" amount of time depends on the type of application and the communications infrastructure.

I've had a scan through the library and there does not seem to be any option for a CONNACK timeout; as such I'm going to tage this issue as an enhancement request.

eclipse / paho.mqtt.python