eclipse-paho / paho.mqtt.golang

Other
2.77k stars 534 forks source link

Context in Subscribe MessageHandler #609

Closed lpegoraro closed 10 months ago

lpegoraro commented 2 years ago

Hi all, great tool btw, thanks a lot for this!

I am using this Subscribe method here

token := a.client.Subscribe(rpcFromCoreTopic, 1, a.handleGroupRPCFromCore)

And I would like to pass a context inside the handleFunc, because of context cancellation propagation.

And looking here

// MessageHandler is a callback type which can be set to be
// executed upon the arrival of messages published to topics
// to which the client is subscribed.
type MessageHandler func(Client, Message)
// ---------------...
    // Subscribe starts a new subscription. Provide a MessageHandler to be executed when
    // a message is published on the topic provided, or nil for the default handler.
    //
    // If options.OrderMatters is true (the default) then callback must not block or
    // call functions within this package that may block (e.g. Publish) other than in
    // a new go routine.
    // callback must be safe for concurrent use by multiple goroutines.
    Subscribe(topic string, qos byte, callback MessageHandler) Token

we don't have any option to pass along the context, what do you guys think of having this change

type MessageHandlerWithContext func(context.Context, Client, Message)

I did look for some issues related but found none.

MattBrittan commented 2 years ago

This package was written before the Context package was released (back in 2016 with Go 1.7!) so was designed without contexts in mind. While I have considered introducing Context I'm not sure that there are sufficient benefits to justify the change.

pass a context inside the handleFunc,

Where would this context come from and what would it contain? The handler is called from here; it would be possible to pass in a context which gets cancelled if the connection drops (or Disconnect() is called) but I don't really see any real benefit to this (generally you would still want to process the message anyway, and long running handlers are a bad idea).

It might help if you provided a usage example and explain what situations would result in the context being cancelled.

MattBrittan commented 10 months ago

Closing this due to inactivity (the requested info was not provided).