Merge pull request #5004 from ywk253100/180521_log

Log error message in server side
goharbor · May 22, 2018 · a15738d · a15738d
2 parents 63bb991 + 34d6e9b
commit a15738d
Show file tree

Hide file tree

Showing 7 changed files with 64 additions and 31 deletions.
diff --git a/src/ui/api/internal.go b/src/ui/api/internal.go
@@ -15,11 +15,12 @@
 package api
 
 import (
+	"net/http"
+
 	"github.com/vmware/harbor/src/common"
 	"github.com/vmware/harbor/src/common/dao"
 	"github.com/vmware/harbor/src/common/models"
 	"github.com/vmware/harbor/src/common/utils/log"
-	"net/http"
 )
 
 // InternalAPI handles request of harbor admin...
@@ -44,7 +45,8 @@ func (ia *InternalAPI) Prepare() {
 func (ia *InternalAPI) SyncRegistry() {
 	err := SyncRegistry(ia.ProjectMgr)
 	if err != nil {
-		ia.CustomAbort(http.StatusInternalServerError, "internal error")
+		ia.HandleInternalServerError(err.Error())
+		return
 	}
 }
 

diff --git a/src/ui/api/replication_job.go b/src/ui/api/replication_job.go
@@ -51,7 +51,8 @@ func (ra *RepJobAPI) Prepare() {
 	if len(ra.GetStringFromPath(":id")) != 0 {
 		id, err := ra.GetInt64FromPath(":id")
 		if err != nil {
-			ra.CustomAbort(http.StatusBadRequest, "ID is invalid")
+			ra.HandleBadRequest(fmt.Sprintf("invalid ID: %s", ra.GetStringFromPath(":id")))
+			return
 		}
 		ra.jobID = id
 	}
@@ -63,7 +64,8 @@ func (ra *RepJobAPI) List() {
 
 	policyID, err := ra.GetInt64("policy_id")
 	if err != nil || policyID <= 0 {
-		ra.CustomAbort(http.StatusBadRequest, "invalid policy_id")
+		ra.HandleBadRequest(fmt.Sprintf("invalid policy_id: %s", ra.GetString("policy_id")))
+		return
 	}
 
 	policy, err := core.GlobalController.GetPolicy(policyID)
@@ -73,7 +75,8 @@ func (ra *RepJobAPI) List() {
 	}
 
 	if policy.ID == 0 {
-		ra.CustomAbort(http.StatusNotFound, fmt.Sprintf("policy %d not found", policyID))
+		ra.HandleNotFound(fmt.Sprintf("policy %d not found", policyID))
+		return
 	}
 
 	if !ra.SecurityCtx.HasAllPerm(policy.ProjectIDs[0]) {
@@ -95,7 +98,8 @@ func (ra *RepJobAPI) List() {
 	if len(startTimeStr) != 0 {
 		i, err := strconv.ParseInt(startTimeStr, 10, 64)
 		if err != nil {
-			ra.CustomAbort(http.StatusBadRequest, "invalid start_time")
+			ra.HandleBadRequest(fmt.Sprintf("invalid start_time: %s", startTimeStr))
+			return
 		}
 		t := time.Unix(i, 0)
 		query.StartTime = &t
@@ -105,7 +109,8 @@ func (ra *RepJobAPI) List() {
 	if len(endTimeStr) != 0 {
 		i, err := strconv.ParseInt(endTimeStr, 10, 64)
 		if err != nil {
-			ra.CustomAbort(http.StatusBadRequest, "invalid end_time")
+			ra.HandleBadRequest(fmt.Sprintf("invalid end_time: %s", endTimeStr))
+			return
 		}
 		t := time.Unix(i, 0)
 		query.EndTime = &t
@@ -133,7 +138,8 @@ func (ra *RepJobAPI) List() {
 // Delete ...
 func (ra *RepJobAPI) Delete() {
 	if ra.jobID == 0 {
-		ra.CustomAbort(http.StatusBadRequest, "id is nil")
+		ra.HandleBadRequest("ID is nil")
+		return
 	}
 
 	job, err := dao.GetRepJob(ra.jobID)
@@ -143,11 +149,13 @@ func (ra *RepJobAPI) Delete() {
 	}
 
 	if job == nil {
-		ra.CustomAbort(http.StatusNotFound, fmt.Sprintf("job %d not found", ra.jobID))
+		ra.HandleNotFound(fmt.Sprintf("job %d not found", ra.jobID))
+		return
 	}
 
 	if job.Status == models.JobPending || job.Status == models.JobRunning {
-		ra.CustomAbort(http.StatusBadRequest, fmt.Sprintf("job is %s, can not be deleted", job.Status))
+		ra.HandleBadRequest(fmt.Sprintf("job is %s, can not be deleted", job.Status))
+		return
 	}
 
 	if err = dao.DeleteRepJob(ra.jobID); err != nil {
@@ -159,7 +167,8 @@ func (ra *RepJobAPI) Delete() {
 // GetLog ...
 func (ra *RepJobAPI) GetLog() {
 	if ra.jobID == 0 {
-		ra.CustomAbort(http.StatusBadRequest, "id is nil")
+		ra.HandleBadRequest("ID is nil")
+		return
 	}
 
 	job, err := dao.GetRepJob(ra.jobID)
@@ -211,7 +220,8 @@ func (ra *RepJobAPI) StopJobs() {
 	}
 
 	if policy.ID == 0 {
-		ra.CustomAbort(http.StatusNotFound, fmt.Sprintf("policy %d not found", req.PolicyID))
+		ra.HandleNotFound(fmt.Sprintf("policy %d not found", req.PolicyID))
+		return
 	}
 
 	jobs, err := dao.GetRepJobs(&models.RepJobQuery{

diff --git a/src/ui/api/replication_policy.go b/src/ui/api/replication_policy.go
@@ -58,7 +58,8 @@ func (pa *RepPolicyAPI) Get() {
 	}
 
 	if policy.ID == 0 {
-		pa.CustomAbort(http.StatusNotFound, http.StatusText(http.StatusNotFound))
+		pa.HandleNotFound(fmt.Sprintf("policy %d not found", id))
+		return
 	}
 
 	if !pa.SecurityCtx.HasAllPerm(policy.ProjectIDs[0]) {
@@ -85,7 +86,8 @@ func (pa *RepPolicyAPI) List() {
 	if len(projectIDStr) > 0 {
 		projectID, err := strconv.ParseInt(projectIDStr, 10, 64)
 		if err != nil || projectID <= 0 {
-			pa.CustomAbort(http.StatusBadRequest, "invalid project ID")
+			pa.HandleBadRequest(fmt.Sprintf("invalid project ID: %s", projectIDStr))
+			return
 		}
 		queryParam.ProjectID = projectID
 	}
@@ -183,7 +185,8 @@ func (pa *RepPolicyAPI) Put() {
 	}
 
 	if originalPolicy.ID == 0 {
-		pa.CustomAbort(http.StatusNotFound, http.StatusText(http.StatusNotFound))
+		pa.HandleNotFound(fmt.Sprintf("policy %d not found", id))
+		return
 	}
 
 	policy := &api_models.ReplicationPolicy{}
@@ -246,7 +249,8 @@ func (pa *RepPolicyAPI) Delete() {
 	}
 
 	if policy.ID == 0 {
-		pa.CustomAbort(http.StatusNotFound, http.StatusText(http.StatusNotFound))
+		pa.HandleNotFound(fmt.Sprintf("policy %d not found", id))
+		return
 	}
 
 	count, err := dao.GetTotalCountOfRepJobs(&models.RepJobQuery{

diff --git a/src/ui/api/repository.go b/src/ui/api/repository.go
@@ -228,17 +228,19 @@ func (ra *RepositoryAPI) Delete() {
 	if len(tag) == 0 {
 		tagList, err := rc.ListTag()
 		if err != nil {
+			log.Errorf("error occurred while listing tags of %s: %v", repoName, err)
+
 			if regErr, ok := err.(*registry_error.HTTPError); ok {
 				ra.CustomAbort(regErr.StatusCode, regErr.Detail)
 			}
 
-			log.Errorf("error occurred while listing tags of %s: %v", repoName, err)
 			ra.CustomAbort(http.StatusInternalServerError, "internal error")
 		}
 
 		// TODO remove the logic if the bug of registry is fixed
 		if len(tagList) == 0 {
-			ra.CustomAbort(http.StatusNotFound, http.StatusText(http.StatusNotFound))
+			ra.HandleNotFound(fmt.Sprintf("no tags found for repository %s", repoName))
+			return
 		}
 
 		tags = append(tags, tagList...)
@@ -279,6 +281,7 @@ func (ra *RepositoryAPI) Delete() {
 				if regErr.StatusCode == http.StatusNotFound {
 					continue
 				}
+				log.Errorf("failed to delete tag %s: %v", t, err)
 				ra.CustomAbort(regErr.StatusCode, regErr.Detail)
 			}
 			log.Errorf("error occurred while deleting tag %s:%s: %v", repoName, t, err)
@@ -588,7 +591,8 @@ func (ra *RepositoryAPI) GetManifests() {
 	}
 
 	if version != "v1" && version != "v2" {
-		ra.CustomAbort(http.StatusBadRequest, "version should be v1 or v2")
+		ra.HandleBadRequest("version should be v1 or v2")
+		return
 	}
 
 	projectName, _ := utils.ParseRepository(repoName)
@@ -622,11 +626,12 @@ func (ra *RepositoryAPI) GetManifests() {
 
 	manifest, err := getManifest(rc, tag, version)
 	if err != nil {
+		log.Errorf("error occurred while getting manifest of %s:%s: %v", repoName, tag, err)
+
 		if regErr, ok := err.(*registry_error.HTTPError); ok {
 			ra.CustomAbort(regErr.StatusCode, regErr.Detail)
 		}
 
-		log.Errorf("error occurred while getting manifest of %s:%s: %v", repoName, tag, err)
 		ra.CustomAbort(http.StatusInternalServerError, "internal error")
 	}
 
@@ -680,7 +685,8 @@ func getManifest(client *registry.Repository,
 func (ra *RepositoryAPI) GetTopRepos() {
 	count, err := ra.GetInt("count", 10)
 	if err != nil || count <= 0 {
-		ra.CustomAbort(http.StatusBadRequest, "invalid count")
+		ra.HandleBadRequest(fmt.Sprintf("invalid count: %s", ra.GetString("count")))
+		return
 	}
 
 	projectIDs := []int64{}

diff --git a/src/ui/api/scan_job.go b/src/ui/api/scan_job.go
@@ -42,7 +42,8 @@ func (sj *ScanJobAPI) Prepare() {
 	}
 	id, err := sj.GetInt64FromPath(":id")
 	if err != nil {
-		sj.CustomAbort(http.StatusBadRequest, "ID is invalid")
+		sj.HandleBadRequest("invalid ID")
+		return
 	}
 	sj.jobID = id
 

diff --git a/src/ui/api/target.go b/src/ui/api/target.go
@@ -135,7 +135,8 @@ func (t *TargetAPI) Get() {
 	}
 
 	if target == nil {
-		t.CustomAbort(http.StatusNotFound, http.StatusText(http.StatusNotFound))
+		t.HandleNotFound(fmt.Sprintf("target %d not found", id))
+		return
 	}
 
 	target.Password = ""
@@ -174,7 +175,8 @@ func (t *TargetAPI) Post() {
 	}
 
 	if ta != nil {
-		t.CustomAbort(http.StatusConflict, "name is already used")
+		t.HandleConflict("name is already used")
+		return
 	}
 
 	ta, err = dao.GetRepTargetByEndpoint(target.URL)
@@ -184,7 +186,8 @@ func (t *TargetAPI) Post() {
 	}
 
 	if ta != nil {
-		t.CustomAbort(http.StatusConflict, fmt.Sprintf("the target whose endpoint is %s already exists", target.URL))
+		t.HandleConflict(fmt.Sprintf("the target whose endpoint is %s already exists", target.URL))
+		return
 	}
 
 	if len(target.Password) != 0 {
@@ -215,7 +218,8 @@ func (t *TargetAPI) Put() {
 	}
 
 	if target == nil {
-		t.CustomAbort(http.StatusNotFound, http.StatusText(http.StatusNotFound))
+		t.HandleNotFound(fmt.Sprintf("target %d not found", id))
+		return
 	}
 
 	if len(target.Password) != 0 {
@@ -264,7 +268,8 @@ func (t *TargetAPI) Put() {
 		}
 
 		if ta != nil {
-			t.CustomAbort(http.StatusConflict, "name is already used")
+			t.HandleConflict("name is already used")
+			return
 		}
 	}
 
@@ -276,7 +281,8 @@ func (t *TargetAPI) Put() {
 		}
 
 		if ta != nil {
-			t.CustomAbort(http.StatusConflict, fmt.Sprintf("the target whose endpoint is %s already exists", target.URL))
+			t.HandleConflict(fmt.Sprintf("the target whose endpoint is %s already exists", target.URL))
+			return
 		}
 	}
 
@@ -305,7 +311,8 @@ func (t *TargetAPI) Delete() {
 	}
 
 	if target == nil {
-		t.CustomAbort(http.StatusNotFound, http.StatusText(http.StatusNotFound))
+		t.HandleNotFound(fmt.Sprintf("target %d not found", id))
+		return
 	}
 
 	policies, err := dao.GetRepPolicyByTarget(id)
@@ -315,6 +322,7 @@ func (t *TargetAPI) Delete() {
 	}
 
 	if len(policies) > 0 {
+		log.Error("the target is used by policies, can not be deleted")
 		t.CustomAbort(http.StatusPreconditionFailed, "the target is used by policies, can not be deleted")
 	}
 
@@ -346,7 +354,8 @@ func (t *TargetAPI) ListPolicies() {
 	}
 
 	if target == nil {
-		t.CustomAbort(http.StatusNotFound, http.StatusText(http.StatusNotFound))
+		t.HandleNotFound(fmt.Sprintf("target %d not found", id))
+		return
 	}
 
 	policies, err := dao.GetRepPolicyByTarget(id)

diff --git a/src/ui/api/user.go b/src/ui/api/user.go
@@ -292,7 +292,8 @@ func (ua *UserAPI) ChangePassword() {
 	}
 
 	if req.NewPassword == "" {
-		ua.CustomAbort(http.StatusBadRequest, "please_input_new_password")
+		ua.HandleBadRequest("new password is null")
+		return
 	}
 	updateUser := models.User{UserID: ua.userID, Password: req.NewPassword, Salt: user.Salt}
 	err = dao.ChangeUserPassword(updateUser, req.OldPassword)