Use retryablehttp to manage retries

This handles more cases, including 504 and other timeouts

client.go

@@ -12,27 +12,18 @@ import (
 	"net/http"
 	"net/url"
 	"regexp"
-	"strconv"
 	"strings"
-	"time"
 
+	"github.com/hashicorp/go-retryablehttp"
 	"github.com/juju/errors"
 )
 
-const (
-	// Number of retries performed when the server returns a 503
-	// response with a 'Retry-after' header.  A request will be issued
-	// at most NumberOfRetries + 1 times.
-	NumberOfRetries = 4
-
-	RetryAfterHeaderName = "Retry-After"
-)
-
 // Client represents a way to communicating with a MAAS API instance.
 // It is stateless, so it can have concurrent requests in progress.
 type Client struct {
-	APIURL *url.URL
-	Signer OAuthSigner
+	APIURL     *url.URL
+	Signer     OAuthSigner
+	httpClient *retryablehttp.Client
 }
 
 // ServerError is an http error (or at least, a non-2xx result) received from
@@ -70,48 +61,14 @@ func readAndClose(stream io.ReadCloser) ([]byte, error) {
 // returned error will be ServerError and the returned body will reflect the
 // server's response.  If the server returns a 503 response with a 'Retry-after'
 // header, the request will be transparenty retried.
-func (client Client) dispatchRequest(request *http.Request) ([]byte, error) {
-	// First, store the request's body into a byte[] to be able to restore it
-	// after each request.
-	bodyContent, err := readAndClose(request.Body)
-	if err != nil {
-		return nil, err
-	}
-	for retry := 0; retry < NumberOfRetries; retry++ {
-		// Restore body before issuing request.
-		newBody := ioutil.NopCloser(bytes.NewReader(bodyContent))
-		request.Body = newBody
-		body, err := client.dispatchSingleRequest(request)
-		// If this is a 503 response with a non-void "Retry-After" header: wait
-		// as instructed and retry the request.
-		if err != nil {
-			serverError, ok := errors.Cause(err).(ServerError)
-			if ok && serverError.StatusCode == http.StatusServiceUnavailable {
-				retry_time_int, errConv := strconv.Atoi(serverError.Header.Get(RetryAfterHeaderName))
-				if errConv == nil {
-					select {
-					case <-time.After(time.Duration(retry_time_int) * time.Second):
-					}
-					continue
-				}
-			}
-		}
-		return body, err
-	}
-	// Restore body before issuing request.
-	newBody := ioutil.NopCloser(bytes.NewReader(bodyContent))
-	request.Body = newBody
-	return client.dispatchSingleRequest(request)
-}
+func (client Client) dispatchRequest(request *retryablehttp.Request) ([]byte, error) {
+	client.Signer.OAuthSign(&request.Header)
 
-func (client Client) dispatchSingleRequest(request *http.Request) ([]byte, error) {
-	client.Signer.OAuthSign(request)
-	httpClient := http.Client{}
 	// See https://code.google.com/p/go/issues/detail?id=4677
 	// We need to force the connection to close each time so that we don't
 	// hit the above Go bug.
 	request.Close = true
-	response, err := httpClient.Do(request)
+	response, err := client.httpClient.Do(request)
 	if err != nil {
 		return nil, err
 	}
@@ -148,9 +105,9 @@ func (client Client) Get(uri *url.URL, operation string, parameters url.Values)
 	if operation != "" {
 		parameters.Set("op", operation)
 	}
-	queryUrl := client.GetURL(uri)
-	queryUrl.RawQuery = parameters.Encode()
-	request, err := http.NewRequest("GET", queryUrl.String(), nil)
+	queryURL := client.GetURL(uri)
+	queryURL.RawQuery = parameters.Encode()
+	request, err := retryablehttp.NewRequest("GET", queryURL.String(), nil)
 	if err != nil {
 		return nil, err
 	}
@@ -204,7 +161,7 @@ func (client Client) nonIdempotentRequestFiles(method string, uri *url.URL, para
 	}
 	writer.Close()
 	url := client.GetURL(uri)
-	request, err := http.NewRequest(method, url.String(), buf)
+	request, err := retryablehttp.NewRequest(method, url.String(), buf)
 	if err != nil {
 		return nil, err
 	}
@@ -217,7 +174,7 @@ func (client Client) nonIdempotentRequestFiles(method string, uri *url.URL, para
 // requests (but not GET or DELETE requests).
 func (client Client) nonIdempotentRequest(method string, uri *url.URL, parameters url.Values) ([]byte, error) {
 	url := client.GetURL(uri)
-	request, err := http.NewRequest(method, url.String(), strings.NewReader(string(parameters.Encode())))
+	request, err := retryablehttp.NewRequest(method, url.String(), strings.NewReader(string(parameters.Encode())))
 	if err != nil {
 		return nil, err
 	}
@@ -245,7 +202,7 @@ func (client Client) Put(uri *url.URL, parameters url.Values) ([]byte, error) {
 // Delete deletes an object on the API, using an HTTP "DELETE" request.
 func (client Client) Delete(uri *url.URL) error {
 	url := client.GetURL(uri)
-	request, err := http.NewRequest("DELETE", url.String(), strings.NewReader(""))
+	request, err := retryablehttp.NewRequest("DELETE", url.String(), strings.NewReader(""))
 	if err != nil {
 		return err
 	}
@@ -259,7 +216,7 @@ func (client Client) Delete(uri *url.URL) error {
 // Anonymous "signature method" implementation.
 type anonSigner struct{}
 
-func (signer anonSigner) OAuthSign(request *http.Request) error {
+func (signer anonSigner) OAuthSign(request *http.Header) error {
 	return nil
 }
 
@@ -330,5 +287,16 @@ func NewAuthenticatedClient(versionedURL, apiKey string) (*Client, error) {
 	if err != nil {
 		return nil, err
 	}
-	return &Client{Signer: signer, APIURL: parsedURL}, nil
+
+	httpClient := retryablehttp.NewClient()
+
+	// Need to re-sign the request before each retry
+	httpClient.RequestLogHook = func(logger retryablehttp.Logger, request *http.Request, count int) {
+		err := signer.OAuthSign(&request.Header)
+		if err != nil {
+			logger.Printf("[ERROR] Failed to sign request: %v", err)
+		}
+	}
+
+	return &Client{Signer: signer, APIURL: parsedURL, httpClient: httpClient}, nil
 }

go.mod

@@ -3,17 +3,19 @@ module github.com/seanhoughton/gomaasapi
 go 1.13
 
 require (
+	github.com/hashicorp/go-retryablehttp v0.6.4
 	github.com/juju/collections v0.0.0-20180515203731-520e0549d51a
 	github.com/juju/errors v0.0.0-20150916125642-1b5e39b83d18
+	github.com/juju/gomaasapi v0.0.0-20190826212825-0ab1eb636aba
 	github.com/juju/loggo v0.0.0-20170605014607-8232ab8918d9
-	github.com/juju/retry v0.0.0-20151029024821-62c620325291
+	github.com/juju/retry v0.0.0-20151029024821-62c620325291 // indirect
 	github.com/juju/schema v0.0.0-20160420044203-075de04f9b7d
 	github.com/juju/testing v0.0.0-20180402130637-44801989f0f7
-	github.com/juju/utils v0.0.0-20180424094159-2000ea4ff043
+	github.com/juju/utils v0.0.0-20180424094159-2000ea4ff043 // indirect
 	github.com/juju/version v0.0.0-20161031051906-1f41e27e54f2
-	golang.org/x/crypto v0.0.0-20180214000028-650f4a345ab4
-	golang.org/x/net v0.0.0-20180406214816-61147c48b25b
+	golang.org/x/crypto v0.0.0-20180214000028-650f4a345ab4 // indirect
+	golang.org/x/net v0.0.0-20180406214816-61147c48b25b // indirect
 	gopkg.in/check.v1 v1.0.0-20160105164936-4f90aeace3a2
 	gopkg.in/mgo.v2 v2.0.0-20160818015218-f2b6f6c918c4
-	gopkg.in/yaml.v2 v2.0.0-20170712054546-1be3d31502d6
+	gopkg.in/yaml.v2 v2.0.0-20170712054546-1be3d31502d6 // indirect
 )

go.sum

@@ -1,19 +1,37 @@
+github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/hashicorp/go-cleanhttp v0.5.1 h1:dH3aiDG9Jvb5r5+bYHsikaOUIpcM0xvgMXVoDkXMzJM=
+github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
+github.com/hashicorp/go-hclog v0.9.2 h1:CG6TE5H9/JXsFWJCfoIVpKFIkFe6ysEuHirp4DxCsHI=
+github.com/hashicorp/go-hclog v0.9.2/go.mod h1:5CU+agLiy3J7N7QjHK5d05KxGsuXiQLrjA0H7acj2lQ=
+github.com/hashicorp/go-retryablehttp v0.6.4 h1:BbgctKO892xEyOXnGiaAwIoSq1QZ/SS4AhjoAh9DnfY=
+github.com/hashicorp/go-retryablehttp v0.6.4/go.mod h1:vAew36LZh98gCBJNLH42IQ1ER/9wtLZZ8meHqQvEYWY=
 github.com/juju/collections v0.0.0-20180515203731-520e0549d51a h1:PPCCWrZzJMhFu4PxX3vRM65dq7LZMyreWAMPsvttUQk=
 github.com/juju/collections v0.0.0-20180515203731-520e0549d51a/go.mod h1:Ep+c0vnxsgmmTtsMibPgEEleZyi0b4uVvyzJ+8ka9EI=
 github.com/juju/errors v0.0.0-20150916125642-1b5e39b83d18 h1:Sem5Flzxj8ZdAgY2wfHBUlOYyP4wrpIfM8IZgANNGh8=
 github.com/juju/errors v0.0.0-20150916125642-1b5e39b83d18/go.mod h1:W54LbzXuIE0boCoNJfwqpmkKJ1O4TCTZMetAt6jGk7Q=
+github.com/juju/gomaasapi v0.0.0-20190826212825-0ab1eb636aba h1:bnsLBCH2BUzTrl+XckuyTypG0AIWSAALekr1Rvqiy0Q=
+github.com/juju/gomaasapi v0.0.0-20190826212825-0ab1eb636aba/go.mod h1:ppx9XlnQMX/+h/kH+cU9kfDUT6GimqGtNRWdobUZVRE=
 github.com/juju/loggo v0.0.0-20170605014607-8232ab8918d9 h1:Y+lzErDTURqeXqlqYi4YBYbDd7ycU74gW1ADt57/bgY=
 github.com/juju/loggo v0.0.0-20170605014607-8232ab8918d9/go.mod h1:vgyd7OREkbtVEN/8IXZe5Ooef3LQePvuBm9UWj6ZL8U=
+github.com/juju/retry v0.0.0-20151029024821-62c620325291 h1:Rp0pLxDOsLDDwh2S73oHLI2KTFFyrF6oM/DgP0FhhBk=
 github.com/juju/retry v0.0.0-20151029024821-62c620325291/go.mod h1:OohPQGsr4pnxwD5YljhQ+TZnuVRYpa5irjugL1Yuif4=
 github.com/juju/schema v0.0.0-20160420044203-075de04f9b7d h1:JYANSZLNBXFgnNfGDOUAV+atWFDmOqJ1WPNmyS+YCCw=
 github.com/juju/schema v0.0.0-20160420044203-075de04f9b7d/go.mod h1:7dL+43wADDfx5rD9ibr5H9Dgr4iOM3uHOa1i4IVLak8=
+github.com/juju/testing v0.0.0-20180402130637-44801989f0f7 h1:IOzyKRl+7X8/fDIqNUDQH73yo8bqDrMEh90y9Il158A=
 github.com/juju/testing v0.0.0-20180402130637-44801989f0f7/go.mod h1:63prj8cnj0tU0S9OHjGJn+b1h0ZghCndfnbQolrYTwA=
+github.com/juju/utils v0.0.0-20180424094159-2000ea4ff043 h1:kjdsJcIYzmK2k4X2yVCi5Nip6sGoAuc7CLbp+qQnQUM=
 github.com/juju/utils v0.0.0-20180424094159-2000ea4ff043/go.mod h1:6/KLg8Wz/y2KVGWEpkK9vMNGkOnu4k/cqs8Z1fKjTOk=
 github.com/juju/version v0.0.0-20161031051906-1f41e27e54f2 h1:loQDi5MyxxNm7Q42mBGuPD6X+F6zw8j5S9yexLgn/BE=
 github.com/juju/version v0.0.0-20161031051906-1f41e27e54f2/go.mod h1:kE8gK5X0CImdr7qpSKl3xB2PmpySSmfj7zVbkZFs81U=
+github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
+github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
+golang.org/x/crypto v0.0.0-20180214000028-650f4a345ab4 h1:OfaUle5HH9Y0obNU74mlOZ/Igdtwi3eGOKcljJsTnbw=
 golang.org/x/crypto v0.0.0-20180214000028-650f4a345ab4/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
+golang.org/x/net v0.0.0-20180406214816-61147c48b25b h1:7rskAFQwNXGW6AD8E/6y0LDHW5mT9rsLD7ViLVFfh5w=
 golang.org/x/net v0.0.0-20180406214816-61147c48b25b/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
+gopkg.in/check.v1 v1.0.0-20160105164936-4f90aeace3a2 h1:+j1SppRob9bAgoYmsdW9NNBdKZfgYuWpqnYHv78Qt8w=
 gopkg.in/check.v1 v1.0.0-20160105164936-4f90aeace3a2/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/mgo.v2 v2.0.0-20160818015218-f2b6f6c918c4 h1:hILp2hNrRnYjZpmIbx70psAHbBSEcQ1NIzDcUbJ1b6g=
 gopkg.in/mgo.v2 v2.0.0-20160818015218-f2b6f6c918c4/go.mod h1:yeKp02qBN3iKW1OzL3MGk2IdtZzaj7SFntXj72NppTA=
+gopkg.in/yaml.v2 v2.0.0-20170712054546-1be3d31502d6 h1:CvAnnm1XvMjfib69SZzDwgWfOk+PxYz0hA0HBupilBA=
 gopkg.in/yaml.v2 v2.0.0-20170712054546-1be3d31502d6/go.mod h1:JAlM8MvJe8wmxCU4Bli9HhUf9+ttbYbLASfIpnQbh74=

oauth.go

@@ -28,7 +28,7 @@ func generateTimestamp() string {
 }
 
 type OAuthSigner interface {
-	OAuthSign(request *http.Request) error
+	OAuthSign(headers *http.Header) error
 }
 
 type OAuthToken struct {
@@ -52,7 +52,7 @@ func NewPlainTestOAuthSigner(token *OAuthToken, realm string) (OAuthSigner, erro
 
 // OAuthSignPLAINTEXT signs the provided request using the OAuth PLAINTEXT
 // method: http://oauth.net/core/1.0/#anchor22.
-func (signer plainTextOAuthSigner) OAuthSign(request *http.Request) error {
+func (signer plainTextOAuthSigner) OAuthSign(headers *http.Header) error {
 
 	signature := signer.token.ConsumerSecret + `&` + signer.token.TokenSecret
 	nonce, err := generateNonce()
@@ -75,6 +75,6 @@ func (signer plainTextOAuthSigner) OAuthSign(request *http.Request) error {
 		authHeader = append(authHeader, fmt.Sprintf(`%s="%s"`, key, url.QueryEscape(value)))
 	}
 	strHeader := "OAuth " + strings.Join(authHeader, ", ")
-	request.Header.Add("Authorization", strHeader)
+	headers.Add("Authorization", strHeader)
 	return nil
 }