From 879b6de9fe70b3f3884614752bc6d5d390c130fe Mon Sep 17 00:00:00 2001
From: Erwin <eticzongh@gmail.com>
Date: Sun, 7 May 2017 01:46:29 +1000
Subject: [PATCH] Add extra info on client related errors (#518)

This commit changes the errors returned from elastic. Error details should now be more precise and they contain the underlying error. We use `github.com/pkg/errors` for that.

Notice that a simple check like `if err == elastic.ErrNoClient` might fail now. To check for a connection error, use the `IsConnErr(err)` helper like so:

```
if elastic.IsConnErr(err) {
  t.Fatalf("Elasticsearch connection problem: %v", err)
}
```
---
 client.go      | 21 ++++++++++++++-------
 client_test.go |  6 +++---
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/client.go b/client.go
index 438de164d..6ef35282f 100644
--- a/client.go
+++ b/client.go
@@ -8,7 +8,6 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"net/http"
 	"net/http/httputil"
@@ -17,6 +16,8 @@ import (
 	"strings"
 	"sync"
 	"time"
+
+	"github.com/pkg/errors"
 )
 
 const (
@@ -811,7 +812,7 @@ func (c *Client) sniff(timeout time.Duration) error {
 	c.connsMu.RUnlock()
 
 	if len(urls) == 0 {
-		return ErrNoClient
+		return errors.Wrap(ErrNoClient, "no URLs found")
 	}
 
 	// Start sniffing on all found URLs
@@ -830,7 +831,7 @@ func (c *Client) sniff(timeout time.Duration) error {
 			}
 		case <-time.After(timeout):
 			// We get here if no cluster responds in time
-			return ErrNoClient
+			return errors.Wrap(ErrNoClient, "sniff timeout")
 		}
 	}
 }
@@ -1063,7 +1064,7 @@ func (c *Client) startupHealthcheck(timeout time.Duration) error {
 			break
 		}
 	}
-	return ErrNoClient
+	return errors.Wrap(ErrNoClient, "health check timeout")
 }
 
 // next returns the next available connection, or ErrNoClient.
@@ -1102,7 +1103,7 @@ func (c *Client) next() (*conn, error) {
 	}
 
 	// We tried hard, but there is no node available
-	return nil, ErrNoClient
+	return nil, errors.Wrap(ErrNoClient, "no avaiable connection")
 }
 
 // mustActiveConn returns nil if there is an active connection,
@@ -1116,7 +1117,7 @@ func (c *Client) mustActiveConn() error {
 			return nil
 		}
 	}
-	return ErrNoClient
+	return errors.Wrap(ErrNoClient, "no active connection found")
 }
 
 // PerformRequest does a HTTP request to Elasticsearch.
@@ -1157,7 +1158,7 @@ func (c *Client) PerformRequest(ctx context.Context, method, path string, params
 
 		// Get a connection
 		conn, err = c.next()
-		if err == ErrNoClient {
+		if errors.Cause(err) == ErrNoClient {
 			n++
 			if !retried {
 				// Force a healtcheck as all connections seem to be dead.
@@ -1716,3 +1717,9 @@ func (c *Client) WaitForGreenStatus(timeout string) error {
 func (c *Client) WaitForYellowStatus(timeout string) error {
 	return c.WaitForStatus("yellow", timeout)
 }
+
+// IsConnError unwraps the given error value and checks if it is equal to
+// elastic.ErrNoClient.
+func IsConnErr(err error) bool {
+	return errors.Cause(err) == ErrNoClient
+}
diff --git a/client_test.go b/client_test.go
index f038b4855..46aa42619 100644
--- a/client_test.go
+++ b/client_test.go
@@ -271,7 +271,7 @@ func TestClientHealthcheckStartupTimeout(t *testing.T) {
 	start := time.Now()
 	_, err := NewClient(SetURL("http://localhost:9299"), SetHealthcheckTimeoutStartup(5*time.Second))
 	duration := time.Now().Sub(start)
-	if err != ErrNoClient {
+	if !IsConnErr(err) {
 		t.Fatal(err)
 	}
 	if duration < 5*time.Second {
@@ -647,9 +647,9 @@ func TestClientSelectConnAllDead(t *testing.T) {
 	client.conns[1].MarkAsDead()
 
 	// If all connections are dead, next should make them alive again, but
-	// still return ErrNoClient when it first finds out.
+	// still return an error when it first finds out.
 	c, err := client.next()
-	if err != ErrNoClient {
+	if !IsConnErr(err) {
 		t.Fatal(err)
 	}
 	if c != nil {