Return error for unhandled HTTP status codes in REST client

Previously, any status not explicitly handled (e.g. 409, 403, 500, 502)
fell through to the JSON decode path, silently producing zero-value
results. Now all >= 400 statuses return an error.

Also fix Decode(&resdata) -> Decode(resdata) to avoid unnecessary
*interface{} indirection.

Author: alxgsv

ucare/error.go

@@ -50,3 +50,11 @@ func (e reqValidationErr) Error() string {
 }
 
 type reqForbiddenErr struct{ respErr }
+
+type unexpectedStatusErr struct {
+	StatusCode int
+}
+
+func (e unexpectedStatusErr) Error() string {
+	return fmt.Sprintf("unexpected HTTP status: %d", e.StatusCode)
+}

ucare/restapi.go

@@ -157,13 +157,22 @@ try:
 		}
 		goto try
 	default:
+		if resp.StatusCode >= 400 {
+			var apiErr respErr
+			if e := json.NewDecoder(resp.Body).Decode(&apiErr); e != nil || apiErr.Details == "" {
+				resp.Body.Close()
+				return unexpectedStatusErr{StatusCode: resp.StatusCode}
+			}
+			resp.Body.Close()
+			return apiErr
+		}
 	}
 
 	if resdata == nil || reflect.ValueOf(resdata).IsNil() {
 		return nil
 	}
 
-	err = json.NewDecoder(resp.Body).Decode(&resdata)
+	err = json.NewDecoder(resp.Body).Decode(resdata)
 	if err != nil {
 		return err
 	}

ucare/restapi_test.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"io"
 	"net/http"
+	"net/http/httptest"
 	"strings"
 	"testing"
 	"time"
@@ -92,3 +93,71 @@ func TestRESTAPIClient(t *testing.T) {
 		})
 	}
 }
+
+func TestDo_UnhandledStatusWithDetail(t *testing.T) {
+	t.Parallel()
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusConflict)
+		w.Write([]byte(`{"detail":"Addon is already running for this file."}`))
+	}))
+	defer srv.Close()
+
+	client := &restAPIClient{conn: srv.Client()}
+	req, err := http.NewRequest(http.MethodPost, srv.URL+"/addons/uc_clamav_virus_scan/execute/", nil)
+	assert.NoError(t, err)
+
+	var result struct {
+		RequestID string `json:"request_id"`
+	}
+	err = client.Do(req, &result)
+
+	assert.Error(t, err)
+	var apiErr respErr
+	assert.True(t, errors.As(err, &apiErr))
+	assert.Equal(t, "Addon is already running for this file.", apiErr.Details)
+	assert.Equal(t, "", result.RequestID)
+}
+
+func TestDo_UnhandledStatusWithoutDetail(t *testing.T) {
+	t.Parallel()
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusBadGateway)
+		w.Write([]byte("Bad Gateway"))
+	}))
+	defer srv.Close()
+
+	client := &restAPIClient{conn: srv.Client()}
+	req, err := http.NewRequest(http.MethodGet, srv.URL+"/files/", nil)
+	assert.NoError(t, err)
+
+	var result map[string]string
+	err = client.Do(req, &result)
+
+	assert.Error(t, err)
+	var statusErr unexpectedStatusErr
+	assert.True(t, errors.As(err, &statusErr))
+	assert.Equal(t, http.StatusBadGateway, statusErr.StatusCode)
+}
+
+func TestDo_UnhandledStatusNilResdata(t *testing.T) {
+	t.Parallel()
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusConflict)
+		w.Write([]byte(`{"detail":"Conflict"}`))
+	}))
+	defer srv.Close()
+
+	client := &restAPIClient{conn: srv.Client()}
+	req, err := http.NewRequest(http.MethodDelete, srv.URL+"/groups/abc~1/", nil)
+	assert.NoError(t, err)
+
+	err = client.Do(req, nil)
+
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "Conflict")
+}