No returning error on UploadFiles

[malkam03]

Using the examples documented on the README.md to upload a files you can upload files but if there's an error it's not returned just logged.

Environment

go version go1.14.2 windows/amd64
Lib version 0.9.1
Personal Artifactory server

How to reproduce the error:

package main
import (
        "os"
        "fmt"

        "github.com/jfrog/jfrog-client-go/artifactory/auth"
        "github.com/jfrog/jfrog-client-go/config"
        "github.com/jfrog/jfrog-client-go/artifactory"
        "github.com/jfrog/jfrog-client-go/artifactory/services"
        "github.com/jfrog/jfrog-client-go/utils/log"
)

func main() {
    log.SetLogger(log.NewLogger(log.INFO, os.Stdout))
    rtDetails := auth.NewArtifactoryDetails()
    rtDetails.SetUrl("http://localhost:8081/artifactory")
    rtDetails.SetUser("wronguser")
    rtDetails.SetPassword("password")
    serviceConfig, err := config.NewConfigBuilder().
        SetArtDetails(rtDetails).Build()
    if err != nil {
        fmt.Printf("%v", err)
        return
    }

    rtManager, err := artifactory.New(&rtDetails, serviceConfig)
    params := services.NewUploadParams()
    params.Pattern = "/tmp/path"
    params.Target = "test/test/"
    params.Recursive = true
    params.IncludeDirs = false
    params.Regexp = false
    params.Flat = true
    info, _, _, err := rtManager.UploadFiles(params)
    if err != nil {
        fmt.Printf("Should print this error %v", err)
        return
    }
    fmt.Printf("%v", info)
}

Expected behavior:

$ go run main.go
[Info] [Thread 0] Uploading artifact: /tmp/test/test2
[Info] [Thread 2] Uploading artifact: /tmp/test/test.7z
[Error] [Thread 2] Artifactory response: 401 Unauthorized
[Error] [Thread 0] Artifactory response: 401 Unauthorized
[Error] Failed uploading 2 artifacts.
Should print this error Artifactory response: 401 Unauthorized

Actual behavior:

$ go run main.go
[Info] [Thread 0] Uploading artifact: /tmp/test/test2
[Info] [Thread 2] Uploading artifact: /tmp/test/test.7z
[Error] [Thread 2] Artifactory response: 401 Unauthorized
[Error] [Thread 0] Artifactory response: 401 Unauthorized
[Error] Failed uploading 2 artifacts.
[]

The order will not match for the threads, but in order to do error handling the error should be returned if there's one.

[alex1989hu]

Hello, I also met this issue today - started to use the library few days ago to let me able to upload files.

/cc @eyalbe4 Is it a possible solution?

diff --git a/artifactory/services/upload.go b/artifactory/services/upload.go
index 7034b12..5fe09c5 100644
--- a/artifactory/services/upload.go
+++ b/artifactory/services/upload.go
@@ -1,6 +1,7 @@
 package services

 import (
+       "errors"
        "net/http"
        "os"
        "path/filepath"
@@ -88,7 +89,9 @@ func (us *UploadService) performUploadTasks(consumer parallel.Runner, uploadSumm
        log.Debug("Uploaded", strconv.Itoa(totalUploaded), "artifacts.")
        totalFailed = totalUploadAttempted - totalUploaded
        if totalFailed > 0 {
-               log.Error("Failed uploading", strconv.Itoa(totalFailed), "artifacts.")
+               errFailed := errorutils.CheckError(errors.New("Failed uploading" + strconv.Itoa(totalFailed) + "artifacts"))
+               errorsQueue.AddError(errFailed)
+               log.Error(errFailed)
        }
        err = errorsQueue.GetError()
        return

[eyalbe4]

Thanks for reporting this issue! We're looking into it now.

[asafgabai]

Hi @malkam03 and @alex1989hu After some investigation we noticed that this issue occurs only for HTTP errors returned from Artifactory. Golang's HTTP client doesn't return an error for non-2xx responses, so jfrog-client-go will have to check the response's status code and throw an error accordingly. To make these errors easier to handle, they will be of a new struct containing the status code, status definition and maybe some more useful information. Pay attention that only the first error to be thrown is returned from the method, even if more than one error was thrown throughout the process. Nowadays I'm working on a cool new feature of archiving and uploading files to Artifactory, so I'll start implementing this solution in the next few days. Meanwhile I'll be happy to hear your thoughts about it.

[alex1989hu]

cool new feature of archiving and uploading files to Artifactory, so I'll start implementing this solution in the next few days

Hello @asafgabai, I am happy that you checked what happens inside. I think throwing the first error only is much better, than the current behaviour which ignores - due to Go HTTP Client as you wrote - a set of ~errors.

[yahavi]

@malkam03 @alex1989hu, Thanks for reporting this issue. This is resolved in 0.22.0 at https://github.com/jfrog/jfrog-client-go/pull/340/files#diff-fbe1867fd4ced5fcc9f3fd0a019edb80095566953e286e5e41e46439310ce228R437

However, I'd recommend upgrading to the latest version v1.10.0 to get better logging and a retry mechanism.

Please let me know if that helped.