Charmhub: Improve download logging on failure

The following changes aims to improve debugging when attempting to download a charm. Unfortunately we can end up with server errors that means that a download of an archive will fail. In order to not leak the client implementation out to the user, we hide the error status code, but with this logging, a user can use `juju debug-log` to at least inspect it if it's required/persisent.
sdg0919 · May 20, 2021 · d92df43 · d92df43
1 parent e1cb593
commit d92df43
Showing 1 changed file with 13 additions and 2 deletions.
diff --git a/charmhub/download.go b/charmhub/download.go
@@ -183,20 +183,31 @@ func (c *DownloadClient) downloadFromURL(ctx context.Context, resourceURL *url.U
 		return nil, errors.Annotatef(err, "cannot make new request")
 	}
 
+	c.logger.Tracef("download from URL %s", resourceURL.String())
+
 	resp, err = c.transport.Do(req)
 	if err != nil {
 		return nil, errors.Annotate(err, "cannot get archive")
 	}
-	if resp.StatusCode >= http.StatusOK && resp.StatusCode < http.StatusNoContent {
+	// If we get anything but a 200 status code, we don't know how to correctly
+	// handle that scenario. Return early and deal with the failure later on.
+	if resp.StatusCode == http.StatusOK {
 		return resp, nil
 	}
 
-	// Clean up, as we can't really offer anything of use here.
+	c.logger.Errorf("download failed from %s: response code: %s", resourceURL.String(), resp.Status)
+
+	// Ensure we drain the response body so this connection can be reused. As
+	// there is no error message, we have no ability other than to check the
+	// status codes.
 	_, _ = io.Copy(ioutil.Discard, resp.Body)
 	_ = resp.Body.Close()
 
 	if resp.StatusCode == http.StatusNotFound {
 		return nil, errors.NotFoundf("archive")
 	}
+
+	// Server error, nothing we can do other than inform the user that the
+	// archive was unaviable.
 	return nil, errors.Errorf("unable to locate archive")
 }