Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

disabling AutoReconnect causes spurious error to be logged #697

Open
rittneje opened this issue Nov 27, 2024 · 2 comments
Open

disabling AutoReconnect causes spurious error to be logged #697

rittneje opened this issue Nov 27, 2024 · 2 comments

Comments

@rittneje
Copy link
Contributor

We set AutoReconnect to false in our ClientOptions. When the connection to the broker is lost unexpectedly, we see the following error come from this library:

[client] BUG BUG BUG reconnection function is nil

Tracing through the code, this is because disDone is assigned like so.

disDone, err := c.status.ConnectionLost(c.options.AutoReconnect && c.status.ConnectionStatus() > connecting)

Since c.options.AutoReconnect is false, that will pass false to c.status.ConnectionLost. Thus the callback that it returns will itself return nil, nil.

paho.mqtt.golang/status.go

Lines 278 to 280 in 714f7c0

if !reconnectRequested || !proceed {
return nil, nil
}

This in turn triggers the log that "should never happen".

paho.mqtt.golang/client.go

Lines 547 to 552 in 714f7c0

reConnDone, err := disDone(true)
if err != nil {
ERROR.Println(CLI, "failure whilst reporting completion of disconnect", err)
} else if reConnDone == nil { // Should never happen
ERROR.Println(CLI, "BUG BUG BUG reconnection function is nil", err)
}

I believe this regression was introduced in v1.4.2. It prevents us from upgrading from v1.3.5.

@stoxx
Copy link

stoxx commented Nov 27, 2024

I've assumed it's harmless.

@MattBrittan
Copy link
Contributor

The library has quite a few options and I was concerned that I may have missed some possible combinations when rearchitecting the status handling (it was a bit of a mess), hence the "BUG BUG" log entry. Have had a quick look and the log entry should be supressed if c.options.AutoReconnect == false (probably best to add something like reconnectExpected := c.options.AutoReconnect && c.status.ConnectionStatus() > connecting at the top, and then use that value).
Having said that the code looks like it will work OK, it's just an incorrect log entry. Will fix when I get a chance (or would welcome a PR).

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants