Handle non-200 responses in CONNECT requests more granularly
Currently, in the Excon::SSLSocket class here: https://github.com/excon/excon/blob/e89bbb718bb67972e2e08109fb4c1edd09568cd5/lib/excon/ssl_socket.rb#L111, when making a CONNECT request to a proxy, we only check if the response status is 200. If it's not 200, we raise a ProxyConnectionError. However, this approach might be too simplistic and could be improved for better error handling and user experience.
I am thinking if we can:
- Handle different status codes (the more common ones) separately.
- Include the actual status code and response body in the error message for better debugging. Or
- Consider adding a way for users to customize how different status codes are handled.
If one of these suggested solutions makes sense, I'd be happy to submit a PR with the changes. Let me know your thoughts on this approach or if you have any other suggestions for improvement.
@shubhhq Thanks for the detailed write up. I don't have much personal experience around the proxy stuff, so it's helpful to have more context. (it maybe shows too in that looking now I even see an obvious typo in the error message when that connect fails, oops).
Of the suggestions you have there, I think adding information about the status code and response to the error message seems like a very straightforward improvement, which would probably be worth doing regardless of what else we might do. What do you think? If you agree, this would be something that you could tackle right away, while we continue to discuss the other portions.
Otherwise, providing better different handling (either directly or in a customizable way) is something I'm open to, but sounds like it could get more complicated. I don't have enough experience with proxies to know what the range of responses one might expect would be (and how to respond), and trying to make it customizable is certainly doable, but may be challenging in terms of figuring out what the right interface would be. I guess I also don't know enough about how consistently one is likely to react to particular errors or not.
Could you help me understand what kinds of errors you might expect and how you might react?
Thanks!
Some digging and figured out I am unblocked already by this piece of work in the past. But I still like the idea of having custom handlers so gonna lurk back soon. https://github.com/excon/excon/pull/715
@shubhhq sounds good. Thanks for digging in there, I totally forgot about that related fix.