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

Accidental rescuing Interactor::Failure errors in an interactor. #120

Closed
hedgesky opened this issue Dec 20, 2016 · 1 comment
Closed

Accidental rescuing Interactor::Failure errors in an interactor. #120

hedgesky opened this issue Dec 20, 2016 · 1 comment

Comments

@hedgesky
Copy link
Contributor

Currently context.fail! raises an error but there is no mention about that in Readme. Therefore a user could accidentally rescue these errors. Example:

class A
  include Interactor

  def call
    do_some_stuff
  rescue => e
    logger.error("#{e.class}: #{e.message}")
  end

  private

  def do_some_stuff
    context.fail!
  end
end

Here user tries to log unforeseen errors but instead finds such lines:

Interactor::Failure: #<Interactor::Context>

From my point of view, we should at least add information about that in README with possible solution:

def call
  do_some_stuff
rescue Interactor::Failure
  raise
rescue => e
  puts "#{e.class}: #{e.message}"
end

But ideal solution is using throw instead of raise. It would stop execution (like raise does) and wouldn't require additional rescues. I can make a PR if you find this solution is acceptable.

@laserlemon
Copy link
Collaborator

Version 4.0 will use throw and catch rather than raise and rescue for this very reason. My attention is turning to development of 4.0, but I would be happy to merge a readme update with a brief mention of context.fail!'s internal behavior and a warning that using a blanket rescue in your interactor may swallow the failure.

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

No branches or pull requests

2 participants