issue-193: add retries on network errors (#194)

* issue-193: add retries on network errors issue-193: cleanup issue-193: make linter happy issue-193: cleanup issue-193: fix tests, add CHANGELOG * issue-193: cleanup * * simplify error checking logic by moving conditions within the function * move tests next to the file they test * re-word changelog message to emphasize that only temporary errors are retried --------- Co-authored-by: hagen1778 <[email protected]>
VictoriaMetrics · Aug 16, 2024 · 3b1925b · 3b1925b
1 parent ac7a87f
commit 3b1925b
Show file tree

Hide file tree

Showing 4 changed files with 408 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 ## tip
 
+* FEATURE: make retry attempt for datasource requests if returned error is a temporary network error. See [this issue](https://github.com/VictoriaMetrics/victoriametrics-datasource/issues/193)  
+
 ## [v0.8.5](https://github.com/VictoriaMetrics/victoriametrics-datasource/releases/tag/v0.8.5)
 
 * BUGFIX: restore support for Grafana versions below `10.0.0`. See [this issue](https://github.com/VictoriaMetrics/victoriametrics-datasource/issues/159).

diff --git a/pkg/plugin/datasource.go b/pkg/plugin/datasource.go
@@ -3,8 +3,11 @@ package plugin
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
+	"io"
 	"net/http"
+	"strings"
 	"sync"
 	"time"
 
@@ -114,8 +117,23 @@ func (d *Datasource) query(ctx context.Context, query backend.DataQuery) backend
 	}
 	resp, err := d.httpClient.Do(req)
 	if err != nil {
-		err = fmt.Errorf("failed to make http request: %w", err)
-		return newResponseError(err, backend.StatusBadRequest)
+		if !isTrivialError(err) {
+			// Return unexpected error to the caller.
+			return newResponseError(err, backend.StatusBadRequest)
+		}
+
+		// Something in the middle between client and datasource might be closing
+		// the connection. So we do a one more attempt in hope request will succeed.
+		req, err = http.NewRequestWithContext(ctx, settings.HTTPMethod, reqURL, nil)
+		if err != nil {
+			err = fmt.Errorf("failed to create new request with context: %w", err)
+			return newResponseError(err, backend.StatusBadRequest)
+		}
+		resp, err = d.httpClient.Do(req)
+		if err != nil {
+			err = fmt.Errorf("failed to make http request: %w", err)
+			return newResponseError(err, backend.StatusBadRequest)
+		}
 	}
 	defer func() {
 		if err := resp.Body.Close(); err != nil {
@@ -136,7 +154,7 @@ func (d *Datasource) query(ctx context.Context, query backend.DataQuery) backend
 
 	frames, err := r.getDataFrames()
 	if err != nil {
-		err = fmt.Errorf("failed to prepare data from reponse: %w", err)
+		err = fmt.Errorf("failed to prepare data from response: %w", err)
 		return newResponseError(err, backend.StatusInternal)
 	}
 	for i := range frames {
@@ -183,3 +201,16 @@ func newResponseError(err error, httpStatus backend.Status) backend.DataResponse
 	log.DefaultLogger.Error(err.Error())
 	return backend.DataResponse{Status: httpStatus, Error: err}
 }
+
+// isTrivialError returns true if the err is temporary and can be retried.
+func isTrivialError(err error) bool {
+	if errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF) {
+		return true
+	}
+	// Suppress trivial network errors, which could occur at remote side.
+	s := err.Error()
+	if strings.Contains(s, "broken pipe") || strings.Contains(s, "reset by peer") {
+		return true
+	}
+	return false
+}