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

Some fixes for ruby 2.4.1 compatibility #60

Closed
wants to merge 2 commits into from

Conversation

SamMolokanov
Copy link

@SamMolokanov SamMolokanov commented Apr 6, 2017

Hi there!

On ruby-2.4.1 it raises an error:

     NoMethodError:
       undefined method `<<' for {:read_timeout=>60, :continue_timeout=>nil, :debug_output=>nil}:Hash
       Did you mean?  <
     # .rvm/gems/ruby-2.4.1/gems/fakeweb-1.3.0/lib/fake_web/ext/net_http.rb:50:in `request_with_fakeweb'

I made some little fixes in order to use the gem with ruby-2.4.1. May be someone would find it helpful.

# Gemfile

group :test do
  gem "fakeweb", github: "SamMolokanov/fakeweb", branch: "ruby-2-4-1-support"
end

#57

@tetherit
Copy link

Same problem here, thank you for the workaround :) - please merge!

chrisk added a commit that referenced this pull request Jun 13, 2017
@SamMolokanov
Copy link
Author

SamMolokanov commented Jun 19, 2017

Closing this since @chrisk continue working. If anything could be useful from this PR, please let me know. Cheers!

@chrisk
Copy link
Owner

chrisk commented Jun 23, 2017

Hey Sam, thanks for posting this! If it's okay, I'd like to leave it open for a little while longer until I've had time to review everything you did and make sure nothing falls through the cracks... sorry to pollute your PR list. I should have everything reviewed and merged soon.

@chrisk chrisk reopened this Jun 23, 2017
chrisk added a commit that referenced this pull request Jun 26, 2017
Background:

1.  As of Ruby 2.2, calling #close on a socket that's already closed
    no longer raises an exception.

2.  As of Ruby 2.4, Net::HTTP no longer checks #closed? every time
    before calling #close on a socket, since there's no longer any
    danger it will raise. See:

    * r56795 (ruby/ruby@6394b63db9abe34234e7)
    * r56865 (ruby/ruby@f845a9ef76c0195254de)

FakeWeb's internal StubSocket has been short-circuiting those pre-2.4
checks by just always returning true when #closed? is checked,
preventing #close from ever being called.

Now that the checks are gone, StubSocket needs its own #close
method. This fixes a variety of NoMethodErrors on Ruby 2.4, but is
basically backwards-compatible with earlier implementations.

(When running on pre-Ruby 2.2, I suppose StubSocket should raise when
you call #close on an already-closed socket. I'll look into that, but
all our functional tests are passing on earlier Rubies at the moment.)

While we're at it, I changed #closed? to stop just always returning
true, to be closer to how the Ruby stdlib's sockets behave. This doesn't
appear to break anything but I'll revert that part if I find anything in
the relevant Net::HTTP code that conflicts.

Thanks for reporting and submitting patches: @voxik in #59,
@SamMolokanov in #60.
chrisk added a commit that referenced this pull request Jun 26, 2017
Background:

1.  As of Ruby 2.2, calling #close on a socket that's already closed
    no longer raises an exception.

2.  As of Ruby 2.4, Net::HTTP no longer checks #closed? every time
    before calling #close on a socket, since there's no longer any
    danger it will raise. See:

    * r56795 (ruby/ruby@6394b63db9abe34234e7)
    * r56865 (ruby/ruby@f845a9ef76c0195254de)

FakeWeb's internal StubSocket has been short-circuiting those pre-2.4
checks by just always returning true when #closed? is checked,
preventing #close from ever being called.

Now that the checks are gone, StubSocket needs its own #close
method. This fixes a variety of NoMethodErrors on Ruby 2.4, but is
basically backwards-compatible with earlier implementations.

(When running on pre-Ruby 2.2, I suppose StubSocket should raise when
you call #close on an already-closed socket. I'll look into that, but
all our functional tests are passing on earlier Rubies at the moment.)

While we're at it, I changed #closed? to stop just always returning
true, to be closer to how the Ruby stdlib's sockets behave. This
doesn't appear to break anything but I'll revert that part if I find
anything in the relevant Net::HTTP code that conflicts.

Thanks for reporting and submitting patches: @voxik in #59,
@SamMolokanov in #60.

Closes #59.
References #57.
chrisk added a commit that referenced this pull request Jun 26, 2017
Background:

1.  As of Ruby 2.2, calling #close on a socket that's already closed
    no longer raises an exception.

2.  As of Ruby 2.4, Net::HTTP no longer checks #closed? every time
    before calling #close on a socket, since there's no longer any
    danger it will raise. See:

    * r56795 (ruby/ruby@6394b63db9abe34234e7)
    * r56865 (ruby/ruby@f845a9ef76c0195254de)

FakeWeb's internal StubSocket has been short-circuiting those pre-2.4
checks by just always returning true when #closed? is checked,
preventing #close from ever being called.

Now that the checks are gone, StubSocket needs its own #close
method. This fixes a variety of NoMethodErrors on Ruby 2.4, but is
basically backwards-compatible with earlier implementations.

(When running on pre-Ruby 2.2, I suppose StubSocket should raise when
you call #close on an already-closed socket. I'll look into that, but
all our functional tests are passing on earlier Rubies at the moment.)

While we're at it, I changed #closed? to stop just always returning
true, to be closer to how the Ruby stdlib's sockets behave. This
doesn't appear to break anything but I'll revert that part if I find
anything in the relevant Net::HTTP code that conflicts.

Thanks for reporting and submitting patches: @voxik in #59,
@SamMolokanov in #60.

Closes #59.
chrisk added a commit that referenced this pull request Jul 9, 2017
In Ruby 2.4, Net::BufferedIO's method signature changed to:

    def initialize(io, read_timeout: 60, continue_timeout: nil, debug_output: nil)

Before that, it was just:

    def initialize(io)

Our around-advice-style monkey-patch wraps this method, so it needs to pass the
arguments through transparently. Ideally, we could just do this:

    def initialize_with_fakeweb(*args, **opts)

But this causes a syntax error on earlier Rubies. Using an eval like this is the
only way I can think of to work around this.

This fixes exceptions like:

    NoMethodError: undefined method `<<' for {:read_timeout=>60,
      :continue_timeout=>nil, :debug_output=>nil}:Hash

Thanks to @SamMolokanov for reporting in #60.
chrisk added a commit that referenced this pull request Jul 9, 2017
* update-bufferedio-method-signature-for-24-compat:
  Update Net::BufferedIO patch for Ruby 2.4-compat
  Extract-method refactor

Refernces #57 and #60.
@chrisk
Copy link
Owner

chrisk commented Jul 17, 2017

Hey @SamMolokanov, would you mind switching to this repo's master branch and seeing if everything works for your project? I have versions of all these fixes merged now (along with many others), but would like to get some users to test while I update the docs, etc. before releasing.

# Gemfile

gem "fakeweb", github: "chrisk/fakeweb", branch: "master"

@SamMolokanov
Copy link
Author

Hi @chrisk thank you for the update!
Your latest build works perfect 👍

PS. I have added a post-install message for people who use temporary solution.

@SamMolokanov
Copy link
Author

Hi @chrisk! can I close this PR? :)

@chrisk
Copy link
Owner

chrisk commented Jul 19, 2017

Hi Sam, thanks for testing! Good idea about the post-install message. Yes, we can close this now. Thanks for all your help.

@chrisk chrisk closed this Jul 19, 2017
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.

3 participants