dry-rb / dry-effects Goto Github PK

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

Describe the bug

When using dry-effects (specifically something like include Dry::Effects::Handler::State(:counter), inside a provide(..) { ... } block, any ActiveSupport::TaggedLogging tags may be lost.

A bug report to Rails was opened by my colleague:

• rails/rails#37535

This issue has been opened here to draw the issue to your attention only. I don't know if it's appropriate to workaround AS here, or if some concession can be made to make this more reliable.

At least, maybe a documentation entry could be added to highlight the incompatibility.

To Reproduce

A simple example that simulates what's going on inside AS::TaggedLogging looks like this:

Thread.current[:fiber_local_var] = 'Result'
Fiber.new do
  puts Thread.current[:fiber_local_var]
end.resume
# => <Empty output>

Thread.current.thread_variable_set(:thread_local_var, 'Result')
Fiber.new do
  puts Thread.current.thread_variable_get(:thread_local_var)
end.resume
# => "Result"

In practice however trying to do something like this would/should cause the issue:

logger = ActiveSupport::TaggedLogging.new(Logger.new(STDOUT))
include Dry::Effects::Handler.Resolve
include Dry::Effects.Resolve(:anything)
logger.tagged("A") do
  logger.info "before provide"
  provide(anything: :foo) { anything; logger.info "in provide" }
  logger.info "after provide"
end

Expected behavior

The code above should print:

[A] before provide
[A] in provide
[A] after provide

but it actually prints:

[A] before provide
in provide
[A] after provide

Your environment

$ ruby --version
ruby 2.6.5p114 (2019-10-01 revision 67812) [x86_64-linux]
$ bin/rails version
Rails 6.0.2.1
$ uname -r
5.3.16-300.fc31.x86_64

• Affects my production application: No (yes, but we around by using https://github.com/steveklabnik/request_store as a "replacemnt" for dry-effects DI)

Describe the bug

Using state effect resets all Thread.current variables.

To Reproduce

include Dry::Effects::Handler.State(:updated_attributes)
include Dry::Effects.State(:updated_attributes)

def test
      puts "outside: #{Thread.current.keys}"
      clients_changes, result = with_updated_attributes(Hash.new { |hash, key| hash[key] = [] }) do
        puts "inside: #{Thread.current.keys}"
      end
end
Thread.current[:foo] = 'bar'
test 

Oututs:

outside: [:foo, :dry_effects_stack]
inside: [:dry_effects_stack]

Expected behavior

As you can see variables set outside of the with_updated_attributes block are not visible inside the block.
I expect the following output:

outside: [:foo, :dry_effects_stack]
inside: [:foo, :dry_effects_stack]

My environment

• Affects my production application: NO
• Ruby version: "2.6.5"
• OS: MacOS
• Dry Effects version: 0.1.5

Describe the bug

Code wrapped by effect handler is not analyzed by profilers (such as New Relic and Skylight) properly (e.g. no sql queries data when wrapped in rails controller and complete mess when wrapped with middleware)

To Reproduce

Wrap code by effect handler and introduce some profiler in the app.

Expected behavior

Code is profiled as usual.

Your environment

• Affects my production application: YES
• Ruby version: 2.7.1
• OS: .Ubuntu 20.04

I'm not sure whether it's new relic/skylight problem or dry's, but hope to get some hint at least.

The workflow sync_configs.yml is referencing action actions/checkout using references v1. However this reference is missing the commit a6747255bd19d7a757dbdda8c654a9f84db19839 which may contain fix to the some vulnerability.
The vulnerability fix that is missing by actions version could be related to:
(1) CVE fix
(2) upgrade of vulnerable dependency
(3) fix to secret leak and others.
Please consider to update the reference to the action.

Describe the bug

Executing the code from the Basic Usage of the Interrupt guide does not yield the expected result.

To Reproduce

Execute the code from the example:

require 'dry/effects'

class RunDivision
  include Dry::Effects::Handler.Interrupt(:division_by_zero, as: :catch_zero_division)

  def call
    success, answer = catch_zero_division do
      yield
    end

    if success
      answer
    else
      :error
    end
  end
end

class Divide
  include Dry::Effects.Interrupt(:division_by_zero)

  def call(dividend, divisor)
    if divisor.zero?
      division_by_zero
    else
      dividend / divisor
    end
  end
end

run = RunDivision.new
divide = Divide.new

app = -> a, b { run.() { divide.(a, b) } }

Expected behavior

Expected:

app.(10, 2) # => 5
app.(1, 0) # => :error

Got:

app.(10, 2) # => :error
app.(1, 0) # => nil

Your environment

• Affects my production application: NO
• Ruby version: 2.6.5
• Gem Version: 0.1.1
• OS: macOS 10.15.1

I'd look into it myself, but won't have time today.

Hey, I was browsing through the code to understand the goal of this lib and I thought I would let you know :)