fix: handle status codes properly

pkg/hub/hub.go

@@ -11,6 +11,7 @@ import (
 
 	retryablehttp "github.com/hashicorp/go-retryablehttp"
 	api "github.com/semaphoreci/artifact/pkg/api"
+	"github.com/semaphoreci/artifact/pkg/common"
 	log "github.com/sirupsen/logrus"
 )
 
@@ -89,8 +90,9 @@ func (c *Client) GenerateSignedURLs(remotePaths []string, requestType GenerateSi
 	retryClient.RetryWaitMax = 1 * time.Second
 	httpResp, err := retryClient.Do(req)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("request did not return a non-5xx response: %v", err)
 	}
+
 	err = decodeResponse(httpResp, &response)
 	if err != nil {
 		return nil, err
@@ -115,6 +117,11 @@ func createRequest(method, url, token string, reqBody interface{}) (*retryableht
 
 func decodeResponse(httpResp *http.Response, response *GenerateSignedURLsResponse) error {
 	defer httpResp.Body.Close()
+
+	if !common.IsStatusOK(httpResp.StatusCode) {
+		return fmt.Errorf("failed to generate signed URLs - hub returned %d status code", httpResp.StatusCode)
+	}
+
 	if err := json.NewDecoder(httpResp.Body).Decode(&response); err != nil {
 		return fmt.Errorf("failed to decode signed URL http response: %v", err)
 	}

pkg/hub/hub_test.go

@@ -48,37 +48,56 @@ func Test__HubClientCreation(t *testing.T) {
 
 func Test__GenerateSignedURL(t *testing.T) {
 
+	t.Run("response has invalid JSON", func(t *testing.T) {
+		noOfCalls := 0
+		mockArtifactHubServer := generateMockServer(&noOfCalls, 200, []byte(""))
+		defer mockArtifactHubServer.Close()
+
+		response, err := generateSignedURLsHelper(mockArtifactHubServer.URL)
+		assert.Nil(t, response)
+		assert.Equal(t, 1, noOfCalls)
+		if assert.NotNil(t, err) {
+			assert.Contains(t, err.Error(), "failed to decode signed URL http response")
+		}
+	})
+
 	t.Run("Retry only once when artifact hub returns 404", func(t *testing.T) {
 		noOfCalls := 0
-		mockArtifactHubServer := generateMockServer(&noOfCalls, 404)
+		mockArtifactHubServer := generateMockServer(&noOfCalls, 404, []byte("{}"))
 		defer mockArtifactHubServer.Close()
 
 		response, err := generateSignedURLsHelper(mockArtifactHubServer.URL)
 		assert.Nil(t, response)
-		assert.NotNil(t, err)
 		assert.Equal(t, 1, noOfCalls)
+		if assert.NotNil(t, err) {
+			assert.Contains(t, err.Error(), "hub returned 404 status code")
+		}
 	})
 
 	t.Run("Retry only once when artifact hub returns 401", func(t *testing.T) {
 		noOfCalls := 0
-		mockArtifactHubServer := generateMockServer(&noOfCalls, 401)
+		mockArtifactHubServer := generateMockServer(&noOfCalls, 401, []byte("{}"))
 		defer mockArtifactHubServer.Close()
 
 		response, err := generateSignedURLsHelper(mockArtifactHubServer.URL)
 		assert.Nil(t, response)
-		assert.NotNil(t, err)
 		assert.Equal(t, 1, noOfCalls)
+		if assert.NotNil(t, err) {
+			assert.Contains(t, err.Error(), "hub returned 401 status code")
+		}
 	})
 
 	t.Run("Retry 5 times when artifact hub returns 500", func(t *testing.T) {
 		noOfCalls := 0
-		mockArtifactHubServer := generateMockServer(&noOfCalls, 500)
+		mockArtifactHubServer := generateMockServer(&noOfCalls, 500, []byte("{}"))
 		defer mockArtifactHubServer.Close()
 
 		response, err := generateSignedURLsHelper(mockArtifactHubServer.URL)
 		assert.Nil(t, response)
-		assert.NotNil(t, err)
 		assert.Equal(t, 5, noOfCalls)
+		if assert.NotNil(t, err) {
+			assert.Contains(t, err.Error(), "request did not return a non-5xx response")
+		}
 	})
 }
 
@@ -91,9 +110,10 @@ func generateSignedURLsHelper(url string) (*GenerateSignedURLsResponse, error) {
 	return client.GenerateSignedURLs([]string{}, GenerateSignedURLsRequestPULL)
 }
 
-func generateMockServer(counter *int, codeToReturn int) *httptest.Server {
+func generateMockServer(counter *int, codeToReturn int, responseBody []byte) *httptest.Server {
 	return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		*counter++
 		w.WriteHeader(codeToReturn)
+		w.Write(responseBody)
 	}))
 }