Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.8.2] valid? returns true if block not called and error added in execute around callback #486

Closed
jonian opened this issue Aug 6, 2020 · 1 comment · Fixed by #487
Closed

Comments

@jonian
Copy link
Contributor

jonian commented Aug 6, 2020

Hi @AaronLasseigne, thanks for this gem!

In v3.8.2, valid? returns true if block is not called and error added in execute around callback. This bug also breaks active_interaction-extras halt! method, as reported in antulik/active_interaction-extras#3.

The issue is on https://github.com/AaronLasseigne/active_interaction/blob/master/lib/active_interaction/concerns/runnable.rb#L74 and can be fixed by replacing the run method with:

module ActiveInteraction
  module Runnable
    def run
      return self.result = nil unless valid?

      self.result = run_callbacks(:execute) do
        execute
      rescue Interrupt => interrupt
        errors.backtrace = interrupt.errors.backtrace || interrupt.backtrace
        errors.merge!(interrupt.errors)
      end
    end
  end
end
@jonian
Copy link
Contributor Author

jonian commented Aug 8, 2020

After some more testing, I can confirm that the issue occurs only in the execute around callback if the block is not called. The interaction below replicates the issue.

class TestInteraction < ActiveInteraction::Base
  set_callback :execute, :around, -> { errors.add(:base, 'invalid') }

  def execute
    true
  end
end

# outcome = TestInteraction.run
# outcome.valid?
# => true

Another failing example (this is how active_interaction-extras implements halt! method):

class TestInteraction < ActiveInteraction::Base
  set_callback :execute, :around, lambda { |_interaction, block|
    catch :strict_error do
      block.call
    end
  }

  def execute
    errors.add(:base, 'invalid')
    throw :strict_error, errors
  end
end

# outcome = TestInteraction.run
# outcome.valid?
# => true

@AaronLasseigne sorry for not testing properly when I first reported the issue and the poor/wrong title and description.

@jonian jonian changed the title [3.8.2] Errors added in callbacks are lost on execute [3.8.2] valid? returns true if block not called and error added in execute around callback Aug 8, 2020
AaronLasseigne added a commit that referenced this issue Aug 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant