From 54f3a0946f32d44f9c2dea07daf90cf20382730e Mon Sep 17 00:00:00 2001
From: Krishnan Parthasarathi <krisis@users.noreply.github.com>
Date: Fri, 4 Aug 2017 14:35:07 -0700
Subject: [PATCH] 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.
---
 pkg/http/listener.go      | 24 +++++++++++++++++++++---
 pkg/http/listener_test.go | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/pkg/http/listener.go b/pkg/http/listener.go
index 96e92951b..414317ed4 100644
--- a/pkg/http/listener.go
+++ b/pkg/http/listener.go
@@ -20,6 +20,7 @@ import (
 	"crypto/tls"
 	"errors"
 	"fmt"
+	"io"
 	"net"
 	"net/http"
 	"strings"
@@ -88,6 +89,16 @@ type httpListener struct {
 	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.
 func (listener *httpListener) start() {
 	listener.acceptCh = make(chan acceptResult)
@@ -122,9 +133,16 @@ func (listener *httpListener) start() {
 		data, err := bufconn.Peek(methodMaxLen)
 		if err != nil {
 			if listener.errorLogFunc != nil {
-				listener.errorLogFunc(err,
-					"Error in reading from new connection %s at server %s",
-					bufconn.RemoteAddr(), bufconn.LocalAddr())
+				// Peek could fail legitimately when clients abruptly close
+				// connection. E.g. Chrome browser opens connections speculatively to
+				// 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()
 			return
diff --git a/pkg/http/listener_test.go b/pkg/http/listener_test.go
index 12b80c022..149e405fb 100644
--- a/pkg/http/listener_test.go
+++ b/pkg/http/listener_test.go
@@ -815,3 +815,42 @@ func TestHTTPListenerAcceptParallel(t *testing.T) {
 		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)
+		}
+	}
+}