dry-effects icon indicating copy to clipboard operation
dry-effects copied to clipboard

wrapping the effect handler with a catch block

Open DangerDawson opened this issue 3 years ago • 5 comments

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:in block 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 avatar Jun 11 '21 09:06 DangerDawson

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 avatar Jun 11 '21 10:06 DangerDawson

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.

DangerDawson avatar Jul 14 '21 14:07 DangerDawson

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.

flash-gordon avatar Jul 14 '21 15:07 flash-gordon

@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 avatar Jul 14 '21 15:07 DangerDawson

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.

DangerDawson avatar Jul 14 '21 15:07 DangerDawson