Show correct port and address in connection exception messages #146
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
When rescuing and reraising
Errno::ECONNREFUSED
andErrno::EHOSTDOWN
errors, the error message shows the incorrect port number as always being port 80 when no proxy is used. Meaning the error will always look likeNet::HTTP::Persistent::Error: connection refused: 127.0.0.1:80
regardless of the actual port number that was attempted to connect to.The cause of this comes from
Net::HTTP
. Innet/http
when no proxy is passed in, the initialization branch has this line which will always set the proxy port to a default value of 80:https://github.com/ruby/ruby/blob/ad4f973ecd0a3481ff1abaa972d457e9f5b5fb4e/lib/net/http.rb#L1083
Meaning that when a new http object is initialized without a proxy and is passed
(address, port, nil, nil, nil, nil)
as it is here:https://github.com/drbrain/net-http-persistent/blob/c1fb1a5ca29c4c2a7102138af22529cc78adb06e/lib/net/http/persistent.rb#L590C29-L590C32
The instance of
Net::HTTP
will have nils everywhere for the proxies except for the port number. Thus the changed lines will always show port 80 in the error.Solution
The
proxy?
method is used internally to know if there is a proxy, so we can depend on that rather than||
the two values:https://github.com/ruby/ruby/blob/ad4f973ecd0a3481ff1abaa972d457e9f5b5fb4e/lib/net/http.rb#L1786-L1788
Reproduction
But now with this fix: