diff --git a/src/chartserver/client.go b/src/chartserver/client.go index b0c5ff7fe..c96c76f73 100644 --- a/src/chartserver/client.go +++ b/src/chartserver/client.go @@ -3,6 +3,8 @@ package chartserver import ( "errors" "fmt" + commonhttp "github.com/goharbor/harbor/src/common/http" + hlog "github.com/goharbor/harbor/src/common/utils/log" "io" "io/ioutil" "net/http" @@ -45,7 +47,7 @@ func NewChartClient(credentail *Credential) *ChartClient { // Create http client // GetContent get the bytes from the specified url func (cc *ChartClient) GetContent(addr string) ([]byte, error) { - response, err := cc.sendRequest(addr, http.MethodGet, nil, []int{http.StatusOK}) + response, err := cc.sendRequest(addr, http.MethodGet, nil) if err != nil { return nil, err } @@ -56,17 +58,48 @@ func (cc *ChartClient) GetContent(addr string) ([]byte, error) { } defer response.Body.Close() + if response.StatusCode != http.StatusOK { + text, err := extractError(content) + if err != nil { + return nil, err + } + return nil, &commonhttp.Error{ + Code: response.StatusCode, + Message: text, + } + } return content, nil } // DeleteContent sends deleting request to the addr to delete content func (cc *ChartClient) DeleteContent(addr string) error { - _, err := cc.sendRequest(addr, http.MethodDelete, nil, []int{http.StatusOK}) - return err + response, err := cc.sendRequest(addr, http.MethodDelete, nil) + if err != nil { + return err + } + + content, err := ioutil.ReadAll(response.Body) + if err != nil { + return err + } + defer response.Body.Close() + + if response.StatusCode != http.StatusOK { + text, err := extractError(content) + if err != nil { + return err + } + return &commonhttp.Error{ + Code: response.StatusCode, + Message: text, + } + } + + return nil } // sendRequest sends requests to the addr with the specified spec -func (cc *ChartClient) sendRequest(addr string, method string, body io.Reader, expectedCodes []int) (*http.Response, error) { +func (cc *ChartClient) sendRequest(addr string, method string, body io.Reader) (*http.Response, error) { if len(strings.TrimSpace(addr)) == 0 { return nil, errors.New("empty url is not allowed") } @@ -88,30 +121,9 @@ func (cc *ChartClient) sendRequest(addr string, method string, body io.Reader, e response, err := cc.httpClient.Do(request) if err != nil { + hlog.Errorf("%s '%s' failed with error: %s", method, fullURI.Path, err) return nil, err } - isExpectedStatusCode := false - for _, eCode := range expectedCodes { - if eCode == response.StatusCode { - isExpectedStatusCode = true - break - } - } - - if !isExpectedStatusCode { - content, err := ioutil.ReadAll(response.Body) - if err != nil { - return nil, err - } - defer response.Body.Close() - - if err := extractError(content); err != nil { - return nil, err - } - - return nil, fmt.Errorf("%s '%s' failed with error: %s", method, fullURI.Path, content) - } - return response, nil } diff --git a/src/chartserver/utils.go b/src/chartserver/utils.go index cb9fa6750..f64148a61 100644 --- a/src/chartserver/utils.go +++ b/src/chartserver/utils.go @@ -16,22 +16,22 @@ const ( // Extract error object '{"error": "****---***"}' from the content if existing // nil error will be returned if it does exist -func extractError(content []byte) error { +func extractError(content []byte) (text string, err error) { if len(content) == 0 { - return nil + return "", nil } errorObj := make(map[string]string) - err := json.Unmarshal(content, &errorObj) + err = json.Unmarshal(content, &errorObj) if err != nil { - return nil + return "", err } if errText, ok := errorObj["error"]; ok { - return errors.New(errText) + return errText, nil } - return nil + return "", nil } // Parse the redis configuration to the beego cache pattern diff --git a/src/core/api/chart_repository.go b/src/core/api/chart_repository.go index c7370ea85..eb2f146fb 100644 --- a/src/core/api/chart_repository.go +++ b/src/core/api/chart_repository.go @@ -186,7 +186,7 @@ func (cra *ChartRepositoryAPI) ListCharts() { charts, err := chartController.ListCharts(cra.namespace) if err != nil { - cra.SendInternalServerError(err) + cra.ParseAndHandleError("fail to list charts", err) return } @@ -204,7 +204,7 @@ func (cra *ChartRepositoryAPI) ListChartVersions() { versions, err := chartController.GetChart(cra.namespace, chartName) if err != nil { - cra.SendInternalServerError(err) + cra.ParseAndHandleError("fail to get chart", err) return } @@ -234,7 +234,7 @@ func (cra *ChartRepositoryAPI) GetChartVersion() { chartVersion, err := chartController.GetChartVersionDetails(cra.namespace, chartName, version) if err != nil { - cra.SendInternalServerError(err) + cra.ParseAndHandleError("fail to get chart version", err) return } @@ -267,7 +267,7 @@ func (cra *ChartRepositoryAPI) DeleteChartVersion() { } if err := chartController.DeleteChartVersion(cra.namespace, chartName, version); err != nil { - cra.SendInternalServerError(err) + cra.ParseAndHandleError("fail to delete chart version", err) return } } @@ -360,7 +360,7 @@ func (cra *ChartRepositoryAPI) DeleteChart() { // Remove labels from all the deleting chart versions under the chart chartVersions, err := chartController.GetChart(cra.namespace, chartName) if err != nil { - cra.SendInternalServerError(err) + cra.ParseAndHandleError("fail to get chart", err) return }