search

eclipse-paho / paho.mqtt.golang

Other
2.77k stars 534 forks source link

No way of retrieving the SessionPresent flag after an autoreconnection #582

Open rbino opened 2 years ago

rbino commented 2 years ago

As described in #240, the session present flag exposed by the ConnAck packet can be used to perform some optimization, and in our specific usecase is part of the protocol itself). Currently, Paho exposes the flag when performing the first connection, but it does not allow to access it during reconnections. We're currently using our fork which exposes the flag in OnConnectHandler (breaking the current API for the callback). If it's fine for you we can submit a pull request with the above mentioned implementation or, if you don't want to break the API and you identify an alternative way to expose the flag during the reconnection, let us know so we can provide a different one.

MattBrittan commented 2 years ago

As you say I'd be reluctant to change the definition of OnConnectHandler because this would be a breaking change (and require a v2 release).

I think that the approach mentioned here might provide what you want? This would effectively replace the onConnect handler with something more flexible (allowing future changes to be made in a non-breaking way).

Annopaolo commented 2 years ago

Your approach is interesting, thanks. I think the session present flag could be exposed in the extraData field when needed. However, your suggestion seems still rather sketchy; do you have any plans about the implementation? If so, did you already think about an ETA?

MattBrittan commented 2 years ago

@Annopaolo - it was a suggestion on how this could be accomplished in a way that would have minimal impact on the client; not a commitment to implement it myself (it's not something I'll use and I'm fairly busy currently). The implementation should be fairly simple (I'd probably add a func (c *client) commsNotification(type CommsNotification, err error.Error, extraData interface{}) function to actually call the callback as we would need to check for a nil value and it's likely to be used in a number of places).

My concern is that we have a number of requests for callbacks that are only of use in very specific circumstances (including yours!). Given that we already have a lot of ClientOption settings I'm reluctant to see additional options added for very niche requirements so felt that a catch-all option for notification of status changes might provide the functionality needed without adding too much complexity.