acme: consistently return original errors from retries
The retry logic returns an "acme: no more retries for ..." error
in some cases, while *Error type in others.
This change makes retries always return the last error as received
from the CA server, if available. No change in returned values
of successful requests.
Change-Id: I3df2cb332a3e2739bba457c0ee50d7ca5bd836d9
Reviewed-on: https://go-review.googlesource.com/119975
Reviewed-by: Maciej Dębski <maciejd@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Alex Vaghin <ddos@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
diff --git a/acme/http.go b/acme/http.go
index 56ba53a..a43ce6a 100644
--- a/acme/http.go
+++ b/acme/http.go
@@ -140,10 +140,13 @@
case ok(res):
return res, nil
case isRetriable(res.StatusCode):
- res.Body.Close()
retry.inc()
- if err := retry.backoff(ctx, req, res); err != nil {
- return nil, err
+ resErr := responseError(res)
+ res.Body.Close()
+ // Ignore the error value from retry.backoff
+ // and return the one from last retry, as received from the CA.
+ if retry.backoff(ctx, req, res) != nil {
+ return nil, resErr
}
default:
defer res.Body.Close()
@@ -169,20 +172,22 @@
if ok(res) {
return res, nil
}
- err = responseError(res)
+ resErr := responseError(res)
res.Body.Close()
switch {
// Check for bad nonce before isRetriable because it may have been returned
// with an unretriable response code such as 400 Bad Request.
- case isBadNonce(err):
+ case isBadNonce(resErr):
// Consider any previously stored nonce values to be invalid.
c.clearNonces()
case !isRetriable(res.StatusCode):
- return nil, err
+ return nil, resErr
}
retry.inc()
+ // Ignore the error value from retry.backoff
+ // and return the one from last retry, as received from the CA.
if err := retry.backoff(ctx, req, res); err != nil {
- return nil, err
+ return nil, resErr
}
}
}
diff --git a/acme/http_test.go b/acme/http_test.go
index c9ce1d6..15e401b 100644
--- a/acme/http_test.go
+++ b/acme/http_test.go
@@ -88,12 +88,14 @@
}
head, err := decodeJWSHead(r)
- if err != nil {
+ switch {
+ case err != nil:
t.Errorf("decodeJWSHead: %v", err)
- } else if head.Nonce == "" {
+ case head.Nonce == "":
t.Error("head.Nonce is empty")
- } else if head.Nonce == "nonce1" {
- // return a badNonce error to force the call to retry
+ case head.Nonce == "nonce1":
+ // Return a badNonce error to force the call to retry.
+ w.Header().Set("Retry-After", "0")
w.WriteHeader(http.StatusBadRequest)
w.Write([]byte(`{"type":"urn:ietf:params:acme:error:badNonce"}`))
return
@@ -114,6 +116,55 @@
}
}
+func TestRetryErrorType(t *testing.T) {
+ ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.Header().Set("Replay-Nonce", "nonce")
+ w.WriteHeader(http.StatusTooManyRequests)
+ w.Write([]byte(`{"type":"rateLimited"}`))
+ }))
+ defer ts.Close()
+
+ client := &Client{
+ Key: testKey,
+ RetryBackoff: func(n int, r *http.Request, res *http.Response) time.Duration {
+ // Do no retries.
+ return 0
+ },
+ dir: &Directory{AuthzURL: ts.URL},
+ }
+
+ t.Run("post", func(t *testing.T) {
+ testRetryErrorType(t, func() error {
+ _, err := client.Authorize(context.Background(), "example.com")
+ return err
+ })
+ })
+ t.Run("get", func(t *testing.T) {
+ testRetryErrorType(t, func() error {
+ _, err := client.GetAuthorization(context.Background(), ts.URL)
+ return err
+ })
+ })
+}
+
+func testRetryErrorType(t *testing.T, callClient func() error) {
+ t.Helper()
+ err := callClient()
+ if err == nil {
+ t.Fatal("client.Authorize returned nil error")
+ }
+ acmeErr, ok := err.(*Error)
+ if !ok {
+ t.Fatalf("err is %v (%T); want *Error", err, err)
+ }
+ if acmeErr.StatusCode != http.StatusTooManyRequests {
+ t.Errorf("acmeErr.StatusCode = %d; want %d", acmeErr.StatusCode, http.StatusTooManyRequests)
+ }
+ if acmeErr.ProblemType != "rateLimited" {
+ t.Errorf("acmeErr.ProblemType = %q; want 'rateLimited'", acmeErr.ProblemType)
+ }
+}
+
func TestRetryBackoffArgs(t *testing.T) {
const resCode = http.StatusInternalServerError
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
