A catch with `Dry::Monads::Do` and transaction safety

Hello,

One thing I’ve run into with Dry::Monads::Do is that there’s a subtle catch in relying on the exceptions thrown for transaction safety. The following two methods are not equivalent:

def safe
  transaction do
    # A failure results in a rollback
    result = yield run_some_action
    Success(result)
  end
end

def unsafe
  transaction do
    # A failure does *not* rollback
    run_some_action
  end
end

In general, you can’t rely on the Monad identity law here, because Do’s bind uses side effects to roll back transactions. This was somewhat surprising to me, although on reflection it makes sense.

I’m curious if this is something people have run into, and if there are any good solutions to prevent this (somewhat innocent-looking) refactoring mistake? I also wonder if the documentation should perhaps point out this gotcha.

I wrote a blog post with a bit more of my thoughts here: dry-rb and lawless monads | mikey.bike

Thanks.
Mikey

1 Like

That’s an interesting finding! I’ve never encountered it, but it’s possible that people have bugs like this in production code. I generally have a different way of writing my code where checks come first and persistence matters later. Nevertheless, I’ll check my code today :slight_smile: It should definitely be in the docs. I wonder if there’s a way to make it safer, though. Probably not, without integration between dry-monads and transaction manager.

It’s pretty novel to encounter someone approaching dry-monads from Haskell, first of all. Usually I get blank stares when I introduce monads.

The mistake here is simply being too accustomed to monads being a language feature.

An Operation object that mixes in Dry::Monads::Do should be thought of as a boundary between exception-based, non-local returns and monad types. You are not following laws, you are translating one set of laws into another.

From the outside, these Operations will follow monad laws. But on the inside, they are adapting exceptions so you need to be thinking at that level. I liken this to the Maybe monad as an alternative to nil: it’s far better to use Maybe instead, but nil still exists. Anything that could receive a nil needs to account for that and translate it.

I have never been comfortable relying on side-effects for transaction rollbacks, but we can make a deliberate choice not to do it that way.

# ActiveRecord
module Transactions
  def transaction(model)
    model.transaction do
      result = yield.to_monad
      result.or { model.connection.transaction_manager.rollback_transaction }
      result
    end
  end
end

# Sequel
module Transactions
  def transaction(db)
    db.transaction do
      result = yield.to_monad
      result.or { db.rollback_on_exit }
      result
    end
  end
end

So with this shim added to your Operation base class, the footgun has been unloaded.

def run(user, title)
  transaction(user) do
    user = yield mark_user_last_posted(user)
    create_post_for_user(user, title)
  end
end

This is certainly a sharp edge that should be emphasized. Perhaps there is opportunity for adapter code that handles specific scenarios like this.

2 Likes

Fun fact, I checked my projects and found only one place where this might be a problem, that piece wasn’t written by me. There must be an unspoken set of rules I follow somehow :smile: