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.

Sign up for GitHub

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

Jump to bottom

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

Open
Capncavedan opened this issue Feb 14, 2024 · 4 comments · May be fixed by #999
Open

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

Capncavedan opened this issue Feb 14, 2024 · 4 comments · May be fixed by #999

Comments

@Capncavedan
Copy link

Capncavedan commented Feb 14, 2024 •
edited
Loading

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

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

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

@sambostock sambostock linked a pull request Mar 8, 2024 that will close this issue
Draft
1 task
@sambostock
Copy link
Contributor

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.

@javierm javierm mentioned this issue Mar 19, 2024
Merged
@tony-pizza
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix socksify incompatibility sambostock/dalli
4 participants
@stereosupersonic @tony-pizza @Capncavedan @sambostock