Simplify Net::HTTP connection handing #992
Conversation
rzane
force-pushed
the
request-connect
branch
2 times, most recently
from
bf8ee64
to
91253df
Compare
| @@ -72,78 +72,57 @@ def constants(inherit=true) | |||
| end | |||
|
|
|||
| def request(request, body = nil, &block) | |||
| return super unless started? | |||
There was a problem hiding this comment.
This lets us avoid having to deal with the case where the connection is not started yet. It makes the implementation much simpler.
| request_signature = WebMock::NetHTTPUtility.request_signature_from_request(self, request, body) | ||
|
|
||
| WebMock::RequestRegistry.instance.requested_signatures.put(request_signature) | ||
|
|
||
| if webmock_response = WebMock::StubRegistry.instance.response_for_request(request_signature) | ||
| @socket = Net::HTTP.socket_type.new |
There was a problem hiding this comment.
There's no reason to replace the socket here. At the very least, we'd want to close the old socket before initializing a new one. This would have caused a file descriptor leak in the scenario where the first request was allowed, but the second was stubbed.
| alias_method :start_with_connect, :start | ||
|
|
||
| def start(&block) | ||
| def connect |
There was a problem hiding this comment.
Net::HTTP#start calls #connect. This is a much simpler place for us to hook in, because unlike #start, #connect doesn't have a block variant.
At the very least, we'd want to close the current socket before assigning a new one. But, in this case, I think it's better to preserve the existing socket
The beginning of `#request` looks like this in net/http:
def request(req, body = nil, &block)
unless started?
start {
req['connection'] ||= 'close'
return request(req, body, &block)
}
end
| @@ -215,14 +215,14 @@ class TestMarshalingInWebMockNetHTTP | |||
|
|
|||
| if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('2.7.0') | |||
| it "uses the StubSocket to provide IP address" do | |||
| Net::HTTP.start("http://example.com") do |http| | |||
| Net::HTTP.start("example.com") do |http| | |||
There was a problem hiding this comment.
This is me fixing a mistake I made in #988 😄
|
Thank you @rzane . Much simpler. |
This PR aims to simplify the way that the stub socket is assigned and swapped in Net::HTTP.