Skip to content

Conversation

@paccator
Copy link

I was getting this error when connection can't be established:

NoMethodError: undefined method 'empty?' for nil
from .../lib/socksify/socksproxyable.rb:107:in 'Socksproxyable::InstanceMethodsConnect#socks_receive_reply'

I think it's related to a change in ruby 3.3:

BasicSocket#recv and BasicSocket#recv_nonblock returns nil instead of an empty string on closed connections.

https://rubyreferences.github.io/rubychanges/3.3.html#standard-library
https://bugs.ruby-lang.org/issues/19012


So here I propose to make sure we operate on a string, not nil.

In few places we could also just check for #nil? or empty? but #to_s in happy path is basically a noop.

Not much to add or change in tests.

Since ruby 3.3 BasicSocket#recv returns nil instead of an empty string on closed connections

https://rubyreferences.github.io/rubychanges/3.3.html#standard-library

https://bugs.ruby-lang.org/issues/19012
@nix-ci-app
Copy link

nix-ci-app bot commented Feb 11, 2025

NixCI is ready to run on this PR.

@astro
Copy link
Owner

astro commented Feb 11, 2025

Personal opinion: I wouldn't merge this because I like the semantics of "" meaning no more data and nil signifying a closed read half.

However, I've passed maintainership to others a long time ago and will accept their decision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants