Skip to content

Client disconnected when error code is LWMQTT_BUFFER_TOO_SHORT #120

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

Closed
luffykesh opened this issue Aug 27, 2018 · 4 comments
Closed

Client disconnected when error code is LWMQTT_BUFFER_TOO_SHORT #120

luffykesh opened this issue Aug 27, 2018 · 4 comments

Comments

@luffykesh
Copy link
Contributor

luffykesh commented Aug 27, 2018

The socket is disconnected when data length greater than the buffer size is received, as a result LWT is published by the broker.

I think, connection should not be closed, and either

  1. The data should be discarded
  2. The maximum data that can be fit into the buffer should be served, or
  3. A option of adding a stream should be given to support maximum possible data length, like as in PubSubClient library.
@luffykesh luffykesh changed the title Client disconnected is error code is LWMQTT_BUFFER_TOO_SHORT Client disconnected when error code is LWMQTT_BUFFER_TOO_SHORT Aug 27, 2018
@luffykesh
Copy link
Contributor Author

I wrote a work around to reconnect, if the error code was on of the following:
LWMQTT_BUFFER_TOO_SHORT
LWMQTT_VARNUM_OVERFLOW
LWMQTT_NETWORK_TIMEOUT
LWMQTT_NETWORK_FAILED_READ
LWMQTT_NETWORK_FAILED_WRITE
LWMQTT_REMAINING_LENGTH_OVERFLOW
LWMQTT_REMAINING_LENGTH_MISMATCH
LWMQTT_MISSING_OR_WRONG_PACKET
LWMQTT_PONG_TIMEOUT
LWMQTT_SUBACK_ARRAY_OVERFLOW

But, if let's say internet is not available through the wireless router, a false error code LWMQTT_NETWORK_FAILED_WRITE is reported and a false return code LWMQTT_CONNECTION_ACCEPTED is reported

@256dpi
Copy link
Owner

256dpi commented Aug 31, 2018

On your last comment:

I fixed the wrong return code here 4d9e708. Thanks for noticing. The LWMQTT_NETWORK_FAILED_WRITE error is correct as writing to the (not anymore) available networked failed. Or how do you mean it's wrong exactly?

On your first comment:

  1. I don't think discarding the data is OK, as sending a payload to big to process can be an accident. And the LWMQTT_BUFFER_TOO_SHORT error identifies that issue clearly. Alternatively, we could add a setting that allows to set the client in a "discard if too big" mode.

  2. That would be a possibility. We would also need to provide a flag bool capped to indicate that the message was no read fully. However, I don't know what the user will do with that data (a JSON payload could not be parsed anyway) a part from logging "a message was too big".

  3. This would be clearly the best solution. Instead of directly supporting C++ streams, I would tend to provide a third callback with a signature like the following:

void messageReceived(MQTTClient *client,
                     char topic[],
                     char payload[],
                     int payload_length,
                     bool chunked,
                     int offset,
                     int total_length) {}

If chunked is true, payload and payload_length represent a chunk while offset and total_length can be used to determine the position in the complete message.

I'm strongly in favor of the last solution as it is the most natural to me. When the "chunked" API has landed in LWMQTT we can easily add another callback signature (as described above) to let the user opt-in if necessary.

@luffykesh
Copy link
Contributor Author

Umm...I guess, yes the error code LWMQTT_NETWORK_FAILED_WRITE is fine.

I was not suggesting supporting C++ streams, but rather a Print class object: like here , although the third solution you suggested is the preferable one.

But until then, I guess the switching to disconnected state should not be done on buffer overflow errors.

@256dpi
Copy link
Owner

256dpi commented Feb 8, 2019

Closing this in favor of #145.

@256dpi 256dpi closed this as completed Feb 8, 2019
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

2 participants