fix: pouch pull image decode exception error

[HusterWan]

Signed-off-by: HusterWan zirenwan@gmail.com

1.Describe what this PR did
fix json decode error when pouch pull image error

2.Does this pull request fix one issue?
fixes #123 .

3.Describe how you did it
return error message through stream body when pouch pull image

4.Describe how to verify it
excute command line: pouch pull busybox, will print error information
pouch:

pouchd

5.Special notes for reviews
https://golang.org/pkg/net/http/#ResponseWriter

1. WriteHeader can only be called once
2. before call Write, if not call WriteHeader before, will call WriteHeader(http.StatusOK) first

there is a goroutine to send downloading progress information to client when pouch pull image, so when goroutine call Write once, the http header code will be set 200, later call WriteHeader(500) will print error: call multi WriteHeader, and 500 will not send to client.

so we should not only use http code to judge if pull is ok, but contains error information in ProgressMessage, then client can deal with error information appropriately

[pouchrobot]

We found this is your first time to contribute to Pouch, @HusterWan
👏 We really appreciate it.
Just remind that you have read the contribution guide: https://github.com/alibaba/pouch/blob/master/CONTRIBUTING.md
If you didn't, you should do that first. If done, welcome again and please enjoy hacking! 🍻

[skoowoo]

Thanks for your "PR", Reviewing....

[skoowoo]

the diff is okay to handle the errors.

There are some designs that I want to discuss them with you.

1. the format of jsonstream is a underlying object, it is visible by the jsonstream only, avoid to be called by others. Now there is only default format, but later, We will add docker's progress format .

2. the error message of pull image may be returned to CLI as same as the progress message. We can put the error message into jsonstream in ctrd.
   e.g.
   go func() {
   if err := c.fetchProgress(pctx, ongoing, stream); err != nil {
   logrus.Errorf("failed to get pull's progress: %v", err)
   }
   close(wait)

    logrus.Infof("fetch progress exited, ref: %s.", ref)
   

   }()

   // start to pull image.
   img, err := c.pullImage(ctx, ref, options)

   // cancel fetch progress befor handle error.
   cancelProgress()
   <-wait
   defer stream.Close()

   if err != nil {
            stream.WriteObject(<the error message>)  <=========== here
    return nil, err
   

   }

3. I think that We don't need to define "ProgressError" and "ProgressMessage", you can define the error field in "ProgressInfo" directly.

@HusterWan

[HusterWan]

@skoo87 Thanks for your opinion, i think all your points is reasonable, i will fix my code later

[allencloud]

Actually I do not like this block. It is so long and so hard to understand without annotations. Maybe in a follow-up, we need to refactor this a little bit to make this more readable.

[HusterWan]

@skoo87 I made a new PR, please review the code, if there has any problem, just ping me any time, thanks.

[skoowoo]

I think the blank line is superfluous.

[skoowoo]

len(status.ErrorMessage) > 0 ===> status.ErrorMessage != ""

Using len(), We will think it's a slice array and so on.

[skoowoo]

s/msgList/messages/g , the variable name does not need to contain it's type.

Others, We can combine those codes into one line.
messages := []ProgressInfo{
{
Code: xxxx,
ErrorMessage: xxxx,
}
}

[skoowoo]

Please squash your commits into single one commit, thanks. @HusterWan

[HusterWan]

Updated, Please take another look. @skoo87 Thanks

[skoowoo]

the "PR" is conflicting, please rebase it. @HusterWan

[HusterWan]

@skoo87 rebase done

[skoowoo]

LGTM