Slightly improve API error handling

This commit 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 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.

Author: jamlo

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 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 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

@@ -73,18 +73,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 {
@@ -115,18 +109,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
@@ -361,12 +349,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)
 	}