Avoid superfluous error messages after connect (#4762)

Peek could fail legitimately when clients abruptly close connection. So,
io.EOF and network timeout errors are not logged while all other errors
will be logged.
master
Krishnan Parthasarathi 7 years ago committed by Dee Koder
parent be6bd52978
commit 54f3a0946f
  1. 24
      pkg/http/listener.go
  2. 39
      pkg/http/listener_test.go

@ -20,6 +20,7 @@ import (
"crypto/tls" "crypto/tls"
"errors" "errors"
"fmt" "fmt"
"io"
"net" "net"
"net/http" "net/http"
"strings" "strings"
@ -88,6 +89,16 @@ type httpListener struct {
errorLogFunc func(error, string, ...interface{}) // function to be called on errors. errorLogFunc func(error, string, ...interface{}) // function to be called on errors.
} }
// ignoreErr returns true if error is due to a network timeout or
// io.EOF and false otherwise
func ignoreErr(err error) bool {
if nErr, ok := err.(*net.OpError); ok {
return nErr.Timeout()
}
return err == io.EOF
}
// start - starts separate goroutine for each TCP listener. A valid insecure/TLS HTTP new connection is passed to httpListener.acceptCh. // start - starts separate goroutine for each TCP listener. A valid insecure/TLS HTTP new connection is passed to httpListener.acceptCh.
func (listener *httpListener) start() { func (listener *httpListener) start() {
listener.acceptCh = make(chan acceptResult) listener.acceptCh = make(chan acceptResult)
@ -122,9 +133,16 @@ func (listener *httpListener) start() {
data, err := bufconn.Peek(methodMaxLen) data, err := bufconn.Peek(methodMaxLen)
if err != nil { if err != nil {
if listener.errorLogFunc != nil { if listener.errorLogFunc != nil {
listener.errorLogFunc(err, // Peek could fail legitimately when clients abruptly close
"Error in reading from new connection %s at server %s", // connection. E.g. Chrome browser opens connections speculatively to
bufconn.RemoteAddr(), bufconn.LocalAddr()) // speed up loading of a web page. Peek may also fail due to network
// saturation on a transport with read timeout set. All other kind of
// errors should be logged for further investigation. Thanks @brendanashworth.
if !ignoreErr(err) {
listener.errorLogFunc(err,
"Error in reading from new connection %s at server %s",
bufconn.RemoteAddr(), bufconn.LocalAddr())
}
} }
bufconn.Close() bufconn.Close()
return return

@ -815,3 +815,42 @@ func TestHTTPListenerAcceptParallel(t *testing.T) {
listener.Close() listener.Close()
} }
} }
type myTimeoutErr struct {
timeout bool
}
func (m *myTimeoutErr) Error() string { return fmt.Sprintf("myTimeoutErr: %v", m.timeout) }
func (m *myTimeoutErr) Timeout() bool { return m.timeout }
// Test for ignoreErr helper function
func TestIgnoreErr(t *testing.T) {
testCases := []struct {
err error
want bool
}{
{
err: io.EOF,
want: true,
},
{
err: &net.OpError{Err: &myTimeoutErr{timeout: true}},
want: true,
},
{
err: &net.OpError{Err: &myTimeoutErr{timeout: false}},
want: false,
},
{
err: io.ErrUnexpectedEOF,
want: false,
},
}
for i, tc := range testCases {
if actual := ignoreErr(tc.err); actual != tc.want {
t.Errorf("Test case %d: Expected %v but got %v for %v", i+1,
tc.want, actual, tc.err)
}
}
}

Loading…
Cancel
Save