[Dry::Transaction] Misleading behavior of step adapters


#1

Hi, guys

I’ve found horrible behavior inside dry-transaction gem and can’t find the motivation behind it. Possibly it is a bug. Please come take a look:

As stated in the docs -

Each instance method must accept an input argument and return an output wrapped in a Right (for success) or Left (for failure)

But the input is never meant to be Left or Right. There is an issue with step adapters in my opinion:

require "dry/transaction"
LOG = Logger.new(STDOUT)

class CreateUser
  include Dry::Transaction

  step :process
  try  :validate, catch: StandardError
  step :persist
  
  def process(input)
    LOG.info(input)
    Right('process')
  end
    
  def validate(input)
    LOG.info(input)
    Right('validate')
  end
    
  def persist(input)
    LOG.info(input)
    Right('persist')
  end
end

CreateUser.new.call('start')

I, [2017-08-07T14:43:34.045072 #80621]  INFO -- : start
I, [2017-08-07T14:43:34.045270 #80621]  INFO -- : process
I, [2017-08-07T14:43:34.045344 #80621]  INFO -- : Right("validate")
 => Right("persist")

And what is broken is this that a try step adapter did not extracted value from returned Either instance as step adapter does. That breaks the agility and intercheageability of the operations.

What am i missing? If it is a bug - I can submit a PR with a fix for that.


#2

at validate implementation I expect to receive 'process' string not a Right('process')


#3

According to docs try step adapter wrap output to Right if no errors was raised.

try – the operation may raise an exception in an error case.
This is caught and returned as Left(exception).
The output is otherwise returned as Right(output).

So you get Right(Right(input)) from validate step.

You could find more about try behaviour there:


#4

Every step operation expects to return a Right or Left.

Maybe the docs for the different step operations could help. http://dry-rb.org/gems/dry-transaction/step-adapters/


#5

The issue is not in the return value but with the input itself:

Look with try changed to step adapter

require "dry/transaction"
LOG = Logger.new(STDOUT)


class CreateUser
  include Dry::Transaction

  ...
  step :validate
  ...
  
  ...
end

CreateUser.new.call('start')

# I, [2017-08-07T18:24:46.204654 #81976]  INFO -- : start
# I, [2017-08-07T18:24:46.204779 #81976]  INFO -- : process
# I, [2017-08-07T18:24:46.204855 #81976]  INFO -- : validate
#  => Right("persist")

So the issue is that try adapter does not call .value on input before passing it down. However others do.


#6

If you use a try step, you don’t need to wrap the output in a Right - the step adapter will do this for you.

The map, try, tee steps have present since the beginning of this gem, and were really there to help people add steps for operations that weren’t already returning Either results.

If all your operations are already returning Either results, then all you need to use is the plain step step adapter.

I’m not sure if this is a contrived example you’ve shared, or something based on your actual working code, but in your case it’d probably make sense to rescue from whatever exception you expect right on the operation method/class, e.g.

def validate(input)
  # stuff
rescue MySpecialError => e
  Left(e)
end

Hope this helps!


#7

Thanks @timriley that makes perfect sense. Just wanted to point out that documentations lacks exactly that words.