Merge pull request cli#4862 from cli/api-err-409

mislav · web-flow · commit 29816b63c370 · 2021-12-07T19:37:19.000+01:00
api: handle HTTP 409 error message from the server
diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go
@@ -540,36 +540,58 @@ func parseErrorResponse(r io.Reader, statusCode int) (io.Reader, string, error)
 
 	var parsedBody struct {
 		Message string
-		Errors  []json.RawMessage
+		Errors  json.RawMessage
 	}
 	err = json.Unmarshal(b, &parsedBody)
 	if err != nil {
-		return r, "", err
+		return bodyCopy, "", err
+	}
+
+	if len(parsedBody.Errors) > 0 && parsedBody.Errors[0] == '"' {
+		var stringError string
+		if err := json.Unmarshal(parsedBody.Errors, &stringError); err != nil {
+			return bodyCopy, "", err
+		}
+		if stringError != "" {
+			if parsedBody.Message != "" {
+				return bodyCopy, fmt.Sprintf("%s (%s)", stringError, parsedBody.Message), nil
+			}
+			return bodyCopy, stringError, nil
+		}
 	}
+
 	if parsedBody.Message != "" {
 		return bodyCopy, fmt.Sprintf("%s (HTTP %d)", parsedBody.Message, statusCode), nil
 	}
 
-	type errorMessage struct {
+	if len(parsedBody.Errors) == 0 || parsedBody.Errors[0] != '[' {
+		return bodyCopy, "", nil
+	}
+
+	var errorObjects []json.RawMessage
+	if err := json.Unmarshal(parsedBody.Errors, &errorObjects); err != nil {
+		return bodyCopy, "", err
+	}
+
+	var objectError struct {
 		Message string
 	}
 	var errors []string
-	for _, rawErr := range parsedBody.Errors {
+	for _, rawErr := range errorObjects {
 		if len(rawErr) == 0 {
 			continue
 		}
 		if rawErr[0] == '{' {
-			var objectError errorMessage
 			err := json.Unmarshal(rawErr, &objectError)
 			if err != nil {
-				return r, "", err
+				return bodyCopy, "", err
 			}
 			errors = append(errors, objectError.Message)
 		} else if rawErr[0] == '"' {
 			var stringError string
 			err := json.Unmarshal(rawErr, &stringError)
 			if err != nil {
-				return r, "", err
+				return bodyCopy, "", err
 			}
 			errors = append(errors, stringError)
 		}
diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go
@@ -1284,3 +1284,85 @@ func Test_processResponse_template(t *testing.T) {
 	`), stdout.String())
 	assert.Equal(t, "", stderr.String())
 }
+
+func Test_parseErrorResponse(t *testing.T) {
+	type args struct {
+		input      string
+		statusCode int
+	}
+	tests := []struct {
+		name       string
+		args       args
+		wantErrMsg string
+		wantErr    bool
+	}{
+		{
+			name: "no error",
+			args: args{
+				input:      `{}`,
+				statusCode: 500,
+			},
+			wantErrMsg: "",
+			wantErr:    false,
+		},
+		{
+			name: "nil errors",
+			args: args{
+				input:      `{"errors":null}`,
+				statusCode: 500,
+			},
+			wantErrMsg: "",
+			wantErr:    false,
+		},
+		{
+			name: "simple error",
+			args: args{
+				input:      `{"message": "OH NOES"}`,
+				statusCode: 500,
+			},
+			wantErrMsg: "OH NOES (HTTP 500)",
+			wantErr:    false,
+		},
+		{
+			name: "errors string",
+			args: args{
+				input:      `{"message": "Conflict", "errors": "Some description"}`,
+				statusCode: 409,
+			},
+			wantErrMsg: "Some description (Conflict)",
+			wantErr:    false,
+		},
+		{
+			name: "errors array of strings",
+			args: args{
+				input:      `{"errors": ["fail1", "asplode2"]}`,
+				statusCode: 500,
+			},
+			wantErrMsg: "fail1\nasplode2",
+			wantErr:    false,
+		},
+		{
+			name: "errors array of objects",
+			args: args{
+				input:      `{"errors": [{"message":"fail1"}, {"message":"asplode2"}]}`,
+				statusCode: 500,
+			},
+			wantErrMsg: "fail1\nasplode2",
+			wantErr:    false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, got1, err := parseErrorResponse(strings.NewReader(tt.args.input), tt.args.statusCode)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("parseErrorResponse() error = %v, wantErr %v", err, tt.wantErr)
+			}
+			if gotString, _ := ioutil.ReadAll(got); tt.args.input != string(gotString) {
+				t.Errorf("parseErrorResponse() got = %q, want %q", string(gotString), tt.args.input)
+			}
+			if got1 != tt.wantErrMsg {
+				t.Errorf("parseErrorResponse() got1 = %q, want %q", got1, tt.wantErrMsg)
+			}
+		})
+	}
+}
