Fix healthcheck and retries

This commit fixes #1179 in that it still executes a request even if all
nodes are dead and retries are disabled.

Also, this commit re-enables adding healthchecks if configured in the
config options. They were commented out in an earlier commit by mistake.

See #1179

Author: olivere

CONTRIBUTORS

@@ -114,6 +114,7 @@ Josh Chorlton [@jchorl](https://github.com/jchorl)
 Jpnock [@Jpnock](https://github.com/Jpnock)
 jun [@coseyo](https://github.com/coseyo)
 Junpei Tsuji [@jun06t](https://github.com/jun06t)
+Karen Yang [@kyangtt](https://github.com/kyangtt)
 kartlee [@kartlee](https://github.com/kartlee)
 Keith Hatton [@khatton-ft](https://github.com/khatton-ft)
 kel [@liketic](https://github.com/liketic)

client.go

@@ -465,11 +465,9 @@ func configToOptions(cfg *config.Config) ([]ClientOptionFunc, error) {
 		if cfg.Sniff != nil {
 			options = append(options, SetSniff(*cfg.Sniff))
 		}
-		/*
-			if cfg.Healthcheck != nil {
-				options = append(options, SetHealthcheck(*cfg.Healthcheck))
-			}
-		*/
+		if cfg.Healthcheck != nil {
+			options = append(options, SetHealthcheck(*cfg.Healthcheck))
+		}
 	}
 	return options, nil
 }
@@ -1275,6 +1273,7 @@ func (c *Client) PerformRequest(ctx context.Context, opt PerformRequestOptions)
 	basicAuthPassword := c.basicAuthPassword
 	sendGetBodyAs := c.sendGetBodyAs
 	gzipEnabled := c.gzipEnabled
+	healthcheckEnabled := c.healthcheckEnabled
 	retrier := c.retrier
 	if opt.Retrier != nil {
 		retrier = opt.Retrier
@@ -1307,6 +1306,10 @@ func (c *Client) PerformRequest(ctx context.Context, opt PerformRequestOptions)
 			if !retried {
 				// Force a healtcheck as all connections seem to be dead.
 				c.healthcheck(ctx, timeout, false)
+				if healthcheckEnabled {
+					retried = true
+					continue
+				}
 			}
 			wait, ok, rerr := retrier.Retry(ctx, n, nil, nil, err)
 			if rerr != nil {

client_test.go

@@ -1296,6 +1296,42 @@ func TestPerformRequestWithMaxResponseSize(t *testing.T) {
 	}
 }
 
+func TestPerformRequestOnNoConnectionsWithHealthcheckRevival(t *testing.T) {
+	fail := func(r *http.Request) (*http.Response, error) {
+		return nil, errors.New("request failed")
+	}
+	tr := &failingTransport{path: "/fail", fail: fail}
+	httpClient := &http.Client{Transport: tr}
+	client, err := NewClient(SetHttpClient(httpClient), SetMaxRetries(0), SetHealthcheck(true))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Run against a failing endpoint to mark connection as dead
+	res, err := client.PerformRequest(context.TODO(), PerformRequestOptions{
+		Method: "GET",
+		Path:   "/fail",
+	})
+	if err == nil {
+		t.Fatal(err)
+	}
+	if res != nil {
+		t.Fatal("expected no response")
+	}
+
+	// Forced healthcheck should bring connection back to life and complete request
+	res, err = client.PerformRequest(context.TODO(), PerformRequestOptions{
+		Method: "GET",
+		Path:   "/",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if res == nil {
+		t.Fatal("expected response to be != nil")
+	}
+}
+
 // failingTransport will run a fail callback if it sees a given URL path prefix.
 type failingTransport struct {
 	path string                                      // path prefix to look for

docker-compose.yml

@@ -2,7 +2,7 @@ version: '3'
 
 services:
   elasticsearch:
-    image: docker.elastic.co/elasticsearch/elasticsearch-oss:7.5.0
+    image: docker.elastic.co/elasticsearch/elasticsearch-oss:7.5.1
     hostname: elasticsearch
     environment:
       - cluster.name=elasticsearch
@@ -27,7 +27,7 @@ services:
     ports:
       - 9200:9200
   platinum:
-    image: docker.elastic.co/elasticsearch/elasticsearch:7.5.0
+    image: docker.elastic.co/elasticsearch/elasticsearch:7.5.1
     hostname: elasticsearch-platinum
     environment:
       - cluster.name=platinum