Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show correct port and address in connection exception messages #146

Merged
merged 1 commit into from
Nov 3, 2023
Merged

Show correct port and address in connection exception messages #146

merged 1 commit into from
Nov 3, 2023

Conversation

kmcphillips
Copy link
Contributor

Problem

When rescuing and reraising Errno::ECONNREFUSED and Errno::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 like Net::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. In net/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

> Net::HTTP::Persistent.new.request(URI("http://127.0.0.1:8765"))
Net::HTTP::Persistent::Error: connection refused: 127.0.0.1:80
from /path/lib/net/http/persistent.rb:611:in `rescue in connection_for'
Caused by Errno::ECONNREFUSED: Failed to open TCP connection to 127.0.0.1:8765 (Connection refused - connect(2) for "127.0.0.1" port 8765)

But now with this fix:

> Net::HTTP::Persistent.new.request(URI("http://127.0.0.1:8765"))
Net::HTTP::Persistent::Error: connection refused: 127.0.0.1:8765
from /path/net-http-persistent/lib/net/http/persistent.rb:623:in `rescue in connection_for'
Caused by Errno::ECONNREFUSED: Failed to open TCP connection to 127.0.0.1:8765 (Connection refused - connect(2) for "127.0.0.1" port 8765)

@tenderlove tenderlove merged commit 12a9230 into drbrain:master Nov 3, 2023
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants