From 69d5c3721a71f33ed6be8cc8bb5d8a476efedea8 Mon Sep 17 00:00:00 2001
From: Erik Dubbelboer <erik@dubbelboer.com>
Date: Fri, 31 Jan 2020 22:21:00 +0100
Subject: [PATCH] Client should return ErrTimeout on timeout (#736)

Not ErrConnectionClosed which is incorrect.

Fixes: https://github.com/valyala/fasthttp/issues/355
---
 client.go                 | 23 ++++++++++++++++++++---
 client_test.go            |  4 +++-
 fasthttputil/pipeconns.go | 19 ++++++++++++++++++-
 header.go                 |  6 +++++-
 4 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/client.go b/client.go
index da4d329af5..961d6c0519 100644
--- a/client.go
+++ b/client.go
@@ -1284,9 +1284,6 @@ var (
 	// see this error.
 	ErrNoFreeConns = errors.New("no free connections available to host")
 
-	// ErrTimeout is returned from timed out calls.
-	ErrTimeout = errors.New("timeout")
-
 	// ErrConnectionClosed may be returned from client methods if the server
 	// closes connection before returning the first response byte.
 	//
@@ -1298,6 +1295,26 @@ var (
 		"Make sure the server returns 'Connection: close' response header before closing the connection")
 )
 
+type timeoutError struct {
+}
+
+func (e *timeoutError) Error() string {
+	return "timeout"
+}
+
+// Only implement the Timeout() function of the net.Error interface.
+// This allows for checks like:
+//
+//   if x, ok := err.(interface{ Timeout() bool }); ok && x.Timeout() {
+func (e *timeoutError) Timeout() bool {
+	return true
+}
+
+var (
+	// ErrTimeout is returned from timed out calls.
+	ErrTimeout = &timeoutError{}
+)
+
 // SetMaxConns sets up the maximum number of connections which may be established to all hosts listed in Addr.
 func (c *HostClient) SetMaxConns(newMaxConns int) {
 	c.connsLock.Lock()
diff --git a/client_test.go b/client_test.go
index dca004650d..841f1eef24 100644
--- a/client_test.go
+++ b/client_test.go
@@ -436,7 +436,9 @@ func TestClientReadTimeout(t *testing.T) {
 		req.SetRequestURI("http://localhost")
 		req.SetConnectionClose()
 
-		c.Do(req, res) //nolint:errcheck
+		if err := c.Do(req, res); err != ErrTimeout {
+			t.Errorf("expected ErrTimeout got %#v", err)
+		}
 
 		ReleaseRequest(req)
 		ReleaseResponse(res)
diff --git a/fasthttputil/pipeconns.go b/fasthttputil/pipeconns.go
index 62e8989d34..c992da30c6 100644
--- a/fasthttputil/pipeconns.go
+++ b/fasthttputil/pipeconns.go
@@ -197,9 +197,26 @@ func (c *pipeConn) readNextByteBuffer(mayBlock bool) error {
 var (
 	errWouldBlock       = errors.New("would block")
 	errConnectionClosed = errors.New("connection closed")
+)
+
+type timeoutError struct {
+}
+
+func (e *timeoutError) Error() string {
+	return "timeout"
+}
 
+// Only implement the Timeout() function of the net.Error interface.
+// This allows for checks like:
+//
+//   if x, ok := err.(interface{ Timeout() bool }); ok && x.Timeout() {
+func (e *timeoutError) Timeout() bool {
+	return true
+}
+
+var (
 	// ErrTimeout is returned from Read() or Write() on timeout.
-	ErrTimeout = errors.New("timeout")
+	ErrTimeout = &timeoutError{}
 )
 
 func (c *pipeConn) Close() error {
diff --git a/header.go b/header.go
index a2179d968e..b7a12ee219 100644
--- a/header.go
+++ b/header.go
@@ -1307,7 +1307,11 @@ func (h *ResponseHeader) tryRead(r *bufio.Reader, n int) error {
 	h.resetSkipNormalize()
 	b, err := r.Peek(n)
 	if len(b) == 0 {
-		// treat all errors on the first byte read as EOF
+		// Return ErrTimeout on any timeout.
+		if x, ok := err.(interface{ Timeout() bool }); ok && x.Timeout() {
+			return ErrTimeout
+		}
+		// treat all other errors on the first byte read as EOF
 		if n == 1 || err == io.EOF {
 			return io.EOF
 		}