martian: fix half-close propagation downstream when using http2 handler #870
Conversation
mmatczuk
force-pushed
the
mmt/h2_rst_fix
branch
3 times, most recently
from
08509be
to
9a0649a
Compare
internal/martian/errors.go
Outdated
| @@ -45,7 +45,8 @@ func isClosedConnError(err error) bool { | |||
| if errors.Is(err, io.EOF) || | |||
| errors.Is(err, io.ErrUnexpectedEOF) || | |||
| errors.Is(err, syscall.ECONNABORTED) || | |||
| errors.Is(err, syscall.ECONNRESET) { | |||
| errors.Is(err, syscall.ECONNRESET) || | |||
| errors.Is(err, errClosedBody) { | |||
There was a problem hiding this comment.
Yes! I've seen a lot of these recently.
internal/martian/unsafe.go
Outdated
| ) | ||
|
|
||
| //go:linkname errClosedBody golang.org/x/net/http2.errClosedBody | ||
| var errClosedBody error |
There was a problem hiding this comment.
Isn't that file name too general?
There was a problem hiding this comment.
Initially I called it h2Err... but reverted to that due to linter
There was a problem hiding this comment.
Yeah, I was thinking of some h2error.go
There was a problem hiding this comment.
go:linkname moved to errors.go and renamed to h2ErrClosedBody.
internal/martian/proxy_handler.go
Outdated
| } | ||
|
|
||
| func (w writeFlusher) Write(p []byte) (n int, err error) { | ||
| n, err = w.rw.Write(p) | ||
| func makeH2Writer(rw http.ResponseWriter, rc *http.ResponseController, req *http.Request) *h2Writer { |
There was a problem hiding this comment.
Return a pointer? Previous flusher was a copy, and the methods receive a copy too, so I'm double checking if that's on purpose.
There was a problem hiding this comment.
Nah, that's too fast generation...
.golangci.yml
Outdated
| - linters: | ||
| - revive | ||
| source: '_ "unsafe"' |
There was a problem hiding this comment.
grep for already existing comments
There was a problem hiding this comment.
Dropped that commit.
Use http2.Response.Close() to send RST_STREAM frame. Reimplement writeFlusher as h2Writer. This is to be tested in Sauce Connect due to lack of testing infrastructure for http2 at the moment. I added #869 to fix that.
No description provided.