diff --git a/cmd/cmd.go b/cmd/cmd.go index dcca08264..91597b5b1 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "net/http" + "regexp" "strings" "io" @@ -23,6 +24,8 @@ var ( Out io.Writer // Err is used to write errors. Err io.Writer + // jsonContentTypeRe is used to match Content-Type which contains JSON. + jsonContentTypeRe = regexp.MustCompile(`^application/([[:alpha:]]+\+)?json$`) ) const msgWelcomePleaseConfigure = ` @@ -77,6 +80,13 @@ 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 { + 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", + resp.Status, + contentType, + ) + } var apiError struct { Error struct { Type string `json:"type"` diff --git a/cmd/cmd_test.go b/cmd/cmd_test.go index 782be18d2..101c52c87 100644 --- a/cmd/cmd_test.go +++ b/cmd/cmd_test.go @@ -2,7 +2,10 @@ package cmd import ( "io" + "io/ioutil" + "net/http" "os" + "strings" "testing" "github.com/spf13/cobra" @@ -119,3 +122,54 @@ func (co capturedOutput) reset() { Out = co.oldOut Err = co.oldErr } + +func errorResponse(contentType string, body string) *http.Response { + response := &http.Response{ + Status: "418 I'm a teapot", + StatusCode: 418, + Header: make(http.Header), + Body: ioutil.NopCloser(strings.NewReader(body)), + ContentLength: int64(len(body)), + } + response.Header.Set("Content-Type", contentType) + return response +} + +func TestDecodeErrorResponse(t *testing.T) { + testCases := []struct { + response *http.Response + wantMessage string + }{ + { + response: errorResponse("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}}`), + 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"]}}`), + wantMessage: "message: a, b", + }, + { + response: errorResponse("application/json", `{"error": {"message": "message"}}`), + wantMessage: "message", + }, + { + response: errorResponse("application/problem+json", `{"error": {"message": "new json format"}}`), + wantMessage: "new json format", + }, + { + response: errorResponse("application/json", `{"error": {}}`), + wantMessage: "unexpected API response: 418", + }, + } + tc := testCases[0] + got := decodedAPIError(tc.response) + assert.Equal(t, tc.wantMessage, got.Error()) + for _, tc = range testCases { + got := decodedAPIError(tc.response) + assert.Equal(t, tc.wantMessage, got.Error()) + } +} diff --git a/cmd/download_test.go b/cmd/download_test.go index aadcf84aa..791be4230 100644 --- a/cmd/download_test.go +++ b/cmd/download_test.go @@ -403,7 +403,7 @@ func TestDownloadError(t *testing.T) { err = runDownload(cfg, flags, []string{}) - assert.Equal(t, "test error", err.Error()) + assert.Equal(t, `expected response with Content-Type "application/json" but got status "400 Bad Request" with Content-Type "text/plain; charset=utf-8"`, err.Error()) } diff --git a/cmd/submit_test.go b/cmd/submit_test.go index 9b645965b..7db152a75 100644 --- a/cmd/submit_test.go +++ b/cmd/submit_test.go @@ -657,7 +657,7 @@ func TestSubmitServerErr(t *testing.T) { err = runSubmit(cfg, pflag.NewFlagSet("fake", pflag.PanicOnError), files) - assert.Regexp(t, "test error", err.Error()) + assert.Regexp(t, `expected response with Content-Type "application/json" but got status "400 Bad Request" with Content-Type "text/plain; charset=utf-8"`, err.Error()) } func TestHandleErrorResponse(t *testing.T) {