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

Use Token.WaitTimeout #192

Merged
merged 1 commit into from
Dec 12, 2023
Merged

Use Token.WaitTimeout #192

merged 1 commit into from
Dec 12, 2023

Conversation

subpop
Copy link
Collaborator

@subpop subpop commented Dec 6, 2023

This is a backport of #165 to yggdrasil-0.2.

When connecting to a broker and publishing a message, use the
Token.WaitTimeout method instead of Token.Wait. Token.Wait waits
indefinitely, which can lead to situations when the cleint never
succeeds in connecting or publishing.

The timeout for each operation can be configured independently by
setting mqtt-connect-timeout and mqtt-publish-timeout. Both values
default to 30 seconds. The flags are hidden, as they should not commonly
be required to be changed by users.

Fixes: CCT-123

@subpop subpop requested review from jirihnidek and ahitacat December 6, 2023 17:21
Copy link
Contributor

@ahitacat ahitacat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks good. Although I have one question. Why on mqtt.go not all the publish that use toke.Wait uses the timeout? (case Ping or cas reconnect)

@subpop
Copy link
Collaborator Author

subpop commented Dec 11, 2023

Ah, this was an oversight on my part. I wrote this patch using the main branch as a reference. The main branch has consolidated all calls to the MQTT library into the transport.MQTTTransport type, so that branch only had to change it in one place. For this branch, I needed to change it in more places (and just forgot). Good catch!

When connecting to a broker and publishing a message, use the
`Token.WaitTimeout` method instead of `Token.Wait`. `Token.Wait` waits
indefinitely, which can lead to situations when the cleint never
succeeds in connecting or publishing.

The timeout for each operation can be configured independently by
setting `mqtt-connect-timeout` and `mqtt-publish-timeout`. Both values
default to 30 seconds. The flags are hidden, as they should not commonly
be required to be changed by users.

Signed-off-by: Link Dupont <[email protected]>
@subpop
Copy link
Collaborator Author

subpop commented Dec 11, 2023

Updated to include the missed uses of `Token.Wait().

Copy link
Contributor

@ahitacat ahitacat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ahitacat ahitacat merged commit 17504ee into yggdrasil-0.2 Dec 12, 2023
6 checks passed
@ahitacat ahitacat deleted the timeout branch December 12, 2023 11:23
Copy link
Contributor

@jirihnidek jirihnidek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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 this pull request may close these issues.

3 participants