WebMock integration is broken

[ixti]

When WebMock used (particulary VCR), first, not yet, recorded request fails with:

HTTP::StateError: body has already been consumed

[tarcieri]

I assume you fixed this @ixti, because I have been using this gem with Webmock without issue for quite some time.

[ixti]

Well I didn't. But after thinking sometime how problem could have disappeared, I realized where it was happening. And it turns out that it was you who fixed the issue... By dropping out caching...

[ixti]

In other words. This should be considered as fixed/closed, yes. :D

[grosser]

still seeing this with http (2.0.3) + webmock (2.1.0) ... removing webmock makes the tests work ... (it is in allow_web_connect mode for this set of tests)

[grosser]

tried simply disabling that check and it did not blow up, but no longer had responses ...

HTTP::Response::Body.class_eval do
  undef stream!
  def stream!
    @streaming = true
  end
end

solved by using

before { WebMock.disable! } # cannot use allow_web_connect! see https://github.com/httprb/http/issues/212

[tarcieri]

@grosser do you have a minimal repro? I'm not encountering this problem with the same versions in my projects that use webmock with http.rb.

[grosser]

here we go :D

... not sure if that helps ... replacing with just a Http.get did not trigger it ...

# Gemfile
source 'https://rubygems.org'

ruby File.read('.ruby-version').strip

gem 'kubeclient', git: 'https://github.com/grosser/kubeclient.git', branch: 'grosser/httpv1'
gem 'rack'
gem 'webmock'

# code
require 'bundler/setup'
require 'minitest/autorun'
require 'kubeclient'
require 'rack'
require 'open-uri'
require 'json'
require 'webmock/minitest'

# frozen_string_literal: true
class FakeServer
  def self.open(port, replies)
    server = new(port, replies)
    server.boot
    yield server
  ensure
    server.shutdown
  end

  def initialize(port, replies)
    @port = port
    @replies = replies
  end

  def boot
    Thread.new do
      Rack::Handler::WEBrick.run(
        self,
        Port: @port,
        Logger: WEBrick::Log.new("/dev/null"),
        AccessLog: []
      ) { |s| @server = s }
    end
  end

  def wait
    Timeout.timeout(10) do
      loop do
        begin
          socket = TCPSocket.new('localhost', @port)
          socket.close if socket
          sleep 0.1 if ENV['CI']
          return
        rescue Errno::ECONNREFUSED
          nil
        end
      end
    end
  end

  def call(env)
    path = env.fetch("PATH_INFO")
    code, reply = @replies[path]
    unless code
      puts "ERROR: Missing reply for path #{path}" # kubeclient does not show current url when failing
      raise
    end
    # adding \n to simulate the stream ... maybe array here ?
    reply = [reply] unless reply.is_a?(Array)
    [code, {}, reply.map { |r| r.to_json << "\n" }]
  end

  def shutdown
    @server.shutdown if @server
  end
end

describe 'foobar' do
  describe "legacy tests, do not add here" do
    before { WebMock.allow_net_connect! } # cannot use allow_web_connect! see https://github.com/httprb/http/issues/212

    it "watches container lifecycle" do
      FakeServer.open(9000, {'/api/v1/watch/events' => [200, {}]}) do |kube|
        kube.wait
        Kubeclient::Client.new('http://localhost:9000/api', "v1").watch_events(field_selector: 'involvedObject.kind=Pod').each {  }
      end
    end
  end
end

[chamnap]

I'm still facing this issue with webmock (3.1.1) and http (3.0.0) too. Thanks @grosser for the workaround! 👍

[wflanagan]

Problem still exists at the time of this writing.

[phoet]

with webmock 3.5.1 this is still an issue: https://github.com/penseo/firefighter/pull/3/files#diff-d2b7cb17e4aea0d9285eca5318ae8885R22

[tarcieri]

Anyone know why it broke this time?

[phoet]

@tarcieri has it been fixed in the mean time?

[tarcieri]

Webmock integration has been broken and fixed a half dozen times now. Perhaps with #499 it could be implemented in a less brittle way.

[phoet]

@tarcieri i'm asking because this particular issue is open since 2016

[phoet]

wouldnt it make sense to have a test-harness using webmock to check if it still works in order to limit the number of bugs occuring after refactorings and such?

[tarcieri]

That sounds like a good idea

[ixti]

Integration is on webmock's side, thus having CI for this on our end seems a bit weird to me. Thus I believe such regression test should be added to webmock's integration instead.

[phoet]

just guessing here, but i think that one of the problems is probably that webmocks "integration" is some kind of monkeypatch on httprb internals because there is not a proper API for those kind of use-cases.

if it seems weird to you, it could be an option to improve the test-support on the webmock side. i dont know how they handle their development and what methods they use in order to keep track with all the supported http libraries.

[ixti]

Basically they just have a different shims and specs for those. :D In any case, I guess it would be awesome to start with test case that will reproduce this bug with using HTTP gem only. That will help a lot.

[tarcieri]

@phoet I'd agree the reason they keep breaking is the way they're presently implemented is quite brittle and easily broken

because there is not a proper API for those kind of use-cases.

I think #499 is the basis of such an API /cc @paul

[ixti]

Without monkey-patches we will have to provide a global middlewares (same way as #499). And this will be even more brittle, as for WebMock shimming must be as close to metal as possible to avoid stubbing the world.

[jrochkind]

WebMock (3.13) currently still seems to break any use of streaming in http-rb (any version really). I got bit by it, and it took me a while to figure out what was going on. It is a big gotcha.

The code and tests may belong in WebMock, but the expertise to figure out what the WebMock http-rb adapter should look like probably liveshere, if anywhere. If there are any http-rb maintainers or hangers on who have the ability to figure out what WebMock http-rb adapter should look like to avoid this...

[bblimke]

I have applied the fix in WebMock master branch bblimke/webmock@216cad9
Big kudos to @aka-nez for the issue report and the sample code provided – it was super helpful in nailing this down.

The fix is a bit of a workaround. When WebMock is switched on, it'll swap out the original Response#body for a new one that can be read or streamed again. I know it's not the best fix, as it changes the original response behavior, but it was the best I could come up with given that the original Response::Body can't be reset.

[bblimke]

The fix is now released in WebMock 3.23.0

[ixti]

@bblimke thank you!