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

wrapping the effect handler with a catch block #84

Open
DangerDawson opened this issue Jun 11, 2021 · 5 comments
Open

wrapping the effect handler with a catch block #84

DangerDawson opened this issue Jun 11, 2021 · 5 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@DangerDawson
Copy link

DangerDawson commented Jun 11, 2021

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 DangerDawson added bug Something isn't working help wanted Extra attention is needed labels Jun 11, 2021
@DangerDawson
Copy link
Author

DangerDawson commented Jun 11, 2021

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

@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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants