From 8cd4d3bdd2dfc4d0f87d2130fe411998b40eb0d5 Mon Sep 17 00:00:00 2001 From: Bar Belity Date: Tue, 5 Feb 2019 14:05:15 +0200 Subject: [PATCH] Add retries to Bintray and go file uploads --- artifactory/services/go/goutils.go | 4 +- artifactory/services/go/publishwithheaders.go | 2 +- .../services/go/publishwithmatrixparams.go | 2 +- artifactory/services/go/publishzipandmod.go | 4 +- bintray/services/download.go | 8 ++-- bintray/services/logs/logs.go | 4 +- bintray/services/upload.go | 3 +- bintray/services/utils/bintrayutils.go | 5 +++ httpclient/client.go | 39 ++++++++++--------- 9 files changed, 40 insertions(+), 31 deletions(-) diff --git a/artifactory/services/go/goutils.go b/artifactory/services/go/goutils.go index 3958f75df..4b77406b8 100644 --- a/artifactory/services/go/goutils.go +++ b/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 -} \ No newline at end of file +} diff --git a/artifactory/services/go/publishwithheaders.go b/artifactory/services/go/publishwithheaders.go index 3dba2537c..0942a6b25 100644 --- a/artifactory/services/go/publishwithheaders.go +++ b/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 } diff --git a/artifactory/services/go/publishwithmatrixparams.go b/artifactory/services/go/publishwithmatrixparams.go index 8f30ed799..f25a15e15 100644 --- a/artifactory/services/go/publishwithmatrixparams.go +++ b/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 } diff --git a/artifactory/services/go/publishzipandmod.go b/artifactory/services/go/publishzipandmod.go index 683faa52f..a86211f25 100644 --- a/artifactory/services/go/publishzipandmod.go +++ b/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 } diff --git a/bintray/services/download.go b/bintray/services/download.go index 1a19d4f5a..aab01b954 100644 --- a/bintray/services/download.go +++ b/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 { diff --git a/bintray/services/logs/logs.go b/bintray/services/logs/logs.go index fbb256995..22ee6004f 100644 --- a/bintray/services/logs/logs.go +++ b/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 } diff --git a/bintray/services/upload.go b/bintray/services/upload.go index cb3c5baaf..fe0792fcf 100644 --- a/bintray/services/upload.go +++ b/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 } diff --git a/bintray/services/utils/bintrayutils.go b/bintray/services/utils/bintrayutils.go index 1e5412053..63caadd0d 100644 --- a/bintray/services/utils/bintrayutils.go +++ b/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) diff --git a/httpclient/client.go b/httpclient/client.go index 86a9dff9e..6e30de2cf 100644 --- a/httpclient/client.go +++ b/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 {