- Sponsor
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
http.CloseNotifier is deprecated #347
Comments
pkieltyka
added a commit
that referenced
this issue
Jan 8, 2019
done in #378 |
7 tasks
leighmcculloch
added a commit
to stellar/go
that referenced
this issue
Feb 11, 2020
…2256) ### What Remove old code that uses a http.ResponseWriter's CloseNotifier channel to cause the request's context to be canceled on the server side. ### Why I want to upgrade us to chi v4 which no longer supports CloseNotifier and I want to remove any use of it in our code to ensure we aren't dependent on chi providing ResponseWriter's that support it. It turns out the removed code is no longer necessary and can be just deleted without any new functionality. In Go 1.8 the context was changed to automatically cancel if the client closes its connection and so we do not need to also listen to the CloseNotifier channel or replace it with anything else. As a bonus this also reduces the number of Go routines required to handle each request in Horizon by one. ### References Go 1.8 Release Notes: >The Context returned by Request.Context is canceled if the underlying net.Conn closes. For instance, if the user closes their browser in the middle of a slow request, the Handler can now detect that the user is gone. This complements the existing CloseNotifier support. https://golang.org/doc/go1.8#net_http The CloseNotifier interface has been deprecated since Go 1.8: https://golang.org/pkg/net/http/#CloseNotifier The muxer/router we use, `chi`, removed any functionality relating to CloseNotifier in its major 4.0.0 release: go-chi/chi#347 ### Testing I wrote a test and confirmed that it fails on Go 1.7 when the code in the httpx package was deleted and passes when it is present. I then ran this test with and without the deleted code in Go 1.8 and Go 1.13 and in both cases it passes, which confirms that this would have been fine to remove once the oldest version of Go we supported was 1.8. I removed the test before merging, but the test is still visible in the commit history of the PR #2256.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Started seeing this pop up in my linters, not sure if this code could simply be removed?
chi/middleware/wrap_writer.go
Line 103 in b5294d1
chi/middleware/wrap_writer.go
Line 143 in b5294d1
From godoc.org:
The text was updated successfully, but these errors were encountered: