Add retries to Bintray and go file uploads

artifactory/services/go/goutils.go

@@ -7,6 +7,8 @@ import (
 	"strings"
 )
 
+const GoUploadRetries = 3
+
 func addHeaders(params GoParams, clientDetails *httputils.HttpClientDetails) {
 	utils.AddHeader("X-GO-MODULE-VERSION", params.GetVersion(), &clientDetails.Headers)
 	utils.AddHeader("X-GO-MODULE-CONTENT", base64.StdEncoding.EncodeToString(params.GetModContent()), &clientDetails.Headers)
@@ -26,4 +28,4 @@ func createUrlPath(moduleId, version, props, extension string, url *string) erro
 		*url = tempUrl
 	}
 	return nil
-}
+}

artifactory/services/go/publishwithheaders.go

@@ -33,7 +33,7 @@ func (pwh *publishWithHeader) PublishPackage(params GoParams, client *rthttpclie
 	if err != nil {
 		return err
 	}
-	resp, _, err := client.UploadFile(params.GetZipPath(), url, "", &clientDetails, 0)
+	resp, _, err := client.UploadFile(params.GetZipPath(), url, "", &clientDetails, GoUploadRetries)
 	if err != nil {
 		return err
 	}

artifactory/services/go/publishwithmatrixparams.go

@@ -40,7 +40,7 @@ func (pwmp *publishWithMatrixParams) PublishPackage(params GoParams, client *rth
 		return err
 	}
 
-	resp, _, err := client.UploadFile(params.GetZipPath(), url, "", &clientDetails, 0)
+	resp, _, err := client.UploadFile(params.GetZipPath(), url, "", &clientDetails, GoUploadRetries)
 	if err != nil {
 		return err
 	}

artifactory/services/go/publishzipandmod.go

@@ -41,7 +41,7 @@ func (pwa *publishZipAndModApi) PublishPackage(params GoParams, client *rthttpcl
 	clientDetails := ArtDetails.CreateHttpClientDetails()
 
 	addGoVersion(params, &zipUrl)
-	resp, _, err := client.UploadFile(params.GetZipPath(), zipUrl, "", &clientDetails, 0)
+	resp, _, err := client.UploadFile(params.GetZipPath(), zipUrl, "", &clientDetails, GoUploadRetries)
 	if err != nil {
 		return err
 	}
@@ -54,7 +54,7 @@ func (pwa *publishZipAndModApi) PublishPackage(params GoParams, client *rthttpcl
 		return err
 	}
 	addGoVersion(params, &url)
-	resp, _, err = client.UploadFile(params.GetModPath(), url, "", &clientDetails, 0)
+	resp, _, err = client.UploadFile(params.GetModPath(), url, "", &clientDetails, GoUploadRetries)
 	if err != nil {
 		return err
 	}

bintray/services/download.go

@@ -20,8 +20,6 @@ import (
 	"sync"
 )
 
-const BintrayDownloadRetries = 3
-
 func NewDownloadService(client *httpclient.HttpClient) *DownloadService {
 	ds := &DownloadService{client: client}
 	return ds
@@ -217,7 +215,7 @@ func (ds *DownloadService) downloadBintrayFile(downloadParams *DownloadFileParam
 			LocalPath:     localPath,
 			LocalFileName: localFileName}
 
-		resp, err := client.DownloadFile(downloadDetails, logMsgPrefix, httpClientsDetails, BintrayDownloadRetries, false)
+		resp, err := client.DownloadFile(downloadDetails, logMsgPrefix, httpClientsDetails, utils.BintrayDownloadRetries, false)
 		if err != nil {
 			return err
 		}
@@ -231,7 +229,7 @@ func (ds *DownloadService) downloadBintrayFile(downloadParams *DownloadFileParam
 		var resp *http.Response
 		var redirectUrl string
 		resp, redirectUrl, err =
-			client.DownloadFileNoRedirect(url, localPath, localFileName, httpClientsDetails, BintrayDownloadRetries)
+			client.DownloadFileNoRedirect(url, localPath, localFileName, httpClientsDetails, utils.BintrayDownloadRetries)
 		// There are two options now. Either the file has just been downloaded as one block, or
 		// we got a redirect to DSN download URL. In case of the later, we should download the file
 		// concurrently from the DSN URL.
@@ -245,7 +243,7 @@ func (ds *DownloadService) downloadBintrayFile(downloadParams *DownloadFileParam
 				LocalPath:     localPath,
 				FileSize:      details.Size,
 				SplitCount:    ds.SplitCount,
-				Retries:       BintrayDownloadRetries}
+				Retries:       utils.BintrayDownloadRetries}
 
 			resp, err = client.DownloadFileConcurrently(concurrentDownloadFlags, "", httpClientsDetails)
 			if errorutils.CheckError(err) != nil {

bintray/services/logs/logs.go

@@ -3,7 +3,7 @@ package logs
 import (
 	"errors"
 	"github.com/jfrog/jfrog-client-go/bintray/auth"
-	"github.com/jfrog/jfrog-client-go/bintray/services"
+	"github.com/jfrog/jfrog-client-go/bintray/services/utils"
 	"github.com/jfrog/jfrog-client-go/bintray/services/versions"
 	"github.com/jfrog/jfrog-client-go/httpclient"
 	clientutils "github.com/jfrog/jfrog-client-go/utils"
@@ -62,7 +62,7 @@ func (ls *LogsService) Download(versionPath *versions.Path, logName string) erro
 		DownloadPath:  downloadUrl,
 		LocalPath:     "",
 		LocalFileName: logName}
-	resp, err := client.DownloadFile(details, "", httpClientsDetails, services.BintrayDownloadRetries, false)
+	resp, err := client.DownloadFile(details, "", httpClientsDetails, utils.BintrayDownloadRetries, false)
 	if err != nil {
 		return err
 	}

bintray/services/upload.go

@@ -3,6 +3,7 @@ package services
 import (
 	"errors"
 	"github.com/jfrog/jfrog-client-go/bintray/auth"
+	"github.com/jfrog/jfrog-client-go/bintray/services/utils"
 	"github.com/jfrog/jfrog-client-go/bintray/services/versions"
 	"github.com/jfrog/jfrog-client-go/httpclient"
 	clientutils "github.com/jfrog/jfrog-client-go/utils"
@@ -155,7 +156,7 @@ func uploadFile(artifact clientutils.Artifact, url, logMsgPrefix string, bintray
 	if err != nil {
 		return false, err
 	}
-	resp, body, err := client.UploadFile(artifact.LocalPath, url, logMsgPrefix, httpClientsDetails, 0)
+	resp, body, err := client.UploadFile(artifact.LocalPath, url, logMsgPrefix, httpClientsDetails, utils.BintrayUploadRetries)
 	if err != nil {
 		return false, err
 	}

bintray/services/utils/bintrayutils.go

@@ -7,6 +7,11 @@ import (
 	"strings"
 )
 
+const (
+	BintrayDownloadRetries = 3
+	BintrayUploadRetries   = 3
+)
+
 func ReadBintrayMessage(resp []byte) string {
 	var response bintrayResponse
 	err := json.Unmarshal(resp, &response)

httpclient/client.go

@@ -36,13 +36,6 @@ func (jc *HttpClient) sendGetForFileDownload(url string, followRedirect bool, ht
 	return
 }
 
-func getFailureReason(resp *http.Response, err error) string {
-	if resp != nil {
-		return resp.Status
-	}
-	return err.Error()
-}
-
 func (jc *HttpClient) Stream(url string, httpClientsDetails httputils.HttpClientDetails) (*http.Response, []byte, string, error) {
 	return jc.sendGetLeaveBodyOpen(url, true, httpClientsDetails)
 }
@@ -151,20 +144,22 @@ func (jc *HttpClient) UploadFile(localPath, url, logMsgPrefix string, httpClient
 		MaxRetries:      retries,
 		RetriesInterval: 0,
 		ErrorMessage:    fmt.Sprintf("Failure occurred while uploading to %s", url),
+		LogMsgPrefix:    logMsgPrefix,
 		ExecutionHandler: func() (bool, error) {
-			resp, body, err = jc.doUploadFile(localPath, url, logMsgPrefix, httpClientsDetails)
+			resp, body, err = jc.doUploadFile(localPath, url, httpClientsDetails)
 			if err != nil {
 				return true, err
 			}
-			message := resp.Status
-			if body != nil {
-				message = fmt.Sprintf("%s %s", message, body)
+			// Response must not be nil
+			if resp == nil {
+				return false, errorutils.CheckError(errors.New(fmt.Sprintf("%sReceived empty response from file upload", logMsgPrefix)))
 			}
 			// If response-code < 500, should not retry
-			if resp != nil && resp.StatusCode < 500 {
+			if resp.StatusCode < 500 {
 				return false, nil
 			}
 			// Perform retry
+			log.Warn(fmt.Sprintf("%sArtifactory response: %s", logMsgPrefix, resp.Status))
 			return true, nil
 		},
 	}
@@ -173,7 +168,7 @@ func (jc *HttpClient) UploadFile(localPath, url, logMsgPrefix string, httpClient
 	return
 }
 
-func (jc *HttpClient) doUploadFile(localPath, url, logMsgPrefix string, httpClientsDetails httputils.HttpClientDetails) (*http.Response, []byte, error) {
+func (jc *HttpClient) doUploadFile(localPath, url string, httpClientsDetails httputils.HttpClientDetails) (*http.Response, []byte, error) {
 	var file *os.File
 	var err error
 	if localPath != "" {
@@ -206,7 +201,6 @@ func (jc *HttpClient) doUploadFile(localPath, url, logMsgPrefix string, httpClie
 		return nil, nil, err
 	}
 	defer resp.Body.Close()
-	log.Info(logMsgPrefix+":", resp.Status+"...")
 	body, err := ioutil.ReadAll(resp.Body)
 	if errorutils.CheckError(err) != nil {
 		return nil, nil, err
@@ -255,11 +249,16 @@ func (jc *HttpClient) downloadFile(downloadFileDetails *DownloadFileDetails, log
 			if err != nil {
 				return true, err
 			}
+			// Response must not be nil
+			if resp == nil {
+				return false, errorutils.CheckError(errors.New(fmt.Sprintf("%sReceived empty response from file download", logMsgPrefix)))
+			}
 			// If response-code < 500, should not retry
-			if resp != nil && resp.StatusCode < 500 {
+			if resp.StatusCode < 500 {
 				return false, nil
 			}
 			// Perform retry
+			log.Warn(fmt.Sprintf("%sArtifactory response: %s", logMsgPrefix, resp.Status))
 			return true, nil
 		},
 	}
@@ -275,7 +274,6 @@ func (jc *HttpClient) doDownloadFile(downloadFileDetails *DownloadFileDetails, l
 		return
 	}
 
-	log.Info(logMsgPrefix+":", resp.Status+"...")
 	defer resp.Body.Close()
 	if resp.StatusCode != http.StatusOK {
 		return resp, redirectUrl, nil
@@ -582,11 +580,16 @@ func (jc *HttpClient) downloadFileRange(flags ConcurrentDownloadFlags, start, en
 			if err != nil {
 				return true, err
 			}
+			// Response must not be nil
+			if resp == nil {
+				return false, errorutils.CheckError(errors.New(fmt.Sprintf("%s[%s]: Received empty response from file download", logMsgPrefix, strconv.Itoa(currentSplit))))
+			}
 			// If response-code < 500, should not retry
-			if resp != nil && resp.StatusCode < 500 {
+			if resp.StatusCode < 500 {
 				return false, nil
 			}
 			// Perform retry
+			log.Warn(fmt.Sprintf("%s[%s]: Artifactory response: %s", logMsgPrefix, strconv.Itoa(currentSplit), resp.Status))
 			return true, nil
 		},
 	}
@@ -618,11 +621,11 @@ func (jc *HttpClient) doDownloadFileRange(flags ConcurrentDownloadFlags, start,
 	}
 	defer resp.Body.Close()
 
-	log.Info(logMsgPrefix+"["+strconv.Itoa(currentSplit)+"]:", resp.Status+"...")
 	// Unexpected http response
 	if resp.StatusCode != http.StatusPartialContent {
 		return
 	}
+	log.Info(fmt.Sprintf("%s[%s]: %s...", logMsgPrefix, strconv.Itoa(currentSplit), resp.Status))
 
 	err = os.MkdirAll(tempLocalPath, 0777)
 	if errorutils.CheckError(err) != nil {