Permalink
Browse files

Use with_connection.

  • Loading branch information...
1 parent 1f0bb11 commit 84fb704d1dbfab5a9d58d9d38a0923ecb49008f8 @i0rek committed Sep 24, 2012
Showing with 16 additions and 18 deletions.
  1. +0 −14 lib/vcr/library_hooks/typhoeus.rb
  2. +7 −3 lib/vcr/library_hooks/webmock.rb
  3. +9 −1 spec/support/http_library_adapters.rb
@@ -87,20 +87,6 @@ def self.vcr_response_from(response)
end
end
-# @private
-module Typhoeus
- class << Hydra
- # ensure HTTP requests are always allowed; VCR takes care of disallowing
- # them at the appropriate times in its hook
- def allow_net_connect_with_vcr?(*args)
- VCR.turned_on? ? true : allow_net_connect_without_vcr?
- end
-
- alias allow_net_connect_without_vcr? allow_net_connect?
- alias allow_net_connect? allow_net_connect_with_vcr?
- end unless Hydra.respond_to?(:allow_net_connect_with_vcr?)
-end
-
VCR.configuration.after_library_hooks_loaded do
# ensure WebMock's Typhoeus adapter does not conflict with us here
# (i.e. to double record requests or whatever).
@@ -128,12 +128,16 @@ def on_stubbed_by_vcr_request
end
::WebMock.after_request(:real_requests_only => true) do |request, response|
+ ret = nil
unless VCR.library_hooks.disabled?(:webmock)
- http_interaction = VCR::HTTPInteraction.new \
- typed_request_for(request), vcr_response_for(response)
+ ::Typhoeus.with_connection do
@myronmarston
myronmarston Sep 24, 2012

Why are you accessing Typhoeus directly in the WebMock hook? Note that WebMock can be used in many environments, including environments that don't have typhoeus installed. This needs to work without accessing Typhoeus directly.

@i0rek
i0rek Sep 24, 2012 Owner

Oh, you're right! I shouldn't. Will look into it.

+ http_interaction = VCR::HTTPInteraction.new \
+ typed_request_for(request), vcr_response_for(response)
- VCR.record_http_interaction(http_interaction)
+ ret = VCR.record_http_interaction(http_interaction)
+ end
end
+ ret
end
::WebMock.after_request do |request, response|
@@ -164,7 +164,15 @@ def get_header(header_key, response)
end
def make_http_request(method, url, body = nil, headers = {})
- Typhoeus::Request.send(method, url, :body => body, :headers => headers)
+ request = Typhoeus::Request.new(url, :method => method, :body => body, :headers => headers)
+ if VCR.turned_on?
+ Typhoeus.with_connection do
+ request.run
+ end
+ else
+ request.run
+ end
+ request.response
@myronmarston
myronmarston Oct 1, 2012

make_http_request should make the request in the same way an end user would. I don't think an end user should have to check VCR.turned_on? or wrap Typhoeus.with_connection around each request they make to use Typhoeus with VCR.

Can we find an alternate way to make this work that puts the logic for this in VCR's typhoeus hook?

@i0rek
i0rek Oct 1, 2012 Owner

You're right, I'll think about it.

@myronmarston
myronmarston Oct 1, 2012

I think the basic problem is that allowing connection (or not) is a global setting. What if it was a setting on each request object, and the request object got its default value for the setting from the global setting you have now? Then in the before hook, VCR could set request.allow_connection if VCR.turned_on? -- that way, VCR could ensure all connections are allowed for requests it is managing, but if VCR is turned off, the global setting would take over (unless something else changed the setting on the request instance, but that's fine--more flexibility!)

@i0rek
i0rek Oct 1, 2012 Owner

Sounds good to me!

@i0rek
i0rek Oct 2, 2012 Owner

I've added Request#block_connection: typhoeus/typhoeus@0caa30b. Do you change it yourself or should I submit a PR?

@myronmarston
myronmarston Oct 2, 2012

I'll take care of it. Thanks!

end
def normalize_request_headers(headers)

0 comments on commit 84fb704

Please sign in to comment.