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.
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 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.
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