cmd error parsing: show a "try again after" message when a response sets a Retry-After header (#1198)

Author: IsaacG

cmd/cmd.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"net/http"
 	"regexp"
+	"strconv"
 	"strings"
 
 	"io"
@@ -80,6 +81,22 @@ func validateUserConfig(cfg *viper.Viper) error {
 // decodedAPIError decodes and returns the error message from the API response.
 // If the message is blank, it returns a fallback message with the status code.
 func decodedAPIError(resp *http.Response) error {
+	// First and foremost, handle Retry-After headers; if set, show this to the user.
+	if retryAfter := resp.Header.Get("Retry-After"); retryAfter != "" {
+		// The Retry-After header can be an HTTP Date or delay seconds.
+		// The date can be used as-is. The delay seconds should have "seconds" appended.
+		if delay, err := strconv.Atoi(retryAfter); err == nil {
+			retryAfter = fmt.Sprintf("%d seconds", delay)
+		}
+		return fmt.Errorf(
+			"request failed with status %s; please try again after %s",
+			resp.Status,
+			retryAfter,
+		)
+	}
+
+	// Check for JSON data. On non-JSON data, show the status and content type then bail.
+	// Otherwise, extract the message details from the JSON.
 	if contentType := resp.Header.Get("Content-Type"); !jsonContentTypeRe.MatchString(contentType) {
 		return fmt.Errorf(
 			"expected response with Content-Type \"application/json\" but got status %q with Content-Type %q",

cmd/cmd_test.go

@@ -123,7 +123,7 @@ func (co capturedOutput) reset() {
 	Err = co.oldErr
 }
 
-func errorResponse(contentType string, body string) *http.Response {
+func errorResponse418(contentType string, body string) *http.Response {
 	response := &http.Response{
 		Status:        "418 I'm a teapot",
 		StatusCode:    418,
@@ -135,35 +135,57 @@ func errorResponse(contentType string, body string) *http.Response {
 	return response
 }
 
+func errorResponse429(retryAfter string) *http.Response {
+	body := ""
+	response := &http.Response{
+		Status:        "429 Too Many Requests",
+		StatusCode:    429,
+		Header:        make(http.Header),
+		Body:          ioutil.NopCloser(strings.NewReader(body)),
+		ContentLength: int64(len(body)),
+	}
+	response.Header.Set("Content-Type", "text/plain")
+	response.Header.Set("Retry-After", retryAfter)
+	return response
+}
+
 func TestDecodeErrorResponse(t *testing.T) {
 	testCases := []struct {
 		response    *http.Response
 		wantMessage string
 	}{
 		{
-			response:    errorResponse("text/html", "Time for tea"),
+			response:    errorResponse418("text/html", "Time for tea"),
 			wantMessage: `expected response with Content-Type "application/json" but got status "418 I'm a teapot" with Content-Type "text/html"`,
 		},
 		{
-			response:    errorResponse("application/json", `{"error": {"type": "json", "valid": no}}`),
+			response:    errorResponse418("application/json", `{"error": {"type": "json", "valid": no}}`),
 			wantMessage: "failed to parse API error response: invalid character 'o' in literal null (expecting 'u')",
 		},
 		{
-			response:    errorResponse("application/json", `{"error": {"type": "track_ambiguous", "message": "message", "possible_track_ids": ["a", "b"]}}`),
+			response:    errorResponse418("application/json", `{"error": {"type": "track_ambiguous", "message": "message", "possible_track_ids": ["a", "b"]}}`),
 			wantMessage: "message: a, b",
 		},
 		{
-			response:    errorResponse("application/json", `{"error": {"message": "message"}}`),
+			response:    errorResponse418("application/json", `{"error": {"message": "message"}}`),
 			wantMessage: "message",
 		},
 		{
-			response:    errorResponse("application/problem+json", `{"error": {"message": "new json format"}}`),
+			response:    errorResponse418("application/problem+json", `{"error": {"message": "new json format"}}`),
 			wantMessage: "new json format",
 		},
 		{
-			response:    errorResponse("application/json", `{"error": {}}`),
+			response:    errorResponse418("application/json", `{"error": {}}`),
 			wantMessage: "unexpected API response: 418",
 		},
+		{
+			response:    errorResponse429("30"),
+			wantMessage: "request failed with status 429 Too Many Requests; please try again after 30 seconds",
+		},
+		{
+			response:    errorResponse429("Wed, 21 Oct 2015 07:28:00 GMT"),
+			wantMessage: "request failed with status 429 Too Many Requests; please try again after Wed, 21 Oct 2015 07:28:00 GMT",
+		},
 	}
 	tc := testCases[0]
 	got := decodedAPIError(tc.response)