This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Discussion: Fault tolerance for errors on event handlers #1180
Comments
I would like to discuss here how to apply this or similar design to base gem. |
That's correct and on purpose. It's up to the developers of sync handlers to decide if in case of an error they should swallow it and let the rest of process proceed or propagate and undo whole operation (this might be especially useful in legacy apps where you extract side-effects into handlers but you want to keep the whole process still fully transactional) |
Current status more or less: module RailsEventStore
Event = RubyEventStore::Event
InMemoryRepository = RubyEventStore::InMemoryRepository
EventBroker = RubyEventStore::PubSub::Broker module RailsEventStore
class Client < RubyEventStore::Client
def initialize(repository: RailsEventStore.event_repository,
event_broker: EventBroker.new, RailsEventStore.event_repository = RailsEventStoreActiveRecord::EventRepository.new module RubyEventStore
module PubSub
class Broker
DEFAULT_DISPATCHER = Dispatcher.new
def initialize(dispatcher: DEFAULT_DISPATCHER)
# ...
end
def notify_subscribers(event)
all_subscribers_for(event.class).each do |subscriber|
dispatcher.call(subscriber, event)
end
end module RubyEventStore
module PubSub
class Broker
class Dispatcher
def call(subscriber, event)
ensure_method_defined(subscriber)
subscriber.call(event)
end solution 1How about:
class SwallowExceptionsBroker < Broker
def notify_subscribers(event)
all_subscribers_for(event.class).each do |subscriber|
begin
dispatcher.call(subscriber, event)
rescue => e
AirBrake.notify(e)
end
end
end
end and RailsEventStore.event_repository = RailsEventStoreActiveRecord::EventRepository.new(event_broker: SwallowExceptionsBroker.new) It could be good to extract solution 2Alternatively I would look into whether you can provide your own
It seems that you would need to only inherit and catch exceptions in RailsEventStore.event_repository = RailsEventStoreActiveRecord::EventRepository.new(
event_broker: RailsEventStore::EventBroker.new(MySwallowDispatcher.new)
) What do you think? |
@paneq I like solution 1, but advantage of my design is that I can define error handling strategy per event handler, have them configured depending from requirements, replacing Broker is to much global in his range (have impact on whole application in our case, I'm avoid having multiple instance of event_stores, command_buses etc. Thats why I have one CoreConfiguration in application.rb: config.to_prepare do
Rails.configuration.core = CoreConfiguration.new
end this lets me safely reload whole configuration without risk that handlers were registered multiple times etc... |
What API would you like to see? I think this is a good starting point to imagine a new feature. Then you can think later about how to achieve it. event_store.subscribe(handler, events, on_exception: :notify)
event_store.subscribe(handler, events, on_exception: :raise) Does that look good for sync handlers? What about async handlers? #103 |
what about: event_store.subscribe(handler, events, on_exception: some_lambda_or_proc)
# or
event_store.subscribe(handler, events, on_exception: ErrorHandling.method(:notify)) To keep it open for extension |
In case of implementing it as ActiveJob, we don't need anything more than standard way of dealing with it by providers of implementation. In my case I'm using https://github.com/tawan/active-elastic-job, which will just put failing message in dead letter queue on SQS, with notifications on Newrelic and Airbrake |
I wonder what's the benefit of
over
and doing:
but maybe the convenience is worth it. |
It would be nice if the exception handler was provided with |
would be great to just implement it as decorator, with having full control over how to rescue and recover (local or distributed transactions handling, notifications or even emitting new event) Some people recommending emting Events also on exceptions/errors in system, because everything going then to the source of true as series of events (which brings me to the parent_id: would be great to start linking events together, to improve insights) |
You can put
|
I think it is
|
PR welcomed. |
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
Current implementation of PubSub Broker is not fault tolerant, which means that if one error handler will fail the "broadcasting" will stop processing event by other handlers which were added after failing one.
https://github.com/RailsEventStore/rails_event_store/blob/master/ruby_event_store/lib/ruby_event_store/pub_sub/broker.rb#L26
Because of current implementation of the broker I had to install such error isolation layer:
Configuration:
Usage of Error handling strategy:
Implementation of Error handling strategy :
The text was updated successfully, but these errors were encountered: