Merge pull request #10626 from ywk253100/200125_handle_error

Unify the method/style to handle error in handler/middleware

api/v2.0/swagger.yaml

@@ -350,7 +350,7 @@ responses:
         description: The ID of the corresponding request for the response
         type: string
     schema:
-      $ref: '#/definitions/Error'
+      $ref: '#/definitions/Errors'
   '401':
     description: Unauthorized
     headers:
@@ -358,7 +358,7 @@ responses:
         description: The ID of the corresponding request for the response
         type: string
     schema:
-      $ref: '#/definitions/Error'
+      $ref: '#/definitions/Errors'
   '403':
     description: Forbidden
     headers:
@@ -366,7 +366,7 @@ responses:
         description: The ID of the corresponding request for the response
         type: string
     schema:
-      $ref: '#/definitions/Error'
+      $ref: '#/definitions/Errors'
   '404':
     description: Not found
     headers:
@@ -374,7 +374,7 @@ responses:
         description: The ID of the corresponding request for the response
         type: string
     schema:
-      $ref: '#/definitions/Error'
+      $ref: '#/definitions/Errors'
   '409':
     description: Conflict
     headers:
@@ -382,7 +382,7 @@ responses:
         description: The ID of the corresponding request for the response
         type: string
     schema:
-      $ref: '#/definitions/Error'
+      $ref: '#/definitions/Errors'
   '500':
     description: Internal server error
     headers:
@@ -390,8 +390,13 @@ responses:
         description: The ID of the corresponding request for the response
         type: string
     schema:
-      $ref: '#/definitions/Error'
+      $ref: '#/definitions/Errors'
 definitions:
+  Errors:
+    description: The error array that describe the errors got during the handling of request
+    type: array
+    items:
+      $ref: '#/definitions/Error'
   Error:
     description: a model for all the error response coming from harbor
     type: object
@@ -402,11 +407,6 @@ definitions:
       message:
         type: string
         description: The error message
-    x-go-type:
-      import:
-        package: "github.com/goharbor/harbor/src/internal/error"
-        alias: "ierrors"
-      type: "Error"
   Artifact:
     type: object
     properties:

src/common/api/base.go

