Run before_drip before running the action

[jon-sully]

Curious for your thoughts on this, Josh!

We've had a few instances of bugs popping up in our primary production app that ultimately came from a cognitive mismatch between the before_drip callback / abort and when the markup of the email gets rendered.

(library metaphor) It feels intuitive to write an abort callback into a before_drip hook that says "hey, before_drip, if the book is already checked back in, cancel this drip sequence!" and then when writing the markup for that mailer action, to assume that the book must still be checked out, so therefore writing something like @book.check_in_date.strftime() should be fine... but it's a nefarious bug!

Since the mailer markup is painted before the before_drip callback actually runs, the @book was already checked back in (making @book.check_in_date = nil). So when the markup calls for @book.check_in_date.strftime(), it'll actually be an error of "you can't call strftime() on nil"... even though the drip would've aborted if it had made it past the markup rendering stage (since the before_drip would've seen the book is actually already checked back in and :aborted)

---

At the technical level this occurs mostly in Caffeinate::Dripper::Delivery.deliver!:

        def deliver!(mailing)
          message = if mailing.drip.parameterized?
                      mailing.mailer_class.constantize.with(mailing: mailing).send(mailing.mailer_action)
                    else
                      mailing.mailer_class.constantize.send(mailing.mailer_action, mailing)
                    end

          message.caffeinate_mailing = mailing
          if ::Caffeinate.config.deliver_later?
            message.deliver_later
          else
            message.deliver
          end

        end

Wherein mailing.mailer_class.constantize.with(mailing: mailing).send(mailing.mailer_action) is called, which instructs ActionMailer to generate the full email and returns the Message wrapper, then message.deliver is called which would ordinarily send that email, except our Caffeinate::ActionMailer::Interceptor:

      def self.delivering_email(message)
        mailing = message.caffeinate_mailing
        return unless mailing

        mailing.caffeinate_campaign.to_dripper.run_callbacks(:before_send, mailing, message)
        drip = mailing.drip
        message.perform_deliveries = drip.enabled?(mailing)
      end

...finally gets involved at the last second to set perform_deliveries = drip.enabled?(mailing) — and since enabled? basically just runs the before_drip callback, all that means that the mailer markup is generated before the before_drip callback runs.

None of that is wrong from a technical standpoint by any means, I mostly want to raise this issue because of the cognitive distortion it leads the developer into: "if I guard for it in a before_drip, it's safe to use in the mailer", but that's not actually true. Maybe we currently have more of a "if I guard for it in a before_drip, I can trust it won't send... but I can't be sure it's safe to use in the mailer, still"

---

Not totally sure where to go with this! Wonder if we might move (in time) the before_drip callback to inside deliver!? Or maybe we add another callback that's like before_deliver? Mostly just want to find some way to actually guarantee the perceived guarantee described above.

WDYT??