Hono: MQTT Adapter: Close device connection when a telemetry/event message cannot be processed.

[kaniyan]

A device publishes an _event/telemetry_ message with _QoS 1_. The _MQTT adapter_ after successfully establishing the device connection, realizes that the message cannot be _processed_ e.g _no credit available_, _the message limit is exceeded_ etc. In this case the device remains connected and keeps waiting for the PUBACK packet from the Hono.

Similar scenario is when a device publishes a _telemtry/event_ message with _QoS 2_ (_not supported by Hono_). In this case, the device remain connected and it keep waiting for the PUBREC packet.

_MQTT 3.1.1 spec_ (MQTT-3.3.5-2) specifies that there is no way to inform the MQTT client that the _publish_ action is not accepted. The server must either make a positive acknowledgement or _close_ the _Network Connection._

[sophokles73]

From the spec:

If a Server implementation does not authorize a PUBLISH to be performed by a
Client; it has no way of informing that Client. It MUST either make a positive
acknowledgement, according to the normal QoS rules, or close the Network
Connection.

First, this seems to refer to a case where the server considers the client to not be authorized to PUBLISH the message. Your example is about insufficient credits, though, which seems to be a different case FMPOV. However, the problem with MQTT 3.1.1 lacking a mechanism to convey an error situation to a client arises in multiple situations. We have discussed this in the past and always came to the conclusion that there is no real use in closing the connection, because the client will (most likely) immediately re-open the connection and re-send the very same message again. On the other hand, the MQTT spec also advises clients to wait for a reasonable amount of time for the server's PUBACK and after that consider the message to be lost. Thus, a client needs to deal with this situation anyway, so FMPOV it doesn't really make much sense to force the client to go through the (expensive) TLS handshake again, just to end up in the same situation as it would be in without disconnecting the client in the first place.

Does this make sense to you?

[ctron]

I made the experience that e.g. Paho doesn't handle missing ACKs very well. As it simply seems to follow the spec and waits/blocks until all ACKs have been received.

With QoS 1 the clients wait for an ACK, as the remote side either sends an ACK, or is expected to close the connection.

So I agree that the client also has to handle the situation where the ACK is missing/lost due to other connection issues. However making it wait for something that the server already knows will never be sent, doesn't add much value IMHO.

I can imagine that in some scenarios you might be better off not closing the connection. However in the more standard scenarios people just complaint that this "it is stuck". I just had the issue in EnMasse tests, where the CI blocks, because Paho blocks, because something went wrong on the server side. Setting the number of "inflight" to a rather high number works around that issue, but it is not more than a workaround.

I think it might make sense to make this behavior configurable. Defaulting to closing the connection, and allowing to keep the connection alive when requested. This should at least be configurable on a tenant level.

[kaniyan]

I also have the same experience with _MQTT_ clients (Paho and mosquitto clients) which kept waiting indefinitely. IMHO it is better to keep the behavior of the MQTT communication in Hono as close to the MQTT spec as possible. Thereby users can always refer to it.

So I agree that the client also has to handle the situation where the ACK is missing/lost due to other connection issues. However making it wait for something that the server already knows will never be sent, doesn't add much value IMHO.

+1. Waiting for the ACK even for a defined period of time also adds overhead to the IoT devices. Since the server already knows that no ACK will be sent, this can be well avoided.

IMHO if it is going to behave as per the _MQTT_ spec, I wouldn't even make it configurable. If we decide on making it configurable, then as @ctron suggests it makes sense to keep the behavior (closing the connection) which is as per the spec as the default behavior.

[sophokles73]

@dejanb @calohmn what's your opinion on this?

[calohmn]

Defaulting to closing the connection sounds reasonable to me, preventing clients getting stuck.

As to whether to make it configurable:
Thinking of a heterogenous client landscape concerning one specific tenant, making it configurable for just the tenant might cause people to want to make it also configurable on a per-device basis.
But I don't know if we want to go this way and introduce such technical details in device data.

How about supporting a property bag option appended to the topic to enable keeping the connection open?

[sophokles73]

With QoS 1 the clients wait for an ACK, as the remote side either sends an ACK, or is expected to close the connection.

At first I didn't think that this is actually mandated by the spec but in 4.8 this seems to be actually stated as a requirment:

If the Client or Server encounters a Transient Error while processing an inbound Control Packet it MUST close the Network Connection on which it received that Control Packet [MQTT-4.8.0-2]. If a Server
detects a Transient Error it SHOULD NOT disconnect or have any other effect on its interactions with any
other Client.

So it seems that closing the connection on any problems we encounter processing an incoming message, actually seems to be what the spec expects us to do. In order to keep things simple I would then propose to not make this configurable (yet). If at a later time we find this helpful, we can still add it.

@kaniyan @calohmn @dejanb @ctron WDYT?

[kaniyan]

@sophokles73 :+1: