dry-rb / dry-effects

Algebraic effects in Ruby
https://dry-rb.org/gems/dry-effects/
MIT License
112 stars 20 forks source link

wrapping the effect handler with a catch block #84

Open DangerDawson opened 3 years ago

DangerDawson commented 3 years ago

Issue when wrapping the effect handler with a catch block

Some libraries / frameworks like warden and hanami, use throw and catch to manage the flow of execution. I am trying to use dry-effects to wrap all the call methods for hanami controller actions, although when a :halt is thrown instead of it being caught by the framework, it is being caught by dry-effects with the following error:

uncaught throw :error (UncaughtThrowError)

which is coming from:

lib/dry/effects/frame.rb:39:inblock in spawn_fiber'`

To Reproduce

class Foo
  include ::Dry::Effects::Handler.Reader(:test)

  def call
    catch(:error) do
      with_test(:foobar) do
        throw :error
      end
    end
    puts 'we should have got here'
  end
end

Foo.new.call

Expected behavior

I would expect dry-effects to ignore the specific throw and bubble it up

DangerDawson commented 3 years ago

Possible workaround:

class Foo
  include ::Dry::Effects::Handler.Reader(:test)

  def call
    catch(:error) do
      unhandled_throw = nil
      with_test(:foobar) do
        throw :error
      rescue UncaughtThrowError => e
         unhandled_throw = [e.tag, e.value]
      end
      throw(*unhandled_throw) if unhandled_throw
    end
    puts 'we should have got here'
  end
end

Foo.new.call

Although it would be better if this was handled inside of dry-effects

DangerDawson commented 3 years ago

Any thoughts on this? as I would rather not use a rescue as it defeats the point of the efficiencies of using control flow over exceptions.

A bit of context here, I am wrapping the hanami-action call method which uses throw(:halt) to handle redirects as well as other things.

flash-gordon commented 3 years ago

I'm not sure if this is "fixable". It's certainly possible to emulate catch/throw with effects and then patch libraries to account for this. The problem here is effects are not compatible with alternative flow control mechanisms, their goal is to unify all possible effects in a predictable way. I'll post examples and patches a bit later.

DangerDawson commented 3 years ago

@flash-gordon

It seems as though a similar problem was fixed in this repo: https://github.com/rmosolgo/graphql-ruby/pull/3333/files

But as you suggest if may not be possible from these discussions:

https://stackoverflow.com/questions/47833760/uncaughtthrowerror-raised-when-throw-is-called-in-a-fiber

and

https://www.reddit.com/r/ruby/comments/l431j4/interesting_throwcatch_behaviour_in_ruby/

I would appreciate any help on this, as I have invested quite heavily into using dry-effects to provide a way to add dynamic scoping to the repository calls in hanami-rb.

DangerDawson commented 3 years ago

I have done some benchmarking, and it is not a disaster in terms of performance:

https://gist.github.com/DangerDawson/9488598302a6d2ff9f2ea45912e5d4d7

Yielded these results:

Rehearsal ------------------------------------------------
Catch/Throw    0.217898   0.000054   0.217952 (  0.218027)
Raise/Rescue   0.738878   0.001477   0.740355 (  0.740536)
Throw/Rescue   0.882375   0.003234   0.885609 (  0.886147)
--------------------------------------- total: 1.843916sec

                   user     system      total        real
Catch/Throw    0.217242   0.000065   0.217307 (  0.217419)
Raise/Rescue   0.719115   0.000175   0.719290 (  0.719359)
Throw/Rescue   0.816662   0.001009   0.817671 (  0.817900)

So the slowdown is just under 4 fold.