feat: retry only idempotent methods #777

go-resty · Oct 27, 2024 · 83ad104 · 83ad104
1 parent 88151c3
commit 83ad104
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 17 deletions.
diff --git a/multipart_test.go b/multipart_test.go
@@ -309,7 +309,7 @@ func TestMultipartCustomBoundary(t *testing.T) {
 }
 
 func TestMultipartLargeFile(t *testing.T) {
-	ts := createFilePostServer(t)
+	ts := createFileUploadServer(t)
 	defer ts.Close()
 
 	t.Run("upload a 2+mb image file with content-type and custom boundary", func(t *testing.T) {
@@ -432,7 +432,7 @@ func (errorReader) Read(p []byte) (n int, err error) {
 }
 
 func TestMultipartReaderErrors(t *testing.T) {
-	ts := createFilePostServer(t)
+	ts := createFileUploadServer(t)
 	defer ts.Close()
 
 	c := dcnl().SetBaseURL(ts.URL)

diff --git a/request.go b/request.go
@@ -1215,12 +1215,11 @@ func (r *Request) Execute(method, url string) (res *Response, err error) {
 	}
 
 	var backoff *backoffWithJitter
-	if r.RetryCount > 0 {
+	if r.RetryCount > 0 && r.isIdempotent() {
 		backoff = newBackoffWithJitter(r.RetryWaitTime, r.RetryMaxWaitTime)
 	}
 
-	// first request + retry count = total no. of requests
-
+	// first request + retry count = total no. of requests (aka total attempts)
 	for i := 0; i <= r.RetryCount; i++ {
 		r.Attempt++
 		err = nil
@@ -1234,7 +1233,7 @@ func (r *Request) Execute(method, url string) (res *Response, err error) {
 			}
 		}
 
-		// we have reached the maximum retry count stop here
+		// we have reached the maximum no. of requests
 		if r.Attempt-1 == r.RetryCount {
 			break
 		}
@@ -1519,6 +1518,22 @@ func (r *Request) resetFileReaders() error {
 	return nil
 }
 
+// https://datatracker.ietf.org/doc/html/rfc9110.html#name-idempotent-methods
+// https://datatracker.ietf.org/doc/html/rfc9110.html#name-method-registration
+var idempotentMethods = map[string]bool{
+	MethodDelete:  true,
+	MethodGet:     true,
+	MethodHead:    true,
+	MethodOptions: true,
+	MethodPut:     true,
+	MethodTrace:   true,
+}
+
+func (r *Request) isIdempotent() bool {
+	_, found := idempotentMethods[r.Method]
+	return found
+}
+
 func jsonIndent(v []byte) []byte {
 	buf := acquireBuffer()
 	defer releaseBuffer(buf)

diff --git a/request_test.go b/request_test.go
@@ -387,7 +387,6 @@ func TestForceContentTypeForGH276andGH240(t *testing.T) {
 	retried := 0
 	c := dcnl()
 	c.SetDebug(false)
-	c.SetRetryCount(3)
 
 	resp, err := c.R().
 		SetBody(map[string]any{"username": "testuser", "password": "testpass"}).
@@ -1572,7 +1571,7 @@ func TestRequestOverridesClientAuthorizationHeader(t *testing.T) {
 }
 
 func TestRequestFileUploadAsReader(t *testing.T) {
-	ts := createFilePostServer(t)
+	ts := createFileUploadServer(t)
 	defer ts.Close()
 
 	file, _ := os.Open(filepath.Join(getTestDataPath(), "test-img.png"))

diff --git a/resty_test.go b/resty_test.go
@@ -469,14 +469,14 @@ func createFormPatchServer(t *testing.T) *httptest.Server {
 	return ts
 }
 
-func createFilePostServer(t *testing.T) *httptest.Server {
+func createFileUploadServer(t *testing.T) *httptest.Server {
 	ts := createTestServer(func(w http.ResponseWriter, r *http.Request) {
 		t.Logf("Method: %v", r.Method)
 		t.Logf("Path: %v", r.URL.Path)
 		t.Logf("Content-Type: %v", r.Header.Get(hdrContentTypeKey))
 
-		if r.Method != MethodPost {
-			t.Log("createPostServer:: Not a Post request")
+		if r.Method != MethodPost && r.Method != MethodPut {
+			t.Log("createFileUploadServer:: Not a POST or PUT request")
 			w.WriteHeader(http.StatusBadRequest)
 			fmt.Fprint(w, http.StatusText(http.StatusBadRequest))
 			return

diff --git a/retry_test.go b/retry_test.go
@@ -658,7 +658,7 @@ func (f failingSeeker) Seek(offset int64, whence int) (int64, error) {
 }
 
 func TestResetMultipartReaderSeekStartError(t *testing.T) {
-	ts := createFilePostServer(t)
+	ts := createFileUploadServer(t)
 	defer ts.Close()
 
 	testSeeker := &failingSeeker{
@@ -671,14 +671,14 @@ func TestResetMultipartReaderSeekStartError(t *testing.T) {
 
 	resp, err := c.R().
 		SetFileReader("name", "filename", testSeeker).
-		Post(ts.URL + "/set-reset-multipart-readers-test")
+		Put(ts.URL + "/set-reset-multipart-readers-test")
 
 	assertEqual(t, 500, resp.StatusCode())
 	assertEqual(t, err.Error(), errSeekFailure.Error())
 }
 
 func TestClientResetMultipartReaders(t *testing.T) {
-	ts := createFilePostServer(t)
+	ts := createFileUploadServer(t)
 	defer ts.Close()
 
 	str := "test"
@@ -702,14 +702,14 @@ func TestClientResetMultipartReaders(t *testing.T) {
 
 	resp, err := c.R().
 		SetFileReader("name", "filename", bufReader).
-		Post(ts.URL + "/set-reset-multipart-readers-test")
+		Put(ts.URL + "/set-reset-multipart-readers-test")
 
 	assertEqual(t, 500, resp.StatusCode())
 	assertNil(t, err)
 }
 
 func TestRequestResetMultipartReaders(t *testing.T) {
-	ts := createFilePostServer(t)
+	ts := createFileUploadServer(t)
 	defer ts.Close()
 
 	str := "test"
@@ -733,7 +733,7 @@ func TestRequestResetMultipartReaders(t *testing.T) {
 	req := c.R().
 		SetRetryCount(2).
 		SetFileReader("name", "filename", bufReader)
-	resp, err := req.Post(ts.URL + "/set-reset-multipart-readers-test")
+	resp, err := req.Put(ts.URL + "/set-reset-multipart-readers-test")
 
 	assertEqual(t, 500, resp.StatusCode())
 	assertNil(t, err)