From f0cb2999600fd3836a005c7037a3dbc2641723f3 Mon Sep 17 00:00:00 2001 From: Nikita Velat Date: Tue, 18 Jun 2019 16:41:22 -0400 Subject: [PATCH 1/3] Retry specified HTTP methods on 500 errors --- restapi/api_client.go | 34 +++++++++++++++++++++++++++++----- restapi/provider.go | 14 ++++++++++++++ 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/restapi/api_client.go b/restapi/api_client.go index 2bb8ab89..7f6d970d 100644 --- a/restapi/api_client.go +++ b/restapi/api_client.go @@ -32,6 +32,7 @@ type apiClientOpt struct { xssi_prefix string use_cookies bool debug bool + retry_methods []string } type api_client struct { @@ -41,7 +42,6 @@ type api_client struct { username string password string headers map[string]string - redirects int use_cookie bool timeout int id_attribute string @@ -54,6 +54,7 @@ type api_client struct { create_returns_object bool xssi_prefix string debug bool + retry_methods []string } // Make a new api client for RESTful calls @@ -122,7 +123,7 @@ func NewAPIClient(opt *apiClientOpt) (*api_client, error) { create_returns_object: opt.create_returns_object, xssi_prefix: opt.xssi_prefix, debug: opt.debug, - redirects: 5, + retry_methods: opt.retry_methods, } if opt.debug { @@ -149,9 +150,23 @@ func (obj *api_client) toString() string { for _, n := range obj.copy_keys { buffer.WriteString(fmt.Sprintf(" %s", n)) } + buffer.WriteString(fmt.Sprintf("retry_methods:\n")) + for _, n := range obj.retry_methods { + buffer.WriteString(fmt.Sprintf(" %s", n)) + } return buffer.String() } +/* helper function for retry_methods condition */ +func sliceContains(a string, list []string) bool { + for _, b := range list { + if b == a { + return true + } + } + return false +} + /* Helper function that handles sending/receiving and handling of HTTP data in and out. TODO: Handle redirects */ @@ -214,7 +229,12 @@ func (client *api_client) send_request(method string, path string, data string) log.Printf("%s\n", body) } - for num_redirects := client.redirects; num_redirects >= 0; num_redirects-- { + num_retries := 0 + if len(client.retry_methods) > 0 && sliceContains(method, client.retry_methods) { + num_retries = 5 + } + + for num_retries >= 0 { resp, err := client.http_client.Do(req) if err != nil { @@ -241,8 +261,12 @@ func (client *api_client) send_request(method string, path string, data string) body := strings.TrimPrefix(string(bodyBytes), client.xssi_prefix) if resp.StatusCode == 301 || resp.StatusCode == 302 { - //Redirecting... decrement num_redirects and proceed to the next loop + //Redirecting... decrement num_retries and proceed to the next loop //uri = URI.parse(rsp['Location']) + } else if num_retries != 0 && (resp.StatusCode >= 500 || resp.StatusCode < 600) { + if client.debug { + log.Printf("Received response code '%d': %s - Retrying", resp.StatusCode, body) + } } else if resp.StatusCode == 404 || resp.StatusCode < 200 || resp.StatusCode >= 303 { return "", errors.New(fmt.Sprintf("Unexpected response code '%d': %s", resp.StatusCode, body)) } else { @@ -251,7 +275,7 @@ func (client *api_client) send_request(method string, path string, data string) } return body, nil } - + num_retries-- } //End loop through redirect attempts return "", errors.New("Error - too many redirects!") diff --git a/restapi/provider.go b/restapi/provider.go index e19881d6..8cfd7fac 100644 --- a/restapi/provider.go +++ b/restapi/provider.go @@ -76,6 +76,12 @@ func Provider() terraform.ResourceProvider { Description: "Defaults to `DELETE`. The HTTP method used to DELETE objects of this type on the API server.", Optional: true, }, + "retry_methods": &schema.Schema{ + Type: schema.TypeList, + Elem: &schema.Schema{Type: schema.TypeString}, + Optional: true, + Description: "Defines a list of HTTP methods to retry when retry_errors is set. Defaults to an empty list (no retries)", + }, "copy_keys": &schema.Schema{ Type: schema.TypeList, Elem: &schema.Schema{Type: schema.TypeString}, @@ -139,6 +145,13 @@ func configureProvider(d *schema.ResourceData) (interface{}, error) { } } + retry_methods := make([]string, 0) + if i_retry_methods := d.Get("retry_methods"); i_retry_methods != nil { + for _, v := range i_retry_methods.([]interface{}) { + retry_methods = append(retry_methods, v.(string)) + } + } + opt := &apiClientOpt{ uri: d.Get("uri").(string), insecure: d.Get("insecure").(bool), @@ -153,6 +166,7 @@ func configureProvider(d *schema.ResourceData) (interface{}, error) { create_returns_object: d.Get("create_returns_object").(bool), xssi_prefix: d.Get("xssi_prefix").(string), debug: d.Get("debug").(bool), + retry_methods: retry_methods, } if v, ok := d.GetOk("create_method"); ok { From 69c2b14595ac530a94dfcd507670981420d76b72 Mon Sep 17 00:00:00 2001 From: Nikita Velat Date: Tue, 18 Jun 2019 17:06:34 -0400 Subject: [PATCH 2/3] Typos --- restapi/api_client.go | 5 +++-- restapi/provider.go | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/restapi/api_client.go b/restapi/api_client.go index 7f6d970d..7a6cacf1 100644 --- a/restapi/api_client.go +++ b/restapi/api_client.go @@ -157,7 +157,7 @@ func (obj *api_client) toString() string { return buffer.String() } -/* helper function for retry_methods condition */ +/* Helper function for retry_methods condition */ func sliceContains(a string, list []string) bool { for _, b := range list { if b == a { @@ -229,6 +229,7 @@ func (client *api_client) send_request(method string, path string, data string) log.Printf("%s\n", body) } + /* Retry only if this is one of the user specified HTTP methods to retry on */ num_retries := 0 if len(client.retry_methods) > 0 && sliceContains(method, client.retry_methods) { num_retries = 5 @@ -276,7 +277,7 @@ func (client *api_client) send_request(method string, path string, data string) return body, nil } num_retries-- - } //End loop through redirect attempts + } //End loop through retry attempts return "", errors.New("Error - too many redirects!") } diff --git a/restapi/provider.go b/restapi/provider.go index 8cfd7fac..83a7aa54 100644 --- a/restapi/provider.go +++ b/restapi/provider.go @@ -80,7 +80,7 @@ func Provider() terraform.ResourceProvider { Type: schema.TypeList, Elem: &schema.Schema{Type: schema.TypeString}, Optional: true, - Description: "Defines a list of HTTP methods to retry when retry_errors is set. Defaults to an empty list (no retries)", + Description: "Defines a list of HTTP methods to retry on when receiving a HTTP response code in the 500 range. Defaults to an empty list (no retries)", }, "copy_keys": &schema.Schema{ Type: schema.TypeList, From ea106f1f2d1f36cc4b8577e2424f7c7cb20b7ee6 Mon Sep 17 00:00:00 2001 From: Nikita Velat Date: Tue, 18 Jun 2019 18:38:21 -0400 Subject: [PATCH 3/3] Testing details and various fixes --- restapi/api_client.go | 4 ++-- restapi/api_client_test.go | 22 ++++++++++++++++++++-- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/restapi/api_client.go b/restapi/api_client.go index 7a6cacf1..40166b12 100644 --- a/restapi/api_client.go +++ b/restapi/api_client.go @@ -264,7 +264,7 @@ func (client *api_client) send_request(method string, path string, data string) if resp.StatusCode == 301 || resp.StatusCode == 302 { //Redirecting... decrement num_retries and proceed to the next loop //uri = URI.parse(rsp['Location']) - } else if num_retries != 0 && (resp.StatusCode >= 500 || resp.StatusCode < 600) { + } else if num_retries != 0 && resp.StatusCode >= 500 && resp.StatusCode < 600 { if client.debug { log.Printf("Received response code '%d': %s - Retrying", resp.StatusCode, body) } @@ -279,5 +279,5 @@ func (client *api_client) send_request(method string, path string, data string) num_retries-- } //End loop through retry attempts - return "", errors.New("Error - too many redirects!") + return "", errors.New("Error - too many retries!") } diff --git a/restapi/api_client_test.go b/restapi/api_client_test.go index 75c56c33..5e73918a 100644 --- a/restapi/api_client_test.go +++ b/restapi/api_client_test.go @@ -29,7 +29,8 @@ func TestAPIClient(t *testing.T) { copy_keys: make([]string, 0), write_returns_object: false, create_returns_object: false, - debug: debug, + debug: debug, + retry_methods: []string{"GET"}, } client, _ := NewAPIClient(opt) @@ -67,6 +68,18 @@ func TestAPIClient(t *testing.T) { t.Fatalf("client_test.go: Timeout did not trigger on slow request") } + /* Verify retry on 500 error works */ + if debug { + log.Printf("api_client_test.go: Testing retry on 500 errors\n") + } + res, err = client.send_request("GET", "/error", "") + if err != nil { + t.Fatalf("client_test.go: %s", err) + } + if res != "The 2nd try will work" { + t.Fatalf("client_test.go: Got back '%s' but expected 'The 2nd try will work'\n", res) + } + if debug { log.Println("client_test.go: Stopping HTTP server") } @@ -88,7 +101,12 @@ func setup_api_client_server() { serverMux.HandleFunc("/redirect", func(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, "/ok", http.StatusPermanentRedirect) }) - + error := http.StatusInternalServerError + serverMux.HandleFunc("/error", func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(error) + w.Write([]byte("The 2nd try will work")) + error = http.StatusOK + }) api_client_server = &http.Server{ Addr: "127.0.0.1:8080", Handler: serverMux,