Skip to content

Commit

Permalink
Add extra info on client related errors (#518)
Browse files Browse the repository at this point in the history
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)
}
```
  • Loading branch information
eticzon authored and olivere committed May 6, 2017
1 parent 42b0e5c commit 879b6de
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 10 deletions.
21 changes: 14 additions & 7 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"net/http"
"net/http/httputil"
Expand All @@ -17,6 +16,8 @@ import (
"strings"
"sync"
"time"

"github.com/pkg/errors"
)

const (
Expand Down Expand Up @@ -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
Expand All @@ -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")
}
}
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
6 changes: 3 additions & 3 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 879b6de

Please sign in to comment.