United error response format for management APIs (legacy and v2.0 APIs)

United error response format for management APIs (legacy and v2.0 APIs)

Signed-off-by: Wenkai Yin <yinw@vmware.com>
This commit is contained in:
Wenkai Yin 2020-03-13 17:31:41 +08:00
parent 4a97cd270d
commit c6940e8184
4 changed files with 31 additions and 27 deletions

View File

@ -16,12 +16,11 @@ package api
import (
"encoding/json"
"errors"
"fmt"
"net/http"
"strconv"
"errors"
"github.com/astaxie/beego"
"github.com/astaxie/beego/validation"
commonhttp "github.com/goharbor/harbor/src/common/http"
@ -65,18 +64,10 @@ func (b *BaseAPI) Render() error {
// RenderError provides shortcut to render http error
func (b *BaseAPI) RenderError(code int, text string) {
http.Error(b.Ctx.ResponseWriter, text, code)
}
// RenderFormattedError renders errors with well formatted style
func (b *BaseAPI) RenderFormattedError(errorCode int, errorMsg string) {
error := commonhttp.Error{
Code: errorCode,
Message: errorMsg,
}
formattedErrMsg := error.String()
log.Errorf("%s %s failed with error: %s", b.Ctx.Request.Method, b.Ctx.Request.URL.String(), formattedErrMsg)
b.RenderError(error.Code, formattedErrMsg)
serror.SendError(b.Ctx.ResponseWriter, &commonhttp.Error{
Code: code,
Message: text,
})
}
// DecodeJSONReq decodes a json request
@ -204,7 +195,7 @@ func (b *BaseAPI) ParseAndHandleError(text string, err error) {
}
log.Errorf("%s: %v", text, err)
if e, ok := err.(*commonhttp.Error); ok {
b.RenderFormattedError(e.Code, e.Message)
b.RenderError(e.Code, e.Message)
return
}
b.SendInternalServerError(errors.New(""))
@ -212,22 +203,22 @@ func (b *BaseAPI) ParseAndHandleError(text string, err error) {
// SendUnAuthorizedError sends unauthorized error to the client.
func (b *BaseAPI) SendUnAuthorizedError(err error) {
b.RenderFormattedError(http.StatusUnauthorized, err.Error())
b.RenderError(http.StatusUnauthorized, err.Error())
}
// SendConflictError sends conflict error to the client.
func (b *BaseAPI) SendConflictError(err error) {
b.RenderFormattedError(http.StatusConflict, err.Error())
b.RenderError(http.StatusConflict, err.Error())
}
// SendNotFoundError sends not found error to the client.
func (b *BaseAPI) SendNotFoundError(err error) {
b.RenderFormattedError(http.StatusNotFound, err.Error())
b.RenderError(http.StatusNotFound, err.Error())
}
// SendBadRequestError sends bad request error to the client.
func (b *BaseAPI) SendBadRequestError(err error) {
b.RenderFormattedError(http.StatusBadRequest, err.Error())
b.RenderError(http.StatusBadRequest, err.Error())
}
// SendInternalServerError sends internal server error to the client.
@ -235,23 +226,22 @@ func (b *BaseAPI) SendBadRequestError(err error) {
// When you send an internal server error to the client, you expect user to check the log
// to find out the root cause.
func (b *BaseAPI) SendInternalServerError(err error) {
log.Error(err.Error())
b.RenderFormattedError(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError))
b.RenderError(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError))
}
// SendForbiddenError sends forbidden error to the client.
func (b *BaseAPI) SendForbiddenError(err error) {
b.RenderFormattedError(http.StatusForbidden, err.Error())
b.RenderError(http.StatusForbidden, err.Error())
}
// SendPreconditionFailedError sends conflict error to the client.
func (b *BaseAPI) SendPreconditionFailedError(err error) {
b.RenderFormattedError(http.StatusPreconditionFailed, err.Error())
b.RenderError(http.StatusPreconditionFailed, err.Error())
}
// SendStatusServiceUnavailableError sends service unavailable error to the client.
func (b *BaseAPI) SendStatusServiceUnavailableError(err error) {
b.RenderFormattedError(http.StatusServiceUnavailable, err.Error())
b.RenderError(http.StatusServiceUnavailable, err.Error())
}
// SendError return the error defined in OCI spec: https://github.com/opencontainers/distribution-spec/blob/master/spec.md#errors

View File

@ -20,6 +20,7 @@ import (
openapi "github.com/go-openapi/errors"
"github.com/go-openapi/runtime"
"github.com/go-openapi/runtime/middleware"
commonhttp "github.com/goharbor/harbor/src/common/http"
"github.com/goharbor/harbor/src/common/utils/log"
ierror "github.com/goharbor/harbor/src/internal/error"
"net/http"
@ -42,8 +43,6 @@ var (
}
)
// TODO use "SendError" instead in the v1 APIs?
// SendError tries to parse the HTTP status code from the specified error, envelops it into
// an error array as the error payload and returns the code and payload to the response.
// And the error is logged as well
@ -76,6 +75,11 @@ func apiError(err error) (statusCode int, errPayload string) {
code = int(openAPIErr.Code())
errCode := strings.Replace(strings.ToUpper(http.StatusText(code)), " ", "_", -1)
err = ierror.New(nil).WithCode(errCode).WithMessage(openAPIErr.Error())
} else if legacyErr, ok := err.(*commonhttp.Error); ok {
// make sure the legacy error format is align with the new one
code = legacyErr.Code
errCode := strings.Replace(strings.ToUpper(http.StatusText(code)), " ", "_", -1)
err = ierror.New(nil).WithCode(errCode).WithMessage(legacyErr.Message)
} else {
code = codeMap[ierror.ErrCode(err)]
}

View File

@ -17,6 +17,7 @@ package error
import (
"errors"
openapi "github.com/go-openapi/errors"
commonhttp "github.com/goharbor/harbor/src/common/http"
ierror "github.com/goharbor/harbor/src/internal/error"
"github.com/stretchr/testify/assert"
"net/http"
@ -55,6 +56,15 @@ func TestAPIError(t *testing.T) {
assert.Equal(t, http.StatusBadRequest, statusCode)
assert.Equal(t, `{"errors":[{"code":"BAD_REQUEST","message":"bad request"}]}`, payload)
// legacy error
err = &commonhttp.Error{
Code: http.StatusNotFound,
Message: "not found",
}
statusCode, payload = apiError(err)
assert.Equal(t, http.StatusNotFound, statusCode)
assert.Equal(t, `{"errors":[{"code":"NOT_FOUND","message":"not found"}]}`, payload)
// ierror.Error
err = &ierror.Error{
Cause: nil,

View File

@ -59,7 +59,7 @@ class TestProjects(unittest.TestCase):
#3. Create a new project(PA) by user(UA), and fail to create a new project;
self.project.create_project(metadata = {"public": "false"}, expect_status_code = 403,
expect_response_body = "{\"code\":403,\"message\":\"Only system admin can create project\"}", **TestProjects.USER_edit_project_creation_CLIENT)
expect_response_body = "{\"errors\":[{\"code\":\"FORBIDDEN\",\"message\":\"Only system admin can create project\"}]}", **TestProjects.USER_edit_project_creation_CLIENT)
#4. Set project creation to "everyone";
self.conf.set_configurations_of_project_creation_restriction("everyone", **ADMIN_CLIENT)