don't log errors that were caused by a context timeout #688
Conversation
| @@ -141,9 +141,11 @@ func (w *Warmer) warm(id flux.ImageID, creds Credentials) { | |||
| // Get the image from the remote | |||
| img, err := client.Manifest(imageID) | |||
| if err != nil { | |||
| if !strings.Contains(err.Error(), context.DeadlineExceeded.Error()) && !strings.Contains(err.Error(), "net/http: request canceled") { | |||
| w.Logger.Log("err", errors.Wrap(err, "requesting manifests")) | |||
| if err, ok := errors.Cause(err).(net.Error); ok && err.Timeout() { | |||
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
squaremo
dismissed
their stale review
Thought of a possible way to improve on this
There was a problem hiding this comment.
It seems like we are setting a deadline for the fetches to complete. In light of that, I think it'd be better to construct a context with context.WithDeadline(...), and use that to do all the fetching etc. The error can then be directly compared to context.DeadlineExceeded to see if we care about logging it.
| @@ -141,9 +141,11 @@ func (w *Warmer) warm(id flux.ImageID, creds Credentials) { | |||
| // Get the image from the remote | |||
| img, err := client.Manifest(imageID) | |||
| if err != nil { | |||
| if !strings.Contains(err.Error(), context.DeadlineExceeded.Error()) && !strings.Contains(err.Error(), "net/http: request canceled") { | |||
| w.Logger.Log("err", errors.Wrap(err, "requesting manifests")) | |||
| if err, ok := errors.Cause(err).(net.Error); ok && err.Timeout() { | |||
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
|
This PR is an advance on what we have -- it tests for the correct kind of error to suppress. I have logged the issue #713 for replacing our use of context timeouts with a deadline. |
For each image fetch (including tags and all manifests) is limited by a context deadline of 10s. After this all remaining http requests will return an
i/o timeouttype error.We wish to allow time for the other images to go and fetch their stuff, so it was recommended that we just suppress this error and re-request the unfetched manifests on a subsequent warm.