search

petergoldstein / dalli

High performance memcached client for Ruby
MIT License
3.1k stars 450 forks source link

TypeError raised in latest Socksify when initializing Dalli client with version 3.2.7+ #996

Open Capncavedan opened 7 months ago

Capncavedan commented 7 months ago

Dalli 3.2.7 and 3.2.8 cause a TypeError in Socksify. 3.2.6 works OK.

Ruby version 3.0.6.

Reproduction script, passes with socksify 1.7.1 and dalli 3.2.6 installed, fails with dalli from 3.2.7.

require 'dalli'
dc = Dalli::Client.new('localhost:11211')
dc.set('abc', 'required dalli')
puts dc.get('abc')

require 'socksify'
dc = Dalli::Client.new('localhost:11211')
dc.set('xyz', 'required socksify')
puts dc.get('xyz')

Backtrace:

/home/.rvm/gems/ruby-3.0.6/gems/socksify-1.7.1/lib/socksify.rb:178:in `initialize': no implicit conversion of Hash into String (TypeError)
    from /home/.rvm/gems/ruby-3.0.6/gems/socksify-1.7.1/lib/socksify.rb:178:in `initialize'
    from /home/.rvm/gems/ruby-3.0.6/gems/dalli-3.2.7/lib/dalli/socket.rb:93:in `new'
    from /home/.rvm/gems/ruby-3.0.6/gems/dalli-3.2.7/lib/dalli/socket.rb:93:in `open'
    from /home/.rvm/gems/ruby-3.0.6/gems/dalli-3.2.7/lib/dalli/protocol/connection_manager.rb:210:in `memcached_socket'
    from /home/.rvm/gems/ruby-3.0.6/gems/dalli-3.2.7/lib/dalli/protocol/connection_manager.rb:55:in `establish_connection'
    from /home/.rvm/gems/ruby-3.0.6/gems/dalli-3.2.7/lib/dalli/protocol/base.rb:207:in `connect'
    from /home/.rvm/gems/ruby-3.0.6/gems/dalli-3.2.7/lib/dalli/protocol/base.rb:196:in `ensure_connected!'
    from /home/.rvm/gems/ruby-3.0.6/gems/dalli-3.2.7/lib/dalli/protocol/base.rb:58:in `alive?'
    from /home/.rvm/gems/ruby-3.0.6/gems/dalli-3.2.7/lib/dalli/options.rb:24:in `block in alive?'
    from /home/.rvm/gems/ruby-3.0.6/gems/dalli-3.2.7/lib/dalli/options.rb:23:in `synchronize'
    from /home/.rvm/gems/ruby-3.0.6/gems/dalli-3.2.7/lib/dalli/options.rb:23:in `alive?'
    from /home/.rvm/gems/ruby-3.0.6/gems/dalli-3.2.7/lib/dalli/ring.rb:46:in `server_for_key'
    from /home/.rvm/gems/ruby-3.0.6/gems/dalli-3.2.7/lib/dalli/client.rb:425:in `perform'
    from /home/.rvm/gems/ruby-3.0.6/gems/dalli-3.2.7/lib/dalli/client.rb:208:in `set_cas'
    from /home/.rvm/gems/ruby-3.0.6/gems/dalli-3.2.7/lib/dalli/client.rb:201:in `set'
    from ./typeerror.rb:8:in `<main>'
Capncavedan commented 7 months ago

I don't like this approach, too brittle, but maybe somebody has a smarter approach to deciding "is Socksify going to cause a problem"?

Updated conditions used within this method

      def self.create_socket_with_timeout(host, port, options)
        new_takes_kwargs = true if RUBY_VERSION >= '3.0' &&
                                    !::TCPSocket.private_instance_methods.include?(:original_resolv_initialize)
        new_takes_kwargs = false if defined?(Socksify)
        if new_takes_kwargs
          sock = new(host, port, connect_timeout: options[:socket_timeout])
          yield(sock)
        else
          Timeout.timeout(options[:socket_timeout]) do
            sock = new(host, port)
            yield(sock)
          end
        end
      end
stereosupersonic commented 7 months ago

i face the same issue with: Dalli 3.2.8 Ruby version 3.1.4

sambostock commented 6 months ago

I've opened #998 to add tests to check compatibility with other gems without depending on them, and #999 to adjust the approach from #989 to handle compatibility with both resolv-replace and socksify.

tony-pizza commented 4 months ago

Chiming in to just to say we're also facing this issue and it's blocking us from upgrading to the latest Dalli version. The approach in #999 looks good to me.