rebus-org / Rebus.ServiceProvider

:bus: Microsoft Extensions Dependency Injection container adapter for Rebus
https://mookid.dk/category/rebus
Other
67 stars 32 forks source link

Add async UseRebus overload #46

Closed skwasjer closed 3 years ago

skwasjer commented 3 years ago

See #44

Do you prefer UseRebus or UseRebusAsync? Considering Rebus API pattern is not to use suffix I stuck to that.

Please note this is a breaking change for current integrators if they use async/void:

app.ApplicationServices.UseRebus(async bus => await bus.Subscribe<SomeMessage>())

which after updating has to be prefixed with await. Otherwise its a fire and forget task (i.e. potential exceptions are lost).

await app.ApplicationServices.UseRebus(async bus => await bus.Subscribe<SomeMessage>())

Rebus is MIT-licensed. The code submitted in this pull request needs to carry the MIT license too. By leaving this text in, I hereby acknowledge that the code submitted in the pull request has the MIT license and can be merged with the Rebus codebase.

mookid8000 commented 3 years ago

Isn't it kind of odd to have an async method show up on IServiceProvider?

skwasjer commented 3 years ago

It is, somewhat (as you also discussed @sorensenmatias in issue #44). And it is good you challenge this API addition.

The alternative we have been using for over a year now is to register a hosted service that subscribes to topics, with an API smth like:

services
    .AddRebus(....)
    .SubscribeOnStart(s => s // This registers a hosted service that starts the bus and subscribes to the events below
        .To<Message1>()   // Register events to subscribe to.
        .To<Message2>()
    )

Note, this will require depending on Microsoft.Extensions.Hosting and will obviously only work when using icw. HostBuilder/WebHostBuilder.

mookid8000 commented 3 years ago

Since it's very surprising to find an async Use(...) extension method on IServiceProvider, I would definitely prefer for the method to be synchronous from the caller's point of view.

Internally, the method could use Rebus' AsyncHelpers to run the method – it will temporarily replace the scheduler and use the calling thread to pump the task queue until done, thus running any kind of asynchronous stuff on the calling thread only.

Since AsyncHelpers is internal in Rebus, I suggest you simply copy the class to use it. What do you think of that solution?

skwasjer commented 3 years ago

Sure, have a look.

I could not find coverage in main Rebus repo for AsyncHelpers quickly (just browsing the repo, have not checked it out). It is not ideal from a maintenance perspective if the situation props up that the helper changes. Something to also consider in the future :)

mookid8000 commented 3 years ago

Excellent 🙂 it's pushed as Rebus.ServiceProvider 6.3.0

Thanks for your contribution!