Skip to content

Event dispatch has no error isolation: one raising subscriber aborts the rest and destroys the emitter's successful result #50

Description

@fractaledmind

Summary

Bus.emit dispatches invocations with no error handling. A single raising subscriber aborts every remaining subscriber and destroys the emitting service's successful Response.

Observed in servus 0.6.0.

The path

# lib/servus/events/bus.rb:82-88
def emit(event_name, payload)
  ActiveSupport::Notifications.instrument(notification_name(event_name), payload) do
    resolve_invocations(event_name, payload)
      .uniq(&:key)
      .each(&:execute)
  end
end

# lib/servus/events/invocation.rb:64-70
def execute
  if options[:async]
    service.call_async(**params, **async_options)
  else
    service.call(**params)
  end
end

Neither Bus.emit, Invocation#execute, nor Emitter#emit_events_for contains a rescue or ensure. Invocation defines only initialize, execute, key, and async_options.

Consequences

1. Later subscribers are silently skipped. .each(&:execute) aborts at the first raise. Invocations contributed by later routers never run. Already-executed invocations are not compensated. There is no record of the partial dispatch.

2. The emitting service's successful result is destroyed. after_call runs before the result is returned:

# lib/servus/base.rb:348-354
def after_call(result, instance)
  Validator.validate_result!(self, result)
  Emitter.emit_result_events!(instance, result)
end

and Base.call only logs and re-raises:

# lib/servus/base.rb:251-257
rescue StandardError => e
  Logger.log_exception(self, e)
  raise e

So a service that completed its work and returned success still raises to its caller because an unrelated downstream subscriber failed. The Response is lost. If a transaction is open around Service.call, it rolls back — undoing work that succeeded.

3. Enqueue failures have the same blast radius. call_async converts and re-raises rather than isolating:

# lib/servus/extensions/async/call.rb:84-85
rescue StandardError => e
  raise Errors::JobEnqueueError, "Failed to enqueue async job for #{self}: #{e.message}"
end

A transient queue-backend blip during enqueue therefore fails the emitting service, even though async: true signals that the caller does not depend on the handler.

4. With enqueue_after_transaction_commit, earlier async invocations are silently discarded. When that setting is enabled and the emit happens inside a transaction, async invocations dispatched earlier in the same emit are buffered until commit. A later subscriber's raise rolls the transaction back and takes those buffered enqueues with it — so handlers that "succeeded" never run, with nothing recorded.

Reproduction

class ThingHappenedEvent < Servus::Event
  schema payload: { type: 'object', properties: { id: { type: 'string' } } }

  invoke RaisesService                      # sync, raises
  invoke NeverRunsService, async: true      # never enqueued
end

# A service that emits it:
result = DoWork::Service.call(...)  # completes successfully, returns success internally
# => raises RaisesService's error; `result` is never assigned;
#    NeverRunsService never runs; an enclosing transaction rolls back.

Note on intent

README.md documents this for invoke blocks: "They run synchronously at emission time as a unit: if any block raises, the entire emission fails and no handlers are enqueued." That is a reasonable contract for payload-mapping blocks, which are pure and author-controlled.

It is a much stronger and more surprising contract when extended to handler bodies, which do arbitrary work. The current implementation does not distinguish the two.

Suggested direction

Isolate each invocation, and make the policy explicit rather than emergent:

  1. Wrap Invocation#execute so a raising handler is reported (e.g. Rails.error.report) and does not abort siblings.
  2. Distinguish mapper/condition errors (fail the emit — the payload is wrong) from handler errors (isolate — the fact already happened).
  3. Consider an opt-in strict mode for callers that genuinely want emit failures to surface.

Happy to open a PR if the direction is agreeable.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions