Skip to content

Instantly share code, notes, and snippets.

@waiting-for-dev
Created November 6, 2023 10:47
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save waiting-for-dev/7615ae577807e3c3b990cd8c53670b2a to your computer and use it in GitHub Desktop.
Save waiting-for-dev/7615ae577807e3c3b990cd8c53670b2a to your computer and use it in GitHub Desktop.

Unhappy path

Before delving into how we support database (DB) transactions, let's first consider how we can handle operations that only pertain to the unhappy path. This consideration is important because, as we will see, it will become relevant when we reach the main topic.

Most of the time, the unhappy path is something managed by the caller (e.g., a controller rendering an error in case of failure). However, there are situations where it makes sense to encapsulate part of the unhappy path within the operations class. For instance, you might want to log the failure somewhere.

When relying on the vanilla #steps method, the implementation is straightforward:

class CreateUser < Dry::Operation
  def call(input)
    steps do
      attrs = step validate(input)
      user = step persist(attrs)
      assign_initial_roles(user)
      step send_welcome_email(user)
      user
    end.tap do |result|
      log_failure(result) if result.failure?
    end
  end
end

However, it is beneficial to automatically prepend the #steps method in #call for a couple of reasons:

  • It reduces boilerplate.
  • It consolidates the interface of the operation, ensuring it always returns a result object.

This leads us to a single option: calling a hook for the failure case:

class CreateUser < Dry::Operation
  def call(input)
    attrs = step validate(input)
    user = step persist(attrs)
    assign_initial_roles(user)
    step send_welcome_email(user)
    user
  end
  
  private
  
  def on_failure(user)
    log_failure(user)
  end
end

Instead of allowing the registration of multiple hooks, it is better to allow a single one where users can dispatch to other methods if needed. This approach allows us to skip dealing with hook ordering and makes the flow more linear.

There is no need, at least for now, to make the hook method name configurable; on_failure is sufficient.

It's worth noting that we now allow multiple methods to be prepended, as in operate_on :call, :run. Therefore, we need a way to specify which hook to call for a given prepended method. We can achieve this by providing a second argument to on_failure when the method is defined with arity 2:

def on_failure(result, method)
  case method
  when :call
    do something()
  when :run
    do something_else()
  end
end

Database Transactions

Leaving aside the interface for now, we have two architectural options:

  1. Wrap the entire #steps call in a transaction:
class CreateUser < Dry::Operation
  use_db_transaction
  
  def call(input)
    attrs = step validate(input)
    user = step persist(attrs)
    assign_initial_roles(user)
    step send_welcome_email(user)
    user
  end
end

Benefits:

  • It supports composing operation classes within a single transaction: CreateUser.new >> CreatePost.new

Drawbacks:

  • It wraps potentially expensive operations in a transaction, such as send_welcome_email in the example.
  • It is not optimized, though not critical, to wrap validate in a transaction.

We find the drawbacks to be unacceptable. If we were to support this option, we would need to use hooks for the setup and success cases:

class CreateUser < Dry::Operation
  use_db_transaction
  
  def call(input)
    user = step persist(attrs)
    assign_initial_roles(user)
    user
  end
  
  private
  
  def setup(input)
    step validate(input)
  end
  
  def on success(user)
    step send_welcome_email(user)
  end
end

In this case, the introduced indirection is also considered unacceptable. While we need to support a hook for the on_failure scenario, dry-operation should prioritize readability when focusing on the happy path.

  1. Explicitly wrap the steps that need to run in a transaction:
class CreateUser < Dry::Operation
  use_db_transaction
  
  def call(input)
    attrs = step validate(input)
    transaction do
      user = step persist(attrs)
      assign_initial_roles(user)
    end
    step send_welcome_email(user)
    user
  end
end

Benefits:

  • It is explicit.
  • It enhances readability.

Drawbacks:

  • It requires manual setup.
  • It makes it impossible to compose operation classes within a single transaction.

In this case, the drawbacks are considered acceptable. There is no way to completely conceal the fact that we are dealing with a database transaction, and developers need to consider it. Furthermore, one of the key concepts of dry-operation is the decoupling of individual operations. Therefore, we should encourage the composition of operations rather than groups of operations in the documentation.

Interface

A Dry::Operation.db_adapter method could be sufficient to configure how Dry::Operation#transaction works.

We can think of three ORM-style libraries we want to support: ROM, Sequel, and ActiveRecord. Different libraries might require different options, and we can use different option names in any case. For example:

class CreateUser < Dry::Operation
  db_adapter :rom, container: Deps[:rom], gateway: :default
  
  # ...
end

Plan of Action

  1. Support the #on_failure hook.
  2. Support the #transaction method through .db_adapter.
  3. Support ROM
  4. Support AR
  5. Support Sequel
@jodosha
Copy link

jodosha commented Nov 6, 2023

@waiting-for-dev Thanks for putting together those thoughts.


Prepend steps

However, it is beneficial to automatically prepend the #steps method in #call [...]

Agreed, as it will reduce the boilerplate.


on_failure Hook

This is not a good idea. IMO, each method should handle the potential errors locally.

Let me expand your example:

class CreateUser < Dry::Operation
  def call(input)
    attrs = step validate(input)
    user = step persist(attrs)
    assign_initial_roles(user)
    step send_welcome_email(user)
    user
  end
  
  private
  
  def on_failure(result, method_name)
    case [method_name, result]
    when [:validate, _]
      # ...
    when [:persist, Db::NotUniqueError]
      # ...
    when [:persist, Db::TimeoutError]
      # ...
    when [:persist, _]
      # ...
    when [:send_welcome_email, _]
      # ...
    end
  end
end

๐Ÿ‘Ž There is a loss in code locality. on_failure will keep growing and knowing about other methods implementations.

๐Ÿ‘‰ Can't we leave the single methods to handle the exceptions and turn them into results?

Example:

def persist(attrs)
  repo.create(attrs)
rescue Db::Error => exception
  # ... return a failing result
end

Database Transactions

Do we need explicit support? Can the following scenario work?

module TestApp
  class Operation < Dry::Operation
    include Deps[:database]

    private

    def transaction(&blk)
      # This implementation changes according to the used persistence layer (ROM, Sequel, etc..)
      database.transaction(&blk)
    end
  end
end
module TestApp
  module Operations
    class CreateUser < Operation
      def call(input)
        attrs = step validate(input)
        transaction do
          user = step persist(attrs)
          assign_initial_roles(user)
        end
        step send_welcome_email(user)
        user
      end
    end
  end
end

๐Ÿ‘‰ In this way, we save a lot of work, not just supporting the several db adapters but also keeping the state in a DSL, transmitting that state to the subclasses, and then resetting it when the app reloads.

@waiting-for-dev
Copy link
Author

waiting-for-dev commented Nov 6, 2023

Many thanks for your feedback, @jodosha! ๐Ÿ™Œ

on_failure Hook

Sorry, I could have been clearer.

each method should handle the potential errors locally.

Definitely, and that's already the case. Each step method (e.g., validate, persist, etc.) must return a Dry::Monads::Result, either a Success or a Failure, doing by itself any required error handling resulting from its body statements. So, yeah, #persist would look like:

def persist(attrs)
  repo.create(attrs)
rescue Db::Error => exception
  # ... return a failing result
end

What #on_failure would receive as an argument is the first Dry::Monads::Result::Failure returned from any of the individual steps (which short-circuits the rest of the entire #call definition). An actual implementation could look like:

  def on_failure(error)
    case error
    when [:validation_error, errors] # `validate` returning Failure[:validation_error, errors]
      # ...
    when [:persist_error, user] # `persist` returning Failure[:persist_error, User]
      # ...
    end
  end
end

Does it make sense? We can handle the happy path within #call. However, the unhappy path needs to be dealt with elsewhere.

When I talked about a second argument received by #on_failure I was talking about the surrounding method from which #on_failure is being called. That will be #call 99% of the time, but we're supporting defining multiple flows in a single instance. We can as well change it and support only one flow per instance, although I think the former can be helpful for debugging.

Database Transactions

Do we need explicit support?

I'm afraid we need it. The #transaction method wouldn't be a raw call to, e.g., row.transaction. It needs to wrap the call in order to raise t.rollback! in case any of the steps executed in the block returns a Failure. A realistic implementation for ROM could look like this:

def transaction(&block)
  result = nil
  rom.gateways[gateway].transaction do |t|
    result = block.()
    raise t.rollback! unless result.success?
  end
  result
end

Please, let me know if everything is clear now ๐Ÿ™‚

@timriley
Copy link

timriley commented Nov 20, 2023

A few thoughts here. I won't be able to go into perfect detail here, but I figure it's better to get something shared now than keep you waiting any longer, Marc โ€” sorry about that!

Setup of DB adapters

This feels ungainly to me, particularly the part where we're having to pass in a dependency (the rom container) at the class-level:

db_adapter :rom, container: Deps[:rom], gateway: :default

(Minor note: Deps[:rom] won't do what you want here; you probably want something like AppOrSlice["persistence.rom"] (with the exact component name to be determined as we go about building Hanami 2.2.)

With Hanami we're encouraging users to provide dependencies at the instance level, and this is the opposite of that.

So I wonder if there's some way to allow these adapters to work with instance-level deps...

Could it be stripped back such that it's more like this:

db_adapter :rom

This would basically be a glorified include of a module (perhaps we could actually just allow it to be an include as our first iteration?) that adds code expecting a rom-like object to be available as an instance method, and adds methods that use it to expose a nice transaction do interface to the users writing these operation classes.

Then, in Hanami apps we could add some separate code that makes sure this rom object is provided automatically as an instance-level dependency.

FWIW, this arrangement gets us closer to the kind of idea that @jodosha pitched in his response above, while making sure we still have dry-operation catch the transaction failures and raise errors as required.

To sum it up: I'd like our DB adapters at their core not to manage state: they should simply add the behaviour that expects the relevant database connection objects to be there, and then use them as appropriate. The job of providing those database connection objects is then a layer above these DB adapters.

In terms of that extra layer: to make dry-operation easy-to-use in a wider range of situations, we may indeed want to bundle some code that finds and loads those connection objects, and we might end up adding hooks to enable that as part of that single class-level API, but in putting this together, we should make sure these two functions are cleanly separated internally, and that it's possible to exercise one without the other, for the case you want to take greater control of providing the database connection object (like we would do in Hanami apps).

In fact, when you boil it down to this being a "module that exposes useful API to users that fits with dry-operation's expectations for failure handling", there's nothing in here that's intrinsically connected to "databases". So I'd encourage us to think about naming this feature so that it could be used for a wider range of use cases. Adapters? Extensions?

Failure hook

I'm fine with the idea of us having this built into dry-operation. Like you outline, this is a necessary hook for us to provide given that our steps do behaviour will be prepended over the user's entry point method.

Like @jodosha says, I think we should encourage users to localise their failure handling as much as possible (i.e. directly in the step methods wherever possible, and in the case of result-returning methods from injected dependencies, by wrapping the call to that dependency in another local method, which can catch failures and adjust as required).

But in the case where these direct approaches cannot be taken, the user has on_failure as a final hook to inject their customisations before control is returned to the caller.

Some thoughts:

  • In the case where the user has a single steps method, they should be able to define their method like this: def on_failure(value) โ€” I think this is what you're suggesting in your initial proposal, but I wanted to confirm just in case :)
  • Another option could be to have def on_<method_name>_failure, e.g. on_call_failure and on_run_failure in the case of classes where both call and run are both declared as steps methods. But I think this would make this feature a lot harder to communicate and document to users, so having the single well-known name makes sense to me.

While I was here, I realised that one thing we're losing from dry-transaction is having a standard failure structure that's exposed to the user. In dry-transaction provided a "step matcher" API when calling the transaction and yielding a block:

create_user.call(name: "Jane", email: "jane@doe.com") do |m|
  m.success do |user|
    puts "Created user for #{user.name}!"
  end

  m.failure :validate do |validation|
    # Runs only when the transaction fails on the :validate step
    puts "Please provide a valid user."
  end

  m.failure do |error|
    # Runs for any other failure
    puts "Couldnโ€™t create this user."
  end
end

I'm not proposing we do this for dry-operation, but rather just pointing out that we've lost this kind of information, because our dry-operation steps don't have names, whereas with dry-transaction's class-level DSL, we gave every step a name. I do wonder if we might give ourselves this ability as an opt-in addition somehow...

  def call(input)
    attrs = step :validate, validate(input)
    user = step :persist, persist(attrs)
    assign_initial_roles(user)
    step :send_welcome_email, send_welcome_email(user)
    user
  end

Maybe? Or some variation on this, possibly?

attrs = step validate(input), as: :validate

Then we could not only pass the failure value to on_failure, but also the name (or possibly full set of kwargs, in the case above?) of the failed step, which would give the code in the on_failure the ability to do even more expressive things, like providing a more structured failure:

def on_failure(value, step_name)
  Failure[step_name, value]
end

No matter what, I want to make sure we still support the most minimal usage, so just step validate(input), but this discussion on the failure hook does make me wonder for our possibilities for "progressive enhancement" here. ๐Ÿค”

@waiting-for-dev
Copy link
Author

With Hanami we're encouraging users to provide dependencies at the instance level, and this is the opposite of that.

Thanks for pointing that out, as it's something that was also bugging me in the back of my mind. The gained convenience blindfolded me, but it's completely true that was an architectural flaw.

I'd like our DB adapters at their core not to manage state: they should simply add the behaviour that expects the relevant database connection objects to be there, and then use them as appropriate. The job of providing those database connection objects is then a layer above these DB adapters.

I like that design. Let's investigate how to make it as user-friendly as possible.

In fact, when you boil it down to this being a "module that exposes useful API to users that fits with dry-operation's expectations for failure handling", there's nothing in here that's intrinsically connected to "databases". So I'd encourage us to think about naming this feature so that it could be used for a wider range of use cases. Adapters? Extensions?

I agree. In fact, I called those extensions in kwork. Maybe just using include as you suggested saves us from coining an abstract name for now.

In the case where the user has a single steps method, they should be able to define their method like this: def on_failure(value) โ€” I think this is what you're suggesting in your initial proposal, but I wanted to confirm just in case :)

That's it ๐Ÿ™‚

Another option could be to have def on_<method_name>_failure, e.g. on_call_failure and on_run_failure in the case of classes where both call and run are both declared as steps methods. But I think this would make this feature a lot harder to communicate and document to users, so having the single well-known name makes sense to me.

I also thought about that option. Besides communication, it could also be the source of slippery bugs (like renaming call to run and forgetting about a #on_call_failure defined on a parent class), or force too much boilerplate (when you want to do the same failure handling for all the flows defined in a single class).

While I was here, I realised that one thing we're losing from dry-transaction is having a standard failure structure that's exposed to the user. In dry-transaction provided a "step matcher" API when calling the transaction and yielding a block:

Part of it is now easily doable thanks to Ruby pattern matching and I think it's better not to build extra API on top of it:

create_user.call(name: "Jane", email: "jane@doe.com").tap do |result|
  case result
  when Success[user]
    puts "Created user for #{user.name}!"
  when Failure[...]
    #...
  end
end

because our dry-operation steps don't have names, whereas with dry-transaction's class-level DSL, we gave every step a name. I do wonder if we might give ourselves this ability as an opt-in addition somehow...

But that's completely true, and maybe that could also be helpful for profiling. One thing I don't like is that would work against our desire to push for locally managing errors (operation encapsulation), but it's worth giving more thoughts to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment