Skip to content
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

Show the reason for client disconnects #33

Closed
jmacd opened this issue Mar 8, 2022 · 1 comment
Closed

Show the reason for client disconnects #33

jmacd opened this issue Mar 8, 2022 · 1 comment

Comments

@jmacd
Copy link
Contributor

jmacd commented Mar 8, 2022

While debugging an application, I was at some point confounded by what looked to both the client and the server like the other had side had closed the connection. On further investigation, there were two identical clients, one a zombie process, that were both trying to connect, and each time one connects the other is disconnected.

The errors that were printed in OnDisconnect both do not include the remote address field, which revealed the presence of duplicate client IDs, these also happened to look like ordinary client-initiated disconnects, so the server logs would not reveal there to be a session takeover.

Auditing the code, I learned that both an explicit Disconnect packet could trigger disconnection, but so could EOF or a variety of errors in several code paths. The existing Client was well synchronized for this, but the original cause of stopping the connection was being lost. This adds the first cause to stop the connection. In EstablishConnection(), the root cause will be the one passed to OnDisconnect, so the server logs will reveal the root of the problem. (Since this is MQTT 3.x, there is no way for the Disconnect packet to tell the client, so it could place an explanation in its own logs.)

In addition, there would be no way for a user to configure visibility into other errors, which might less obviously be the cause of or associated with disconnection or simply provide warning flags. Proposing to add an OnError handler to capture support logging all errors.

Related to #21, since if there are duplicate clients it will be nice to say where they're coming from.

@jmacd
Copy link
Contributor Author

jmacd commented Mar 15, 2022

Fixed by #38.

@jmacd jmacd closed this as completed Mar 15, 2022
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

Successfully merging a pull request may close this issue.

1 participant