Fix: Abort connection handshake when connection timeout is reached #84
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is a potential fix for the issue described in #83.
Background:
As part of the
ConnectionSettings, a connection timeout can be configured. This timeout is used when establishing a socket connection to the broker. It is not used after successfully establishing a socket connection though (i.e. when reading and writing data).Because we can connect to any port with our client (like
22for SSH instead of1883for MQTT), we should also detect failed connection attempts after successfully establishing a socket connection. This happens automatically if the broker (or SSH server in this example) responds with an invalid response. It does not happen when no response is received at all though. Unfortunately, in containerized environments, this is a common thing to happen because the container runtime listens on forwarded ports immediately after installing a container, even if the application within the container has not finished starting (and started to listen on specific ports) yet.Solution:
During the connection handshake, the
MqttClientis actively polling the socket for new data. If no data is received, the client keeps polling infinitely (current situation). Because users may expect the connection handshake to be part of the connection phase, the configured connection timeout should apply here as well. Therfore we simply check if the timeout is reached and exit with an exception if that is the case.This solution does actually allow the configured connection timeout to be applied twice, once for establishing a socket connection and a second time for the connection handshake. In theory, this leaves us with a connection phase which can take twice as long as the configured connection timeout.
In real world scenarios, it is very unlikely for this to happen though. Therefore we accept the inaccuracy at this point.