-
Type: Improvement
-
Resolution: Fixed
-
Priority: Unknown
-
Affects Version/s: None
-
Component/s: None
There are some alternative event handler APIs we should consider whenever we do a 3.0 release.
For one proposal:
Currently our API requires that users give us Arc-ed event handlers for command, SDAM, and CMAP events. However, not all event handlers necessarily have any state to persist across them - consider a handler that just prints events or forwards events elsewhere. We might consider a more flexible API that does not require the sender to be Arc-ed and only requires that it is Sync + Send, and then users who actually have state to share within their handler can Arc that inner state instead. This would be more consistent with the APIs for specifying handler types for both the log and tracing crates, which seem like the closest points of comparison:
The Log trait requires Sync + Send:
https://docs.rs/log/latest/log/trait.Log.html
The Subscriber trait itself does not require these but methods for registering a tracing subscriber require that it is is Sync + Send as well:
https://docs.rs/tracing/latest/tracing/trait.Subscriber.html
https://docs.rs/tracing/latest/tracing/subscriber/fn.set_default.html
One reason we did not initially go with this approach was that we did not want to have to make ClientOptions generic over the handler types. That said, we could avoid that here by Box -ing them, or alternatively by having generic methods to set handlers similar to those in tracing and log, e.g. Client.set_command_event_handler.
For another proposal:
We might consider getting rid of the handler traits altogether, and moving to an API where we just publish the events to channels users can listen on. We already have built an API like this atop handlers for testing purposes. It's currently possible for users to "behave badly" in handler implementation (e.g. write blocking code in a handler function, which we will end up invoking in an async context in the driver), and this would prevent that type of issue.