Error management

[edupo]

I think the error management can be improved.

My experience with cali up to now is that many errors that are generated does not bring new info:

return "", fmt.Errorf("Failed to create container: %s", err)

In this case err comes from docker API and is pretty self explanatory in the description.

Perhaps is better to do something like:

log.WithError(err).Error("Failed to create the container")
return "", err

[lucymhdavies]

I've not seen log.WithError before, but looks useful.

Testing that change:

$ git diff
diff --git a/docker.go b/docker.go
index 995f111..f9d10fe 100644
--- a/docker.go
+++ b/docker.go
@@ -191,7 +191,8 @@ func (c *DockerClient) StartContainer(rm bool, name string) (string, error) {
        }).Debug("Creating new container")

        if err := c.PullImage(c.Conf.Image); err != nil {
-               return "", fmt.Errorf("Failed to fetch image: %s", err)
+               log.WithError(err).Error("Failed to fetch image")
+               return "", err
        }
        resp, err := c.Cli.ContainerCreate(context.Background(), c.Conf, c.HostConf, c.NetConf, name)

diff --git a/task.go b/task.go
index cad2b96..084684c 100644
--- a/task.go
+++ b/task.go
@@ -22,7 +22,7 @@ var defaultTaskFunc TaskFunc = func(t *Task, args []string) {
                log.Fatalf("Error initialising Docker: %s", err)
        }
        if _, err := t.StartContainer(true, ""); err != nil {
-               log.Fatalf("Error executing task: %s", err)
+               log.WithError(err).Fatal("Error executing task")
        }
 }

An example of one of the errors with the current version:

INFO[0000] Pulling image layers... please wait           image=this-image-does-not-exist
FATA[0001] Error executing task: Failed to fetch image: API could not fetch "this-image-does-not-exist": Error response from daemon: pull access denied for this-image-does-not-exist, repository does not exist or may require 'docker login'
exit status 1

Compared to your suggestion:

INFO[0000] Pulling image layers... please wait           image=this-image-does-not-exist
ERRO[0001] Failed to fetch image                         error="API could not fetch \"this-image-does-not-exist\": Error response from daemon: pull access denied for this-image-does-not-exist, repository does not exist or may require 'docker login'"
FATA[0001] Error executing task                          error="API could not fetch \"this-image-does-not-exist\": Error response from daemon: pull access denied for this-image-does-not-exist, repository does not exist or may require 'docker login'"
exit status 1

So it seems useful, though we'd probably want to do it in such a way as we're not duplicating log output like the above.

[edupo]

I usually return the error on libraries. Is up to the app to log the error or work with it. The problem I see is that there are too many logs and perhaps we should analyze where makes real sense to log something.
I will try to come up with related PR's