fix(deps): replace deprecated httputil.ClientConn and hijack functionality #1090
Conversation
…ality Fix #919 BREAKING CHANGE: Remove usage of deprecated httputil.ClientConn in favor of net/http.Client per golang/go#28030. Since HTTP connection hijacking is no longer supported in modern Go, implemented alternative approach for bidirectional TCP communication.
There was a problem hiding this comment.
sorry for the delay to review this PR.
praise: that's a long standing issue, thanks a lot for taking it!
question: you mention a breaking change in the PR description but I don't really understand what it this breaking change. How is it breaking the current CLI API?
question: did you tested your change against the staging and / or prod environment?
I mentioned a breaking change because I'm replacing the use of httputil.ClientConn with net/http.Client, and there's no longer any hijacking properly speaking. I'll remove the breaking change mention as it's not relevant in the context of a cli client.
I've tested locally (http) and on staging for the TLS layer. I've not tested against production but in theory I could indeed |
- Replace errgo.Mask/New/Notef with errors.Wrap/Wrapf/New/Newf - Add context parameter to internal methods to avoid using nil (SA1012) - Remove "fail to" phrases from error messages - Accept both 200 and 202 as valid status codes for run responses
|
Let me know what you think about those changes, I took the opportunity to replace errgo throughout all errors. I tested |
Fix #919
Remove usage of deprecated httputil.ClientConn in favor of net/http.Client per golang/go#28030. Since HTTP connection hijacking is no longer supported in modern Go, implemented alternative approach for bidirectional TCP communication.