Closed mgsalafia closed 9 months ago
I think it would be proper to close the connection after the future has been sent, can you try that and see what it gives ?
@vietj i changed the connack() method as follows:
private MqttEndpointImpl connack(MqttConnectReturnCode returnCode, boolean sessionPresent, MqttProperties properties) {
MqttFixedHeader fixedHeader =
new MqttFixedHeader(MqttMessageType.CONNACK, false, MqttQoS.AT_MOST_ONCE, false, 0);
MqttConnAckVariableHeader variableHeader =
new MqttConnAckVariableHeader(returnCode, sessionPresent, properties);
io.netty.handler.codec.mqtt.MqttMessage connack = MqttMessageFactory.newMessage(fixedHeader, variableHeader, null);
// now close the socket connection or set IsConectd = true only when the future completes
this.write(connack).onComplete(ar -> {
// if a server sends a CONNACK packet containing a non zero return code it MUST then close the Network Connection (MQTT 3.1.1 spec)
if (returnCode != MqttConnectReturnCode.CONNECTION_ACCEPTED) {
this.close();
} else {
this.isConnected = true;
}
});
return this;
}
and i still have my issue. But, i think my problem is not due to this because, as far i understand just going depth in the code, the instruction this.write(connack)
runs on the event loop, thus this.close() runs only when the write() completes (the next cycle of the event loop). So i think the problem is related to how the client-side websocket connection is managed in mqtt.js in the browser.
By the way, even though my problem is not due to the connack(), i think that the Future returned by the write() mehtod must be used in order to continue with either the close() call or the isConnected = true. The reason is that (i repeat, as far i can understand looking at the code) there are no guarantees that this.write() code runs in the event loop. In fact, going down following the write() call, there are lot of if
statements checking if the channel context is in the event loop.
Hope this can help you.
do you know any way to reproduce this ?
I think you can try to develop a simple javascript mqtt client using mqtt.js (in browser, not node.js) that just connect to the vertx backend. In the handle
function of the handler just reject the connection. This is the simplest scenario i think.
@msalafia can you share a wireshark dump of the connection traffic ?
please provide a reproducer for this and reopen an issue
I am using a browser version of mqtt.js to create an MQTT client and trying to connect with an MQTT broker created with vertx-mqtt.
When the broker reject the connection coming from the client from whatever reason, the client sometimes receive the CONNACK and sometimes don't (causing my mqtt.client to trying to reconnect indefinitely when a connack is not received).
From broker-side in my MqttEndpointHandler, i reject using the following code:
Going in depth, i found out that reject() in turn calls connack() of MqttEndpointImpl. The code of connack seems writing the message and closing the connection right after(
this.close()
). This seems to cause my issue because client-side the event 'close' on the websocket connection cause a 'reconnect' event and the connack is ignored.Here the code of connack():
As i commented in the code, this.write return a Future but is not being used at all and the method this.close() is called sequentially right after.
If i put a break point in this.close() (hence preventing the close of the websocket connection), the mqtt-client work like expected receiving the CONNACK for the mqtt connection and then printing the right error.
Is this a bug maybe? Or am i doing something wrong?
Version
4.2.1