@@ -262,6 +262,5 @@ func (b *BaseAPI) SendStatusServiceUnavailableError(err error) {
 //	]
 // }
 func (b *BaseAPI) SendError(err error) {
-	statusCode, payload := serror.APIError(err)
-	b.RenderError(statusCode, payload)
+	serror.SendError(b.Ctx.ResponseWriter, err)
 }

src/server/error/error.go

@@ -16,11 +16,14 @@ package error
 
 import (
 	"errors"
-	"net/http"
-
+	"fmt"
+	openapi "github.com/go-openapi/errors"
 	"github.com/go-openapi/runtime"
 	"github.com/go-openapi/runtime/middleware"
+	"github.com/goharbor/harbor/src/common/utils/log"
 	ierror "github.com/goharbor/harbor/src/internal/error"
+	"net/http"
+	"strings"
 )
 
 var (
@@ -28,25 +31,57 @@ var (
 		ierror.BadRequestCode:                  http.StatusBadRequest,
 		ierror.UnAuthorizedCode:                http.StatusUnauthorized,
 		ierror.ForbiddenCode:                   http.StatusForbidden,
+		ierror.DENIED:                          http.StatusForbidden,
 		ierror.NotFoundCode:                    http.StatusNotFound,
 		ierror.ConflictCode:                    http.StatusConflict,
 		ierror.PreconditionCode:                http.StatusPreconditionFailed,
 		ierror.ViolateForeignKeyConstraintCode: http.StatusPreconditionFailed,
+		ierror.PROJECTPOLICYVIOLATION:          http.StatusPreconditionFailed,
 		ierror.GeneralCode:                     http.StatusInternalServerError,
 	}
 )
 
-// APIError generates the HTTP status code and error payload based on the input err
-func APIError(err error) (int, string) {
-	return getHTTPStatusCode(ierror.ErrCode(err)), ierror.NewErrs(err).Error()
+// 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
+func SendError(w http.ResponseWriter, err error) {
+	w.Header().Set("Content-Type", "application/json; charset=utf-8")
+	statusCode, errPayload := apiError(err)
+	// the error detail is logged only, and will not be sent to the client to avoid leaking server information
+	if statusCode >= http.StatusInternalServerError {
+		log.Error(errPayload)
+		err = ierror.New(nil).WithCode(ierror.GeneralCode).WithMessage("internal server error")
+		errPayload = ierror.NewErrs(err).Error()
+	} else {
+		// only log the error whose status code < 500 when debugging to avoid log flooding
+		log.Debug(errPayload)
+	}
+	w.WriteHeader(statusCode)
+	fmt.Fprintln(w, errPayload)
 }
 
-func getHTTPStatusCode(errCode string) int {
-	statusCode, ok := codeMap[errCode]
-	if !ok {
-		statusCode = http.StatusInternalServerError
+// generates the HTTP status code based on the specified error,
+// envelops the error into an error array as the payload and return them
+func apiError(err error) (statusCode int, errPayload string) {
+	code := 0
+	var openAPIErr openapi.Error
+	if errors.As(err, &openAPIErr) {
+		// Before executing operation handler, go-swagger will bind a parameters object to a request and validate the request,
+		// it will return directly when bind and validate failed.
+		// The response format of the default ServeError implementation does not match the internal error response format.
+		// So we needed to convert the format to the internal error response format.
+		code = int(openAPIErr.Code())
+		errCode := strings.Replace(strings.ToUpper(http.StatusText(code)), " ", "_", -1)
+		err = ierror.New(nil).WithCode(errCode).WithMessage(openAPIErr.Error())
+	} else {
+		code = codeMap[ierror.ErrCode(err)]
 	}
-	return statusCode
+	if code == 0 {
+		code = http.StatusInternalServerError
+	}
+	return code, ierror.NewErrs(err).Error()
 }
 
 var _ middleware.Responder = &ErrResponder{}
@@ -58,20 +93,7 @@ type ErrResponder struct {
 
 // WriteResponse ...
 func (r *ErrResponder) WriteResponse(rw http.ResponseWriter, producer runtime.Producer) {
-	code := ierror.ErrCode(r.err)
-	rw.WriteHeader(getHTTPStatusCode(code))
-
-	var e *ierror.Error
-	if !errors.As(r.err, &e) {
-		e = &ierror.Error{
-			Code:    code,
-			Message: r.err.Error(),
-		}
-	}
-
-	if err := producer.Produce(rw, e); err != nil {
-		panic(err) // let the recovery middleware deal with this
-	}
+	SendError(rw, r.err)
 }
 
 // NewErrResponder returns responder for err

src/server/error/error_test.go

@@ -16,38 +16,51 @@ package error
 
 import (
 	"errors"
+	openapi "github.com/go-openapi/errors"
 	ierror "github.com/goharbor/harbor/src/internal/error"
 	"github.com/stretchr/testify/assert"
 	"net/http"
+	"net/http/httptest"
 	"testing"
 )
 
-func TestGetHTTPStatusCode(t *testing.T) {
-	// pre-defined error code
-	errCode := ierror.NotFoundCode
-	statusCode := getHTTPStatusCode(errCode)
-	assert.Equal(t, http.StatusNotFound, statusCode)
+func TestSendError(t *testing.T) {
+	// internal server error
+	rw := httptest.NewRecorder()
+	err := ierror.New(nil).WithCode(ierror.GeneralCode).WithMessage("unknown")
+	SendError(rw, err)
+	assert.Equal(t, http.StatusInternalServerError, rw.Code)
+	assert.Equal(t, `{"errors":[{"code":"UNKNOWN","message":"internal server error"}]}`+"\n", rw.Body.String())
 
-	// not-defined error code
-	errCode = "NOT_DEFINED_ERROR_CODE"
-	statusCode = getHTTPStatusCode(errCode)
-	assert.Equal(t, http.StatusInternalServerError, statusCode)
+	// not internal server error
+	rw = httptest.NewRecorder()
+	err = ierror.New(nil).WithCode(ierror.NotFoundCode).WithMessage("object not found")
+	SendError(rw, err)
+	assert.Equal(t, http.StatusNotFound, rw.Code)
+	assert.Equal(t, `{"errors":[{"code":"NOT_FOUND","message":"object not found"}]}`+"\n", rw.Body.String())
 }
 
 func TestAPIError(t *testing.T) {
+	var err error
+	// open API error: github.com/go-openapi/errors.Error
+	err = openapi.New(400, "bad request")
+	statusCode, payload := apiError(err)
+	assert.Equal(t, http.StatusBadRequest, statusCode)
+	assert.Equal(t, `{"errors":[{"code":"BAD_REQUEST","message":"bad request"}]}`, payload)
+
 	// ierror.Error
-	err := &ierror.Error{
+	err = &ierror.Error{
 		Cause:   nil,
 		Code:    ierror.NotFoundCode,
 		Message: "resource not found",
 	}
-	statusCode, payload := APIError(err)
+	statusCode, payload = apiError(err)
 	assert.Equal(t, http.StatusNotFound, statusCode)
 	assert.Equal(t, `{"errors":[{"code":"NOT_FOUND","message":"resource not found"}]}`, payload)
 
 	// common error
 	e := errors.New("customized error")
-	statusCode, payload = APIError(e)
+	statusCode, payload = apiError(e)
 	assert.Equal(t, http.StatusInternalServerError, statusCode)
 	assert.Equal(t, `{"errors":[{"code":"UNKNOWN","message":"customized error"}]}`, payload)
 }

src/server/middleware/artifactinfo/artifact_info.go

@@ -17,16 +17,15 @@ package artifactinfo
 import (
 	"context"
 	"fmt"
+	"github.com/goharbor/harbor/src/common/utils/log"
+	ierror "github.com/goharbor/harbor/src/internal/error"
+	serror "github.com/goharbor/harbor/src/server/error"
+	"github.com/goharbor/harbor/src/server/middleware"
+	"github.com/opencontainers/go-digest"
 	"net/http"
 	"net/url"
 	"regexp"
 	"strings"
-
-	"github.com/goharbor/harbor/src/common/utils/log"
-	ierror "github.com/goharbor/harbor/src/internal/error"
-	"github.com/goharbor/harbor/src/server/middleware"
-	reg_err "github.com/goharbor/harbor/src/server/registry/error"
-	"github.com/opencontainers/go-digest"
 )
 
 const (
@@ -58,7 +57,7 @@ func Middleware() func(http.Handler) http.Handler {
 			repo := m[middleware.RepositorySubexp]
 			pn, err := projectNameFromRepo(repo)
 			if err != nil {
-				reg_err.Handle(rw, req, ierror.BadRequestError(err))
+				serror.SendError(rw, ierror.BadRequestError(err))
 				return
 			}
 			art := &middleware.ArtifactInfo{
@@ -77,7 +76,7 @@ func Middleware() func(http.Handler) http.Handler {
 				// it's not clear in OCI spec how to handle invalid from parm
 				bmp, err := projectNameFromRepo(bmr)
 				if err != nil {
-					reg_err.Handle(rw, req, ierror.BadRequestError(err))
+					serror.SendError(rw, ierror.BadRequestError(err))
 					return
 				}
 				art.BlobMountDigest = m[blobMountDigest]

src/server/middleware/contenttrust/contenttrust.go

@@ -6,6 +6,7 @@ import (
 	"github.com/goharbor/harbor/src/core/config"
 	"github.com/goharbor/harbor/src/core/middlewares/util"
 	internal_errors "github.com/goharbor/harbor/src/internal/error"
+	serror "github.com/goharbor/harbor/src/server/error"
 	"github.com/goharbor/harbor/src/server/middleware"
 	"net/http"
 	"net/http/httptest"
@@ -28,16 +29,12 @@ func Middleware() func(http.Handler) http.Handler {
 			if rec.Result().StatusCode == http.StatusOK {
 				match, err := matchNotaryDigest(mf)
 				if err != nil {
-					pkgE := internal_errors.New(nil).WithCode(internal_errors.PROJECTPOLICYVIOLATION).WithMessage("Failed in communication with Notary please check the log")
-					msg := internal_errors.NewErrs(pkgE).Error()
-					http.Error(rw, msg, http.StatusInternalServerError)
+					serror.SendError(rw, err)
 					return
 				}
 				if !match {
-					log.Debugf("digest mismatch, failing the response.")
 					pkgE := internal_errors.New(nil).WithCode(internal_errors.PROJECTPOLICYVIOLATION).WithMessage("The image is not signed in Notary.")
-					msg := internal_errors.NewErrs(pkgE).Error()
-					http.Error(rw, msg, http.StatusPreconditionFailed)
+					serror.SendError(rw, pkgE)
 					return
 				}
 			}

src/server/middleware/immutable/deletemf.go

@@ -6,6 +6,7 @@ import (
 	"github.com/goharbor/harbor/src/api/artifact"
 	common_util "github.com/goharbor/harbor/src/common/utils"
 	internal_errors "github.com/goharbor/harbor/src/internal/error"
+	serror "github.com/goharbor/harbor/src/server/error"
 	"github.com/goharbor/harbor/src/server/middleware"
 	"net/http"
 )
@@ -18,13 +19,12 @@ func MiddlewareDelete() func(http.Handler) http.Handler {
 				var e *ErrImmutable
 				if errors.As(err, &e) {
 					pkgE := internal_errors.New(e).WithCode(internal_errors.PreconditionCode)
-					msg := internal_errors.NewErrs(pkgE).Error()
-					http.Error(rw, msg, http.StatusPreconditionFailed)
+					serror.SendError(rw, pkgE)
 					return
 				}
 				pkgE := internal_errors.New(fmt.Errorf("error occurred when to handle request in immutable handler: %v", err)).WithCode(internal_errors.GeneralCode)
-				msg := internal_errors.NewErrs(pkgE).Error()
-				http.Error(rw, msg, http.StatusInternalServerError)
+				serror.SendError(rw, pkgE)
+				return
 			}
 			next.ServeHTTP(rw, req)
 		})

src/server/middleware/immutable/pushmf.go

@@ -6,6 +6,7 @@ import (
 	"github.com/goharbor/harbor/src/api/artifact"
 	common_util "github.com/goharbor/harbor/src/common/utils"
 	internal_errors "github.com/goharbor/harbor/src/internal/error"
+	serror "github.com/goharbor/harbor/src/server/error"
 	"github.com/goharbor/harbor/src/server/middleware"
 	"net/http"
 )
@@ -18,13 +19,12 @@ func MiddlewarePush() func(http.Handler) http.Handler {
 				var e *ErrImmutable
 				if errors.As(err, &e) {
 					pkgE := internal_errors.New(e).WithCode(internal_errors.PreconditionCode)
-					msg := internal_errors.NewErrs(pkgE).Error()
-					http.Error(rw, msg, http.StatusPreconditionFailed)
+					serror.SendError(rw, pkgE)
 					return
 				}
 				pkgE := internal_errors.New(fmt.Errorf("error occurred when to handle request in immutable handler: %v", err)).WithCode(internal_errors.GeneralCode)
-				msg := internal_errors.NewErrs(pkgE).Error()
-				http.Error(rw, msg, http.StatusInternalServerError)
+				serror.SendError(rw, pkgE)
+				return
 			}
 			next.ServeHTTP(rw, req)
 		})

src/server/middleware/manifestinfo/manifest_info.go

@@ -5,8 +5,8 @@ import (
 	"github.com/goharbor/harbor/src/common/utils"
 	ierror "github.com/goharbor/harbor/src/internal/error"
 	project2 "github.com/goharbor/harbor/src/pkg/project"
+	serror "github.com/goharbor/harbor/src/server/error"
 	"github.com/goharbor/harbor/src/server/middleware"
-	reg_err "github.com/goharbor/harbor/src/server/registry/error"
 	"github.com/opencontainers/go-digest"
 	"net/http"
 	"regexp"
@@ -23,7 +23,7 @@ func Middleware() func(http.Handler) http.Handler {
 		return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
 			mf, err := parseManifestInfoFromPath(req)
 			if err != nil {
-				reg_err.Handle(rw, req, err)
+				serror.SendError(rw, err)
 				return
 			}
 			*req = *(req.WithContext(middleware.NewManifestInfoContext(req.Context(), mf)))

src/server/middleware/readonly/readonly.go

@@ -15,9 +15,9 @@
 package readonly
 
 import (
+	serror "github.com/goharbor/harbor/src/server/error"
 	"net/http"
 
-	"github.com/goharbor/harbor/src/common/utils/log"
 	"github.com/goharbor/harbor/src/core/config"
 	internal_errors "github.com/goharbor/harbor/src/internal/error"
 	"github.com/goharbor/harbor/src/server/middleware"
@@ -67,9 +67,8 @@ func MiddlewareWithConfig(config Config, skippers ...middleware.Skipper) func(ht
 
 	return middleware.New(func(w http.ResponseWriter, r *http.Request, next http.Handler) {
 		if config.ReadOnly(r) {
-			log.Warningf("The request is prohibited in readonly mode, url is: %s", r.URL.Path)
 			pkgE := internal_errors.New(nil).WithCode(internal_errors.DENIED).WithMessage("The system is in read only mode. Any modification is prohibited.")
-			http.Error(w, internal_errors.NewErrs(pkgE).Error(), http.StatusForbidden)
+			serror.SendError(w, pkgE)
 			return
 		}
 

src/server/middleware/regtoken/regtoken.go

@@ -7,8 +7,8 @@ import (
 	"github.com/goharbor/harbor/src/common/utils/log"
 	pkg_token "github.com/goharbor/harbor/src/pkg/token"
 	"github.com/goharbor/harbor/src/pkg/token/claims/registry"
+	serror "github.com/goharbor/harbor/src/server/error"
 	"github.com/goharbor/harbor/src/server/middleware"
-	reg_err "github.com/goharbor/harbor/src/server/registry/error"
 	"net/http"
 	"strings"
 )
@@ -19,7 +19,7 @@ func Middleware() func(http.Handler) http.Handler {
 		return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
 			err := parseToken(req)
 			if err != nil {
-				reg_err.Handle(rw, req, err)
+				serror.SendError(rw, err)
 				return
 			}
 			next.ServeHTTP(rw, req)

src/server/middleware/transaction/transaction.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	serror "github.com/goharbor/harbor/src/server/error"
 	"net/http"
 
 	"github.com/goharbor/harbor/src/common/utils/log"
@@ -77,7 +78,7 @@ func Middleware(skippers ...middleware.Skipper) func(http.Handler) http.Handler
 		}
 
 		if err := orm.WithTransaction(h)(r.Context()); err != nil && err != errNonSuccess {
-			log.Errorf("deal with %s request in transaction failed: %v", r.URL.Path, err)
+			err = fmt.Errorf("deal with %s request in transaction failed: %v", r.URL.Path, err)
 
 			// begin, commit or rollback transaction db error happened,
 			// reset the response and set status code to 500
@@ -85,7 +86,7 @@ func Middleware(skippers ...middleware.Skipper) func(http.Handler) http.Handler
 				log.Errorf("reset the response failed: %v", err)
 				return
 			}
-			res.WriteHeader(http.StatusInternalServerError)
+			serror.SendError(res, err)
 		}
 	}, skippers...)
 }

src/server/middleware/v2auth/auth.go

@@ -17,6 +17,7 @@ package v2auth
 import (
 	"context"
 	"fmt"
+	serror "github.com/goharbor/harbor/src/server/error"
 	"net/http"
 	"sync"
 
@@ -27,7 +28,6 @@ import (
 	"github.com/goharbor/harbor/src/core/promgr"
 	ierror "github.com/goharbor/harbor/src/internal/error"
 	"github.com/goharbor/harbor/src/server/middleware"
-	reg_err "github.com/goharbor/harbor/src/server/registry/error"
 )
 
 type reqChecker struct {
@@ -131,7 +131,7 @@ func Middleware() func(http.Handler) http.Handler {
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
 			if err := checker.check(req); err != nil {
-				reg_err.Handle(rw, req, ierror.UnauthorizedError(err).WithMessage(err.Error()))
+				serror.SendError(rw, ierror.UnauthorizedError(err).WithMessage(err.Error()))
 				return
 			}
 			next.ServeHTTP(rw, req)

src/server/middleware/vulnerable/vulnerable.go

@@ -8,6 +8,7 @@ import (
 	"github.com/goharbor/harbor/src/pkg/scan/report"
 	v1 "github.com/goharbor/harbor/src/pkg/scan/rest/v1"
 	"github.com/goharbor/harbor/src/pkg/scan/vuln"
+	serror "github.com/goharbor/harbor/src/server/error"
 	"github.com/goharbor/harbor/src/server/middleware"
 	"github.com/pkg/errors"
 	"net/http"
@@ -30,7 +31,8 @@ func Middleware() func(http.Handler) http.Handler {
 				// Invalid project ID
 				if wl.ProjectID == 0 {
 					err := errors.Errorf("project verification error: project %s", img.ProjectName)
-					sendError(err, rw)
+					pkgE := internal_errors.New(err).WithCode(internal_errors.PROJECTPOLICYVIOLATION)
+					serror.SendError(rw, pkgE)
 					return
 				}
 
@@ -52,7 +54,8 @@ func Middleware() func(http.Handler) http.Handler {
 
 				if err != nil {
 					err = errors.Wrap(err, "middleware: vulnerable handler")
-					sendError(err, rw)
+					pkgE := internal_errors.New(err).WithCode(internal_errors.PROJECTPOLICYVIOLATION)
+					serror.SendError(rw, pkgE)
 					return
 				}
 
@@ -60,7 +63,8 @@ func Middleware() func(http.Handler) http.Handler {
 				// No report yet?
 				if !ok {
 					err = errors.Errorf("no scan report existing for the artifact: %s:%s@%s", img.Repository, img.Tag, img.Digest)
-					sendError(err, rw)
+					pkgE := internal_errors.New(err).WithCode(internal_errors.PROJECTPOLICYVIOLATION)
+					serror.SendError(rw, pkgE)
 					return
 				}
 
@@ -70,7 +74,8 @@ func Middleware() func(http.Handler) http.Handler {
 				if summary.Severity.Code() >= projectVulnerableSeverity.Code() {
 					err = errors.Errorf("current image with '%q vulnerable' cannot be pulled due to configured policy in 'Prevent images with vulnerability severity of %q from running.' "+
 						"Please contact your project administrator for help'", summary.Severity, projectVulnerableSeverity)
-					sendError(err, rw)
+					pkgE := internal_errors.New(err).WithCode(internal_errors.PROJECTPOLICYVIOLATION)
+					serror.SendError(rw, pkgE)
 					return
 				}
 
@@ -110,10 +115,3 @@ func validate(req *http.Request) (bool, *middleware.ManifestInfo, vuln.Severity,
 	}
 	return true, mf, projectVulnerableSeverity, wl
 }
-
-func sendError(err error, rw http.ResponseWriter) {
-	log.Error(err)
-	pkgE := internal_errors.New(err).WithCode(internal_errors.PROJECTPOLICYVIOLATION)
-	msg := internal_errors.NewErrs(pkgE).Error()
-	http.Error(rw, msg, http.StatusPreconditionFailed)
-}

src/server/registry/catalog.go

@@ -19,7 +19,7 @@ import (
 	"fmt"
 	"github.com/goharbor/harbor/src/api/repository"
 	ierror "github.com/goharbor/harbor/src/internal/error"
-	reg_error "github.com/goharbor/harbor/src/server/registry/error"
+	serror "github.com/goharbor/harbor/src/server/error"
 	"github.com/goharbor/harbor/src/server/registry/util"
 	"net/http"
 	"sort"
@@ -47,7 +47,7 @@ func (r *repositoryHandler) ServeHTTP(w http.ResponseWriter, req *http.Request)
 		maxEntries, err = strconv.Atoi(reqQ.Get("n"))
 		if err != nil || maxEntries < 0 {
 			err := ierror.New(err).WithCode(ierror.BadRequestCode).WithMessage("the N must be a positive int type")
-			reg_error.Handle(w, req, err)
+			serror.SendError(w, err)
 			return
 		}
 	}
@@ -57,7 +57,7 @@ func (r *repositoryHandler) ServeHTTP(w http.ResponseWriter, req *http.Request)
 	// ToDo filter out the untagged repos
 	total, repoRecords, err := r.repoCtl.List(req.Context(), nil)
 	if err != nil {
-		reg_error.Handle(w, req, err)
+		serror.SendError(w, err)
 		return
 	}
 	if total <= 0 {
@@ -81,7 +81,7 @@ func (r *repositoryHandler) ServeHTTP(w http.ResponseWriter, req *http.Request)
 		lastEntryIndex := util.IndexString(repoNames, lastEntry)
 		if lastEntryIndex == -1 {
 			err := ierror.New(nil).WithCode(ierror.BadRequestCode).WithMessage(fmt.Sprintf("the last: %s should be a valid repository name.", lastEntry))
-			reg_error.Handle(w, req, err)
+			serror.SendError(w, err)
 			return
 		}
 		resRepos = repoNames[lastEntryIndex+1 : lastEntryIndex+maxEntries]
@@ -94,7 +94,7 @@ func (r *repositoryHandler) ServeHTTP(w http.ResponseWriter, req *http.Request)
 	if repoNames[len(repoNames)-1] != resRepos[len(resRepos)-1] {
 		urlStr, err := util.SetLinkHeader(req.URL.String(), maxEntries, resRepos[len(resRepos)-1])
 		if err != nil {
-			reg_error.Handle(w, req, err)
+			serror.SendError(w, err)
 			return
 		}
 		w.Header().Set("Link", urlStr)
@@ -111,7 +111,7 @@ func (r *repositoryHandler) sendResponse(w http.ResponseWriter, req *http.Reques
 	if err := enc.Encode(catalogAPIResponse{
 		Repositories: repositoryNames,
 	}); err != nil {
-		reg_error.Handle(w, req, err)
+		serror.SendError(w, err)
 		return
 	}
 }

src/server/registry/error/error.go

@@ -1,29 +0,0 @@
-// Copyright Project Harbor Authors
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//    http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package error
-
-import (
-	"github.com/goharbor/harbor/src/common/utils/log"
-	serror "github.com/goharbor/harbor/src/server/error"
-	"net/http"
-)
-
-// Handle generates the HTTP status code and error payload and writes them to the response
-func Handle(w http.ResponseWriter, req *http.Request, err error) {
-	log.Errorf("failed to handle the request %s %s: %v", req.Method, req.URL, err)
-	statusCode, payload := serror.APIError(err)
-	w.WriteHeader(statusCode)
-	w.Write([]byte(payload))
-}

src/server/registry/manifest.go

@@ -19,7 +19,7 @@ import (
 	"github.com/goharbor/harbor/src/api/repository"
 	"github.com/goharbor/harbor/src/common/utils/log"
 	"github.com/goharbor/harbor/src/internal"
-	"github.com/goharbor/harbor/src/server/registry/error"
+	serror "github.com/goharbor/harbor/src/server/error"
 	"github.com/goharbor/harbor/src/server/router"
 	"github.com/opencontainers/go-digest"
 	"net/http"
@@ -32,7 +32,7 @@ func getManifest(w http.ResponseWriter, req *http.Request) {
 	reference := router.Param(req.Context(), ":reference")
 	artifact, err := artifact.Ctl.GetByReference(req.Context(), repository, reference, nil)
 	if err != nil {
-		error.Handle(w, req, err)
+		serror.SendError(w, err)
 		return
 	}
 
@@ -53,11 +53,11 @@ func deleteManifest(w http.ResponseWriter, req *http.Request) {
 	reference := router.Param(req.Context(), ":reference")
 	art, err := artifact.Ctl.GetByReference(req.Context(), repository, reference, nil)
 	if err != nil {
-		error.Handle(w, req, err)
+		serror.SendError(w, err)
 		return
 	}
 	if err = artifact.Ctl.Delete(req.Context(), art.ID); err != nil {
-		error.Handle(w, req, err)
+		serror.SendError(w, err)
 		return
 	}
 	w.WriteHeader(http.StatusAccepted)
@@ -72,7 +72,7 @@ func putManifest(w http.ResponseWriter, req *http.Request) {
 	// make sure the repository exist before pushing the manifest
 	_, repositoryID, err := repository.Ctl.Ensure(req.Context(), repo)
 	if err != nil {
-		error.Handle(w, req, err)
+		serror.SendError(w, err)
 		return
 	}
 
@@ -100,7 +100,7 @@ func putManifest(w http.ResponseWriter, req *http.Request) {
 
 	_, _, err = artifact.Ctl.Ensure(req.Context(), repositoryID, dgt, tags...)
 	if err != nil {
-		error.Handle(w, req, err)
+		serror.SendError(w, err)
 		return
 	}
 

src/server/registry/tag.go

@@ -21,7 +21,7 @@ import (
 	"github.com/goharbor/harbor/src/api/repository"
 	ierror "github.com/goharbor/harbor/src/internal/error"
 	"github.com/goharbor/harbor/src/pkg/q"
-	reg_error "github.com/goharbor/harbor/src/server/registry/error"
+	serror "github.com/goharbor/harbor/src/server/error"
 	"github.com/goharbor/harbor/src/server/registry/util"
 	"github.com/goharbor/harbor/src/server/router"
 	"net/http"
@@ -65,7 +65,7 @@ func (t *tagHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		maxEntries, err = strconv.Atoi(reqQ.Get("n"))
 		if err != nil || maxEntries < 0 {
 			err := ierror.New(err).WithCode(ierror.BadRequestCode).WithMessage("the N must be a positive int type")
-			reg_error.Handle(w, req, err)
+			serror.SendError(w, err)
 			return
 		}
 	}
@@ -75,7 +75,7 @@ func (t *tagHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	t.repositoryName = router.Param(req.Context(), ":splat")
 	repository, err := t.repoCtl.GetByName(req.Context(), t.repositoryName)
 	if err != nil {
-		reg_error.Handle(w, req, err)
+		serror.SendError(w, err)
 		return
 	}
 
@@ -85,7 +85,7 @@ func (t *tagHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 			"RepositoryID": repository.RepositoryID,
 		}}, nil)
 	if err != nil {
-		reg_error.Handle(w, req, err)
+		serror.SendError(w, err)
 		return
 	}
 	if total == 0 {
@@ -110,7 +110,7 @@ func (t *tagHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		lastEntryIndex := util.IndexString(tagNames, lastEntry)
 		if lastEntryIndex == -1 {
 			err := ierror.New(nil).WithCode(ierror.BadRequestCode).WithMessage(fmt.Sprintf("the last: %s should be a valid tag name.", lastEntry))
-			reg_error.Handle(w, req, err)
+			serror.SendError(w, err)
 			return
 		}
 		resTags = tagNames[lastEntryIndex+1 : lastEntryIndex+maxEntries]
@@ -123,7 +123,7 @@ func (t *tagHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	if tagNames[len(tagNames)-1] != resTags[len(resTags)-1] {
 		urlStr, err := util.SetLinkHeader(req.URL.String(), maxEntries, resTags[len(resTags)-1])
 		if err != nil {
-			reg_error.Handle(w, req, err)
+			serror.SendError(w, err)
 			return
 		}
 		w.Header().Set("Link", urlStr)
@@ -140,7 +140,7 @@ func (t *tagHandler) sendResponse(w http.ResponseWriter, req *http.Request, tagN
 		Name: t.repositoryName,
 		Tags: tagNames,
 	}); err != nil {
-		reg_error.Handle(w, req, err)
+		serror.SendError(w, err)
 		return
 	}
 }

src/server/v2.0/handler/handler.go

@@ -15,16 +15,10 @@
 package handler
 
 import (
-	"encoding/json"
-	"fmt"
+	serror "github.com/goharbor/harbor/src/server/error"
+	"github.com/goharbor/harbor/src/server/v2.0/restapi"
 	"log"
 	"net/http"
-	"net/http/httptest"
-	"strings"
-
-	"github.com/go-openapi/errors"
-	ierrors "github.com/goharbor/harbor/src/internal/error"
-	"github.com/goharbor/harbor/src/server/v2.0/restapi"
 )
 
 // New returns http handler for API V2.0
@@ -42,31 +36,10 @@ func New() http.Handler {
 	return h
 }
 
-type apiError struct {
-	Code    int32  `json:"code"`
-	Message string `json:"message"`
-}
-
 // Before executing operation handler, go-swagger will bind a parameters object to a request and validate the request,
 // it will return directly when bind and validate failed.
 // The response format of the default ServeError implementation does not match the internal error response format.
 // So we needed to convert the format to the internal error response format.
 func serveError(rw http.ResponseWriter, r *http.Request, err error) {
-	w := httptest.NewRecorder()
-	errors.ServeError(w, r, err)
-
-	rw.WriteHeader(w.Code)
-	for key, values := range w.Header() {
-		for _, value := range values {
-			rw.Header().Add(key, value)
-		}
-	}
-
-	var er apiError
-	json.Unmarshal(w.Body.Bytes(), &er)
-
-	code := strings.Replace(strings.ToUpper(http.StatusText(w.Code)), " ", "_", -1)
-
-	e := ierrors.New(fmt.Errorf(er.Message)).WithCode(code)
-	rw.Write([]byte(e.Error()))
+	serror.SendError(rw, err)
 }