active_delivery icon indicating copy to clipboard operation
active_delivery copied to clipboard

Exception handling for delivery lines

Open synth opened this issue 1 year ago • 1 comments

With respect to our ApplicationDelivery class:

class ApplicationDelivery < ActiveDelivery::Base
  self.abstract_class = true

  register_line :mailer, ActiveDelivery::Lines::Mailer,
                resolver: ->(klass) { klass.name&.gsub(/Delivery$/, 'Notifier')&.safe_constantize }

  register_line :sms, ApplicationLine,
                resolver: ->(klass) { "#{klass.name}::SmsNotifier".safe_constantize }

  register_line :webhook, ApplicationLine,
                resolver: ->(klass) { "#{klass.name}::WebhookNotifier".safe_constantize }

  register_line :slack, ApplicationLine,
                resolver: ->(klass) { "#{klass.name}::SlackNotifier".safe_constantize }
end

Is your feature request related to a problem? Please describe.

We have noticed a situation where an exception is raised in one of the delivery lines, namely the :sms line, and then other lines declared after it are not running. I tried looking for information on the expectation of exceptions in this gem, but couldn't find much. My initial personal expectation is that each line should be robust and an exception in one line shouldn't affect the ability for the others to run. But perhaps that is out of scope for this gem and the behavior is too application specific, not sure.

Describe the solution you'd like

Perhaps, a flag or option to have a consolidated exception handler, so we can generally log and not block additional line processing.

Describe alternatives you've considered

Building exception handling ourselves into each delivery line.

synth avatar Sep 08 '24 01:09 synth

each line should be robust and an exception in one line shouldn't affect the ability for the others to run

Yeah, that's something I considered initially but decided to go with the

that is out of scope for this gem and the behavior is too application specific

The problem is that I want to ensure that developers observe the exceptions, but there was no good one API to rule them all solution at the time (like, logger.error is just sweeping the dirt under rug). In modern Rails, however, we have the Rails.error.report interface which could be used here.

Though I'm still not convinced. In general, I prefer libraries not to make assumptions regarding exceptions and just let them go up the stack. However, we must clearly state in the docs that an exception in one line would interrupt the notification propagation.

palkan avatar Sep 17 '24 00:09 palkan

Hey @synth,

I decided to implement a basic error handling feature. Now you can set self.raise_on_line_error = false in the base delivery class to rescue line delivery errors and report them (either via the Rails error reporter, or Kernel.warn, or your custom logic defined in the #capture_line_error(type, line_error) method).

palkan avatar Aug 20 '25 21:08 palkan

Amazing! This is fantastic and perfect timing for us. We're in-flight on some work to lean more heavily into this gem and are refactoring all of our notifications to use delivery lines so this is definitely a very welcome update, thank you!

synth avatar Aug 22 '25 06:08 synth