ncr / rack-proxy

A request/response rewriting HTTP proxy. A Rack app.
MIT License
269 stars 94 forks source link

undefined method `closed?' for nil:NilClass when using rack-proxy with WebMock #94

Open ndbroadbent opened 3 years ago

ndbroadbent commented 3 years ago

I'm using rack-reverse-proxy, which uses rack-proxy.

I'm trying to write a test to ensure that rack-reverse-proxy is working correctly, but it looks like WebMock / VCR is causing a conflict with the net_http_hacked changes in this library.

When I call get in my test, I see this error:

#<NoMethodError: undefined method `closed?' for nil:NilClass>
/Users/ndbroadbent/.rbenv/versions/2.7.1/lib/ruby/2.7.0/net/http.rb:1566:in `begin_transport'
/Users/ndbroadbent/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rack-proxy-0.6.5/lib/net_http_hacked.rb:50:in `begin_request_hacked'
/Users/ndbroadbent/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rack-proxy-0.6.5/lib/rack/http_streaming_response.rb:60:in `response'
/Users/ndbroadbent/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rack-proxy-0.6.5/lib/rack/http_streaming_response.rb:29:in `headers'
/Users/ndbroadbent/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rack-reverse-proxy-0.12.0/lib/rack_reverse_proxy/roundtrip.rb:166:in `rack_response_headers'
/Users/ndbroadbent/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rack-reverse-proxy-0.12.0/lib/rack_reverse_proxy/roundtrip.rb:157:in `build_response_headers'
/Users/ndbroadbent/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rack-reverse-proxy-0.12.0/lib/rack_reverse_proxy/roundtrip.rb:153:in `response_headers'
/Users/ndbroadbent/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rack-reverse-proxy-0.12.0/lib/rack_reverse_proxy/roundtrip.rb:182:in `need_replace_location?'
/Users/ndbroadbent/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rack-reverse-proxy-0.12.0/lib/rack_reverse_proxy/roundtrip.rb:172:in `replace_location_header'
/Users/ndbroadbent/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rack-reverse-proxy-0.12.0/lib/rack_reverse_proxy/roundtrip.rb:197:in `setup_response_headers'
/Users/ndbroadbent/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rack-reverse-proxy-0.12.0/lib/rack_reverse_proxy/roundtrip.rb:209:in `proxy'
/Users/ndbroadbent/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rack-reverse-proxy-0.12.0/lib/rack_reverse_proxy/roundtrip.rb:21:in `call'
/Users/ndbroadbent/.rbenv/versions/2.7.1/lib/ruby/gems/2.7.0/gems/rack-reverse-proxy-0.12.0/lib/rack_reverse_proxy/middleware.rb:25:in `call'

Code:

# /Users/ndbroadbent/.rbenv/versions/2.7.1/lib/ruby/2.7.0/net/http.rb:1566

    def begin_transport(req)
      if @socket.closed?
        connect
      elsif @last_communicated
      # ...

It looks like @socket is nil, maybe because WebMock is also stubbing parts of Net::HTTP?

Note that this gem does work great in production, but it's really dangerous that I am unable to write any integration tests to ensure that it keeps working properly.


UPDATE: I can get the test to pass if I call WebMock.disable!:

  around(:each) do |example|
    WebMock.disable!
    example.run
    WebMock.enable!
  end

But it would be great if I could actually mock the requests and save the request/response in VCR.

pmackay commented 3 years ago

I'm also experiencing this, if there is any solution that doesnt require disabling webmock it would be great understand if thats possible.

tlubz commented 2 years ago

I've found a workaround for this in tests, but it short-circuits streaming responses, so I would not use the workaround in production due to performance concerns.

The solution is to pass the streaming: false option to the Rack::Proxy middleware when initializing it.

The problem seems to be that WebMock monkey-patches Net::HTTP#request and Net::HTTP#start, including injecting a stubbed socket. Meanwhile, (but only in streaming mode), Rack::Proxy bypasses Net::HTTP#request to use lower-level Net::HTTP methods and obtain a streamable rack response, so the WebMock abstraction is broken and the socket is never set before being used by the underlying Net::HTTP implementation.

you may choose to disable streaming only in tests, since webmock should only monkey-patch in that environment.