Improve API error handling (#4607)

This PR tries to fix the "FromHttpResponse" function. Returning 2 errors
from a function can be confusing and uncommon in Golang, especially if
the function only returns these 2 errors.

The new code will return one error, which is always an APIError, and it
also improves the error messaging wording, adding more details in the
output.

Several more tweaks can be done to the current error message handling if
needed.

Note: this is identical to [PR
4565](#4565), though
created from a branch, and not a fork.

pkg/publicapi/apimodels/error.go

@@ -2,7 +2,6 @@ package apimodels
 
 import (
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -67,31 +66,37 @@ func (e *APIError) Error() string {
 }
 
 // Parse HTTP Resposne to APIError
-func FromHttpResponse(resp *http.Response) (*APIError, error) {
-
+func GenerateAPIErrorFromHTTPResponse(resp *http.Response) *APIError {
 	if resp == nil {
-		return nil, errors.New("response is nil, cannot be unmarsheld to APIError")
+		return NewAPIError(0, "API call error, invalid response")
 	}
 
 	defer resp.Body.Close()
 
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
-		return nil, fmt.Errorf("error reading response body: %w", err)
+		return NewAPIError(
+			resp.StatusCode,
+			fmt.Sprintf("Unable to read API call response body. Error: %q", err.Error()))
 	}
 
 	var apiErr APIError
 	err = json.Unmarshal(body, &apiErr)
 	if err != nil {
-		return nil, fmt.Errorf("error parsing response body: %w", err)
+		return NewAPIError(
+			resp.StatusCode,
+			fmt.Sprintf("Unable to parse API call response body. Error: %q. Body received: %q",
+				err.Error(),
+				string(body),
+			))
 	}
 
 	// If the JSON didn't include a status code, use the HTTP Status
 	if apiErr.HTTPStatusCode == 0 {
 		apiErr.HTTPStatusCode = resp.StatusCode
 	}
 
-	return &apiErr, nil
+	return &apiErr
 }
 
 // FromBacError converts a bacerror.Error to an APIError

pkg/publicapi/client/v2/client.go

@@ -74,18 +74,12 @@ func (c *httpClient) Get(ctx context.Context, endpoint string, in apimodels.GetR
 		return apimodels.NewUnauthorizedError("invalid token")
 	}
 
-	var apiError *apimodels.APIError
 	if resp.StatusCode != http.StatusOK {
-		apiError, err = apimodels.FromHttpResponse(resp)
-		if err != nil {
-			return err
+		if apiError := apimodels.GenerateAPIErrorFromHTTPResponse(resp); apiError != nil {
+			return apiError
 		}
 	}
 
-	if apiError != nil {
-		return apiError
-	}
-
 	defer resp.Body.Close()
 
 	if out != nil {
@@ -116,18 +110,12 @@ func (c *httpClient) write(ctx context.Context, verb, endpoint string, in apimod
 		return apimodels.ErrInvalidToken
 	}
 
-	var apiError *apimodels.APIError
 	if resp.StatusCode != http.StatusOK {
-		apiError, err = apimodels.FromHttpResponse(resp)
-		if err != nil {
-			return err
+		if apiError := apimodels.GenerateAPIErrorFromHTTPResponse(resp); apiError != nil {
+			return apiError
 		}
 	}
 
-	if apiError != nil {
-		return apiError
-	}
-
 	if out != nil {
 		if err := decodeBody(resp, &out); err != nil {
 			return err
@@ -362,12 +350,13 @@ func (c *httpClient) interceptError(ctx context.Context, err error, resp *http.R
 				WithCode(bacerrors.UnauthorizedError)
 		}
 
-		apiError, apiErr := apimodels.FromHttpResponse(resp)
-		if apiErr == nil {
+		apiError := apimodels.GenerateAPIErrorFromHTTPResponse(resp)
+		if apiError != nil {
 			return apiError.ToBacError()
 		}
 
-		return bacerrors.Wrap(apiErr, "server error").
+		return bacerrors.New("server error").
+			WithHTTPStatusCode(http.StatusInternalServerError).
 			WithCode(bacerrors.InternalError)
 	}