Dry::Transaction::InvalidResultError: step must return a Result object

[cristicatalan]

If I have a transaction with the same name as one of its operations I get an error:

Dry::Transaction::InvalidResultError: step +do_stuff+ must return a Result object

The transaction class is this one:

require 'dry_test/container'
require "dry/transaction"

module DryIssue
  module Transactions
    class DoStuff
      include Dry::Transaction(container: DryTest::Container)

      step :do_basic_stuff, with: 'dry_issue.operations.do_basic_stuff'
      step :do_stuff, with: 'dry_issue.operations.do_stuff'
    end
  end
end

Here is a demo repo: https://github.com/cristicatalan/transaction_test

You can see the error in action running bundle exec rake dry_issue:do_stuff

[alexandru-calinoiu]

The strangest think is that if one changes the name of the step, let's say from do_stuff to stuff it will all work magically.

[timriley]

I have to say, this is a really strange situation!

I think the problem is to do with how you've set up that Rakefile.

If I put a byebug at the top of your dry_issue:do_stuff rake task, like this:

  desc 'Task to prove the issue'
  task :do_stuff do
    byebug

And then I list out the private methods of do_stuff (the transaction object):

(byebug) do_stuff.private_methods.grep(/stuff/)
[:do_stuff]

We'll see that it somehow now has a private method called #do_stuff (which is clearly not in the class' source file). It gets weirder:

(byebug) do_stuff.method(:do_stuff).source_location
["/Users/tim/Source/dry-rb/_bugs/transaction_test/dry-test/Rakefile", 54]

This private method is actually the one defined in the Rakefile, just above the task! This one:

  def do_stuff
    DryTest::Container['dry_issue.transactions.do_stuff']
  end

So now, when the dry-transaction code runs as part of initialising this transaction object, when it runs #resolve_operation, it will detect that the :do_stuff step has a corresponding private method with the same name, so it create a method object out of that method and uses that as the step operation, rather than the actual operation object provided by the container! This is what causes your error, since this #do_stuff method is returning another instance of your Transactions::DoStuff transaction, and not the result object that is meant to be returned from your Operations::DoStuff#call.

So the bug here, really, is to do with whatever Rake is doing to get that private method to be put on the transaction class.

You can clear this up, simply, by changing your Rakefile section to this:

namespace :dry_issue do
  desc 'Task to prove the issue'
  task :do_stuff do
    DryTest::Container['dry_issue.transactions.do_stuff']
        .with_step_args(do_stuff: [my_args: 'fancy args'])
        .call('something') do |m|
      m.success { puts 'Success' }
      m.failure { |err| puts err }
    end
  end
end

Running this rake task will now result in "Success" being printed, as you'd expect.

So, short story? Methods in rake tasks: baaaaad 😬

I think this is how that method is getting everywhere:

(byebug) Kernel.private_methods.grep(/stuff/)
[:do_stuff]

(byebug) Object.new.private_methods.grep(/stuff/)
[:do_stuff]

Rake seems to be putting that method on Kernel, somehow, which means it will also get into every object in the current runtime. 😬😬😬

I tried to find out exactly why this was happening, but ran out of time. But I think at the very least this investigation rules out this being a bug in dry-transaction.

[timriley]

Here's a much smaller reproduction of the Rake issue: https://gist.github.com/timriley/e76d99ee8fbc69633482bd0d48667b18

[timriley]

OK, it turns out this is nothing to do with Rake, it's just plain #ruby behaviour.

Define a method in the global namespace, and it goes onto Kernel, and therefore becomes a private method for every object created henceforth.

class MyClass
end

def top_level_method
  "hi"
end

p MyClass.new.private_methods.grep(/top_level/)
# => [:top_level_method]

tl;dr don't define un-scoped methods in Rakefiles! 😬

[cristicatalan]

I understand what you mean.

[cristicatalan]

It is interesting we began to have this problem only after updating the gems. I digged deeper and found out that after updating from dry-transaction 0.10.2 to 0.11 this issue appeared.

[timriley]

You likely updated to a version that looks for instance methods in a different way to the one prior, which is why the problem coincided with the upgrade.

Either way, this is not something we can easily defend against within dry-transaction. The best fix is to make sure you don’t pollute Kernel and don’t name steps in a way that end up connecting them to unwanted instance methods.

On 23 Aug 2018, at 6:05 pm, cristicatalan notifications@github.com wrote:

But we had this issue only after updating the gem dry-transaction.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or mute the thread.