unified API error response

13 files changed, 61 insertions, 67 deletions

diff --git a/cmd/web.go b/cmd/web.go
index 0c0eea3f81..30fd4b7957 100644
--- a/cmd/web.go
+++ b/cmd/web.go
@@ -226,7 +226,7 @@ func runWeb(ctx *cli.Context) {
 
 				m.Group("", func() {
 					m.Post("/migrate", bindIgnErr(auth.MigrateRepoForm{}), v1.MigrateRepo)
-					m.Delete("/:owner/:reponame", v1.RemoveRepo)
+					m.Delete("/:username/:reponame", v1.DeleteRepo)
 				}, middleware.ApiReqToken())
 
 				m.Group("/:username/:reponame", func() {
@@ -239,7 +239,7 @@ func runWeb(ctx *cli.Context) {
 			})
 
 			m.Any("/*", func(ctx *middleware.Context) {
-				ctx.HandleAPI(404, "Page not found")
+				ctx.Error(404)
 			})
 		})
 	}, ignSignIn)
diff --git a/gogs.go b/gogs.go
index 15bc69ba5a..e4c072d676 100644
--- a/gogs.go
+++ b/gogs.go
@@ -17,7 +17,7 @@ import (
 	"github.com/gogits/gogs/modules/setting"
 )
 
-const APP_VER = "0.6.16.1007 Beta"
+const APP_VER = "0.6.16.1008 Beta"
 
 func init() {
 	runtime.GOMAXPROCS(runtime.NumCPU())
diff --git a/modules/base/base.go b/modules/base/base.go
index 47a4137c97..864ede055f 100644
--- a/modules/base/base.go
+++ b/modules/base/base.go
@@ -8,11 +8,6 @@ const DOC_URL = "https://github.com/gogits/go-gogs-client/wiki"
 
 type (
 	TplName string
-
-	ApiJsonErr struct {
-		Message string `json:"message"`
-		DocUrl  string `json:"url"`
-	}
 )
 
 var GoGetMetas = make(map[string]bool)
diff --git a/modules/middleware/auth.go b/modules/middleware/auth.go
index 4f92c8f8e3..be8db35733 100644
--- a/modules/middleware/auth.go
+++ b/modules/middleware/auth.go
@@ -95,7 +95,7 @@ func Toggle(options *ToggleOptions) macaron.Handler {
 			if !ctx.IsSigned {
 				// Restrict API calls with error message.
 				if auth.IsAPIPath(ctx.Req.URL.Path) {
-					ctx.HandleAPI(403, "Only signed in user is allowed to call APIs.")
+					ctx.APIError(403, "", "Only signed in user is allowed to call APIs.")
 					return
 				}
 
diff --git a/modules/middleware/context.go b/modules/middleware/context.go
index 141e8ace40..c08f84925e 100644
--- a/modules/middleware/context.go
+++ b/modules/middleware/context.go
@@ -157,15 +157,22 @@ func (ctx *Context) HandleText(status int, title string) {
 	ctx.RenderData(status, []byte(title))
 }
 
-func (ctx *Context) HandleAPI(status int, obj interface{}) {
+// APIError logs error with title if status is 500.
+func (ctx *Context) APIError(status int, title string, obj interface{}) {
 	var message string
 	if err, ok := obj.(error); ok {
 		message = err.Error()
 	} else {
 		message = obj.(string)
 	}
+
+	if status == 500 {
+		log.Error(4, "%s: %s", title, message)
+	}
+
 	ctx.JSON(status, map[string]string{
 		"message": message,
+		"url":     base.DOC_URL,
 	})
 }
 
diff --git a/modules/middleware/repo.go b/modules/middleware/repo.go
index 1d1723445d..0b519a6bdb 100644
--- a/modules/middleware/repo.go
+++ b/modules/middleware/repo.go
@@ -14,7 +14,6 @@ import (
 	"github.com/mssola/user_agent"
 
 	"github.com/gogits/gogs/models"
-	"github.com/gogits/gogs/modules/base"
 	"github.com/gogits/gogs/modules/git"
 	"github.com/gogits/gogs/modules/log"
 	"github.com/gogits/gogs/modules/setting"
@@ -44,7 +43,7 @@ func ApiRepoAssignment() macaron.Handler {
 				if models.IsErrUserNotExist(err) {
 					ctx.Error(404)
 				} else {
-					ctx.JSON(500, &base.ApiJsonErr{"GetUserByName: " + err.Error(), base.DOC_URL})
+					ctx.APIError(500, "GetUserByName", err)
 				}
 				return
 			}
@@ -57,17 +56,17 @@ func ApiRepoAssignment() macaron.Handler {
 			if models.IsErrRepoNotExist(err) {
 				ctx.Error(404)
 			} else {
-				ctx.JSON(500, &base.ApiJsonErr{"GetRepositoryByName: " + err.Error(), base.DOC_URL})
+				ctx.APIError(500, "GetRepositoryByName", err)
 			}
 			return
 		} else if err = repo.GetOwner(); err != nil {
-			ctx.JSON(500, &base.ApiJsonErr{"GetOwner: " + err.Error(), base.DOC_URL})
+			ctx.APIError(500, "GetOwner", err)
 			return
 		}
 
 		mode, err := models.AccessLevel(ctx.User, repo)
 		if err != nil {
-			ctx.JSON(500, &base.ApiJsonErr{"AccessLevel: " + err.Error(), base.DOC_URL})
+			ctx.APIError(500, "AccessLevel", err)
 			return
 		}
 
diff --git a/routers/api/v1/miscellaneous.go b/routers/api/v1/miscellaneous.go
index 0da5dc1567..7ffce85761 100644
--- a/routers/api/v1/miscellaneous.go
+++ b/routers/api/v1/miscellaneous.go
@@ -16,7 +16,7 @@ import (
 // Render an arbitrary Markdown document.
 func Markdown(ctx *middleware.Context, form apiv1.MarkdownForm) {
 	if ctx.HasApiError() {
-		ctx.JSON(422, base.ApiJsonErr{ctx.GetErrMsg(), base.DOC_URL})
+		ctx.APIError(422, "", ctx.GetErrMsg())
 		return
 	}
 
@@ -38,7 +38,7 @@ func Markdown(ctx *middleware.Context, form apiv1.MarkdownForm) {
 func MarkdownRaw(ctx *middleware.Context) {
 	body, err := ctx.Req.Body().Bytes()
 	if err != nil {
-		ctx.JSON(422, base.ApiJsonErr{err.Error(), base.DOC_URL})
+		ctx.APIError(422, "", err)
 		return
 	}
 	ctx.Write(base.RenderRawMarkdown(body, ""))
diff --git a/routers/api/v1/repo.go b/routers/api/v1/repo.go
index 9fd2f92a1a..9cdb16f846 100644
--- a/routers/api/v1/repo.go
+++ b/routers/api/v1/repo.go
@@ -15,7 +15,6 @@ import (
 
 	"github.com/gogits/gogs/models"
 	"github.com/gogits/gogs/modules/auth"
-	"github.com/gogits/gogs/modules/base"
 	"github.com/gogits/gogs/modules/log"
 	"github.com/gogits/gogs/modules/middleware"
 	"github.com/gogits/gogs/modules/setting"
@@ -104,14 +103,14 @@ func SearchRepos(ctx *middleware.Context) {
 func ListMyRepos(ctx *middleware.Context) {
 	ownRepos, err := models.GetRepositories(ctx.User.Id, true)
 	if err != nil {
-		ctx.JSON(500, &base.ApiJsonErr{"GetRepositories: " + err.Error(), base.DOC_URL})
+		ctx.APIError(500, "GetRepositories", err)
 		return
 	}
 	numOwnRepos := len(ownRepos)
 
 	accessibleRepos, err := ctx.User.GetAccessibleRepositories()
 	if err != nil {
-		ctx.JSON(500, &base.ApiJsonErr{"GetAccessibleRepositories: " + err.Error(), base.DOC_URL})
+		ctx.APIError(500, "GetAccessibleRepositories", err)
 		return
 	}
 
@@ -147,15 +146,14 @@ func createRepo(ctx *middleware.Context, owner *models.User, opt api.CreateRepoO
 		if models.IsErrRepoAlreadyExist(err) ||
 			models.IsErrNameReserved(err) ||
 			models.IsErrNamePatternNotAllowed(err) {
-			ctx.JSON(422, &base.ApiJsonErr{err.Error(), base.DOC_URL})
+			ctx.APIError(422, "", err)
 		} else {
-			log.Error(4, "CreateRepository: %v", err)
 			if repo != nil {
 				if err = models.DeleteRepository(ctx.User.Id, repo.ID); err != nil {
 					log.Error(4, "DeleteRepository: %v", err)
 				}
 			}
-			ctx.Error(500)
+			ctx.APIError(500, "CreateRepository", err)
 		}
 		return
 	}
@@ -167,7 +165,7 @@ func createRepo(ctx *middleware.Context, owner *models.User, opt api.CreateRepoO
 func CreateRepo(ctx *middleware.Context, opt api.CreateRepoOption) {
 	// Shouldn't reach this condition, but just in case.
 	if ctx.User.IsOrganization() {
-		ctx.JSON(422, "not allowed creating repository for organization")
+		ctx.APIError(422, "", "not allowed creating repository for organization")
 		return
 	}
 	createRepo(ctx, ctx.User, opt)
@@ -177,15 +175,15 @@ func CreateOrgRepo(ctx *middleware.Context, opt api.CreateRepoOption) {
 	org, err := models.GetOrgByName(ctx.Params(":org"))
 	if err != nil {
 		if models.IsErrUserNotExist(err) {
-			ctx.Error(404)
+			ctx.APIError(422, "", err)
 		} else {
-			ctx.Error(500)
+			ctx.APIError(500, "GetOrgByName", err)
 		}
 		return
 	}
 
 	if !org.IsOwnedBy(ctx.User.Id) {
-		ctx.Error(403)
+		ctx.APIError(403, "", "Given user is not owner of organization.")
 		return
 	}
 	createRepo(ctx, org, opt)
@@ -198,9 +196,9 @@ func MigrateRepo(ctx *middleware.Context, form auth.MigrateRepoForm) {
 		org, err := models.GetUserByID(form.Uid)
 		if err != nil {
 			if models.IsErrUserNotExist(err) {
-				ctx.HandleAPI(422, err)
+				ctx.APIError(422, "", err)
 			} else {
-				ctx.HandleAPI(500, err)
+				ctx.APIError(500, "GetUserByID", err)
 			}
 			return
 		}
@@ -208,14 +206,14 @@ func MigrateRepo(ctx *middleware.Context, form auth.MigrateRepoForm) {
 	}
 
 	if ctx.HasError() {
-		ctx.HandleAPI(422, ctx.GetErrMsg())
+		ctx.APIError(422, "", ctx.GetErrMsg())
 		return
 	}
 
 	if ctxUser.IsOrganization() {
 		// Check ownership of organization.
 		if !ctxUser.IsOwnedBy(ctx.User.Id) {
-			ctx.HandleAPI(403, "Given user is not owner of organization.")
+			ctx.APIError(403, "", "Given user is not owner of organization.")
 			return
 		}
 	}
@@ -227,7 +225,7 @@ func MigrateRepo(ctx *middleware.Context, form auth.MigrateRepoForm) {
 		strings.HasPrefix(form.CloneAddr, "git://") {
 		u, err := url.Parse(form.CloneAddr)
 		if err != nil {
-			ctx.HandleAPI(422, err)
+			ctx.APIError(422, "", err)
 			return
 		}
 		if len(form.AuthUsername) > 0 || len(form.AuthPassword) > 0 {
@@ -235,7 +233,7 @@ func MigrateRepo(ctx *middleware.Context, form auth.MigrateRepoForm) {
 		}
 		remoteAddr = u.String()
 	} else if !com.IsDir(remoteAddr) {
-		ctx.HandleAPI(422, "Invalid local path, it does not exist or not a directory.")
+		ctx.APIError(422, "", "Invalid local path, it does not exist or not a directory.")
 		return
 	}
 
@@ -246,7 +244,7 @@ func MigrateRepo(ctx *middleware.Context, form auth.MigrateRepoForm) {
 				log.Error(4, "DeleteRepository: %v", errDelete)
 			}
 		}
-		ctx.HandleAPI(500, err)
+		ctx.APIError(500, "MigrateRepository", err)
 		return
 	}
 
@@ -254,13 +252,13 @@ func MigrateRepo(ctx *middleware.Context, form auth.MigrateRepoForm) {
 	ctx.JSON(201, ToApiRepository(ctxUser, repo, api.Permission{true, true, true}))
 }
 
-func RemoveRepo(ctx *middleware.Context) {
-	user, err := models.GetUserByName(ctx.Params(":owner"))
+func DeleteRepo(ctx *middleware.Context) {
+	user, err := models.GetUserByName(ctx.Params(":username"))
 	if err != nil {
 		if models.IsErrUserNotExist(err) {
-			ctx.HandleAPI(404, err)
+			ctx.APIError(422, "", err)
 		} else {
-			ctx.JSON(500, &base.ApiJsonErr{"GetUserByName: " + err.Error(), base.DOC_URL})
+			ctx.APIError(500, "GetUserByName", err)
 		}
 		return
 	}
@@ -268,21 +266,20 @@ func RemoveRepo(ctx *middleware.Context) {
 	repo, err := models.GetRepositoryByName(user.Id, ctx.Params(":reponame"))
 	if err != nil {
 		if models.IsErrRepoNotExist(err) {
-			ctx.HandleAPI(404, err)
+			ctx.Error(404)
 		} else {
-			ctx.JSON(500, &base.ApiJsonErr{"GetRepositoryByName: " + err.Error(), base.DOC_URL})
+			ctx.APIError(500, "GetRepositoryByName", err)
 		}
 		return
 	}
 
 	if user.IsOrganization() && !user.IsOwnedBy(ctx.User.Id) {
-		ctx.HandleAPI(403, "Given user is not owner of organization.")
+		ctx.APIError(403, "", "Given user is not owner of organization.")
 		return
 	}
 
 	if err := models.DeleteRepository(user.Id, repo.ID); err != nil {
-		log.Error(4, "DeleteRespository: %v:", err)
-		ctx.HandleAPI(500, err)
+		ctx.APIError(500, "DeleteRepository", err)
 		return
 	}
 
diff --git a/routers/api/v1/repo_file.go b/routers/api/v1/repo_file.go
index 540cd32fa8..3b2225e669 100644
--- a/routers/api/v1/repo_file.go
+++ b/routers/api/v1/repo_file.go
@@ -6,7 +6,6 @@ package v1
 
 import (
 	"github.com/gogits/gogs/models"
-	"github.com/gogits/gogs/modules/base"
 	"github.com/gogits/gogs/modules/git"
 	"github.com/gogits/gogs/modules/middleware"
 	"github.com/gogits/gogs/routers/repo"
@@ -23,12 +22,12 @@ func GetRepoRawFile(ctx *middleware.Context) {
 		if err == git.ErrNotExist {
 			ctx.Error(404)
 		} else {
-			ctx.JSON(500, &base.ApiJsonErr{"GetBlobByPath: " + err.Error(), base.DOC_URL})
+			ctx.APIError(500, "GetBlobByPath", err)
 		}
 		return
 	}
 	if err = repo.ServeBlob(ctx, blob); err != nil {
-		ctx.JSON(500, &base.ApiJsonErr{"ServeBlob: " + err.Error(), base.DOC_URL})
+		ctx.APIError(500, "ServeBlob", err)
 	}
 }
 
@@ -36,7 +35,7 @@ func GetRepoArchive(ctx *middleware.Context) {
 	repoPath := models.RepoPath(ctx.Params(":username"), ctx.Params(":reponame"))
 	gitRepo, err := git.OpenRepository(repoPath)
 	if err != nil {
-		ctx.Handle(500, "RepoAssignment Invalid repo: "+repoPath, err)
+		ctx.APIError(500, "OpenRepository", err)
 		return
 	}
 	ctx.Repo.GitRepo = gitRepo
diff --git a/routers/api/v1/repo_hooks.go b/routers/api/v1/repo_hooks.go
index 020ac7e2f2..91547cf16f 100644
--- a/routers/api/v1/repo_hooks.go
+++ b/routers/api/v1/repo_hooks.go
@@ -13,7 +13,6 @@ import (
 	api "github.com/gogits/go-gogs-client"
 
 	"github.com/gogits/gogs/models"
-	"github.com/gogits/gogs/modules/base"
 	"github.com/gogits/gogs/modules/middleware"
 )
 
@@ -47,7 +46,7 @@ func ToApiHook(repoLink string, w *models.Webhook) *api.Hook {
 func ListRepoHooks(ctx *middleware.Context) {
 	hooks, err := models.GetWebhooksByRepoId(ctx.Repo.Repository.ID)
 	if err != nil {
-		ctx.JSON(500, &base.ApiJsonErr{"GetWebhooksByRepoId: " + err.Error(), base.DOC_URL})
+		ctx.APIError(500, "GetWebhooksByRepoId", err)
 		return
 	}
 
@@ -62,17 +61,17 @@ func ListRepoHooks(ctx *middleware.Context) {
 // https://github.com/gogits/go-gogs-client/wiki/Repositories#create-a-hook
 func CreateRepoHook(ctx *middleware.Context, form api.CreateHookOption) {
 	if !models.IsValidHookTaskType(form.Type) {
-		ctx.JSON(422, &base.ApiJsonErr{"invalid hook type", base.DOC_URL})
+		ctx.APIError(422, "", "Invalid hook type")
 		return
 	}
 	for _, name := range []string{"url", "content_type"} {
 		if _, ok := form.Config[name]; !ok {
-			ctx.JSON(422, &base.ApiJsonErr{"missing config option: " + name, base.DOC_URL})
+			ctx.APIError(422, "", "Missing config option: "+name)
 			return
 		}
 	}
 	if !models.IsValidHookContentType(form.Config["content_type"]) {
-		ctx.JSON(422, &base.ApiJsonErr{"invalid content type", base.DOC_URL})
+		ctx.APIError(422, "", "Invalid content type")
 		return
 	}
 
@@ -97,7 +96,7 @@ func CreateRepoHook(ctx *middleware.Context, form api.CreateHookOption) {
 	if w.HookTaskType == models.SLACK {
 		channel, ok := form.Config["channel"]
 		if !ok {
-			ctx.JSON(422, &base.ApiJsonErr{"missing config option: channel", base.DOC_URL})
+			ctx.APIError(422, "", "Missing config option: channel")
 			return
 		}
 		meta, err := json.Marshal(&models.SlackMeta{
@@ -107,17 +106,17 @@ func CreateRepoHook(ctx *middleware.Context, form api.CreateHookOption) {
 			Color:    form.Config["color"],
 		})
 		if err != nil {
-			ctx.JSON(500, &base.ApiJsonErr{"slack: JSON marshal failed: " + err.Error(), base.DOC_URL})
+			ctx.APIError(500, "slack: JSON marshal failed", err)
 			return
 		}
 		w.Meta = string(meta)
 	}
 
 	if err := w.UpdateEvent(); err != nil {
-		ctx.JSON(500, &base.ApiJsonErr{"UpdateEvent: " + err.Error(), base.DOC_URL})
+		ctx.APIError(500, "UpdateEvent", err)
 		return
 	} else if err := models.CreateWebhook(w); err != nil {
-		ctx.JSON(500, &base.ApiJsonErr{"CreateWebhook: " + err.Error(), base.DOC_URL})
+		ctx.APIError(500, "CreateWebhook", err)
 		return
 	}
 
@@ -128,7 +127,7 @@ func CreateRepoHook(ctx *middleware.Context, form api.CreateHookOption) {
 func EditRepoHook(ctx *middleware.Context, form api.EditHookOption) {
 	w, err := models.GetWebhookByID(ctx.ParamsInt64(":id"))
 	if err != nil {
-		ctx.JSON(500, &base.ApiJsonErr{"GetWebhookById: " + err.Error(), base.DOC_URL})
+		ctx.APIError(500, "GetWebhookById", err)
 		return
 	}
 
@@ -138,7 +137,7 @@ func EditRepoHook(ctx *middleware.Context, form api.EditHookOption) {
 		}
 		if ct, ok := form.Config["content_type"]; ok {
 			if !models.IsValidHookContentType(ct) {
-				ctx.JSON(422, &base.ApiJsonErr{"invalid content type", base.DOC_URL})
+				ctx.APIError(422, "", "Invalid content type")
 				return
 			}
 			w.ContentType = models.ToHookContentType(ct)
@@ -153,7 +152,7 @@ func EditRepoHook(ctx *middleware.Context, form api.EditHookOption) {
 					Color:    form.Config["color"],
 				})
 				if err != nil {
-					ctx.JSON(500, &base.ApiJsonErr{"slack: JSON marshal failed: " + err.Error(), base.DOC_URL})
+					ctx.APIError(500, "slack: JSON marshal failed", err)
 					return
 				}
 				w.Meta = string(meta)
@@ -171,7 +170,7 @@ func EditRepoHook(ctx *middleware.Context, form api.EditHookOption) {
 	w.Create = com.IsSliceContainsStr(form.Events, string(models.HOOK_EVENT_CREATE))
 	w.Push = com.IsSliceContainsStr(form.Events, string(models.HOOK_EVENT_PUSH))
 	if err = w.UpdateEvent(); err != nil {
-		ctx.JSON(500, &base.ApiJsonErr{"UpdateEvent: " + err.Error(), base.DOC_URL})
+		ctx.APIError(500, "UpdateEvent", err)
 		return
 	}
 
@@ -180,7 +179,7 @@ func EditRepoHook(ctx *middleware.Context, form api.EditHookOption) {
 	}
 
 	if err := models.UpdateWebhook(w); err != nil {
-		ctx.JSON(500, &base.ApiJsonErr{"UpdateWebhook: " + err.Error(), base.DOC_URL})
+		ctx.APIError(500, "UpdateWebhook", err)
 		return
 	}
 
diff --git a/routers/api/v1/user.go b/routers/api/v1/user.go
index 57bf68bb56..f27cd3ae44 100644
--- a/routers/api/v1/user.go
+++ b/routers/api/v1/user.go
@@ -10,7 +10,6 @@ import (
 	api "github.com/gogits/go-gogs-client"
 
 	"github.com/gogits/gogs/models"
-	"github.com/gogits/gogs/modules/base"
 	"github.com/gogits/gogs/modules/middleware"
 )
 
@@ -69,7 +68,7 @@ func GetUserInfo(ctx *middleware.Context) {
 		if models.IsErrUserNotExist(err) {
 			ctx.Error(404)
 		} else {
-			ctx.JSON(500, &base.ApiJsonErr{"GetUserByName: " + err.Error(), base.DOC_URL})
+			ctx.APIError(500, "GetUserByName", err)
 		}
 		return
 	}
diff --git a/routers/api/v1/user_app.go b/routers/api/v1/user_app.go
index 5e5156aca4..590d187e53 100644
--- a/routers/api/v1/user_app.go
+++ b/routers/api/v1/user_app.go
@@ -8,7 +8,6 @@ import (
 	api "github.com/gogits/go-gogs-client"
 
 	"github.com/gogits/gogs/models"
-	"github.com/gogits/gogs/modules/base"
 	"github.com/gogits/gogs/modules/middleware"
 )
 
@@ -16,7 +15,7 @@ import (
 func ListAccessTokens(ctx *middleware.Context) {
 	tokens, err := models.ListAccessTokens(ctx.User.Id)
 	if err != nil {
-		ctx.JSON(500, &base.ApiJsonErr{"ListAccessTokens: " + err.Error(), base.DOC_URL})
+		ctx.APIError(500, "ListAccessTokens", err)
 		return
 	}
 
@@ -38,7 +37,7 @@ func CreateAccessToken(ctx *middleware.Context, form CreateAccessTokenForm) {
 		Name: form.Name,
 	}
 	if err := models.NewAccessToken(t); err != nil {
-		ctx.JSON(500, &base.ApiJsonErr{"NewAccessToken: " + err.Error(), base.DOC_URL})
+		ctx.APIError(500, "NewAccessToken", err)
 		return
 	}
 	ctx.JSON(201, &api.AccessToken{t.Name, t.Sha1})
diff --git a/templates/.VERSION b/templates/.VERSION
index d9b8eb43e7..6be1b25532 100644
--- a/templates/.VERSION
+++ b/templates/.VERSION
@@ -1 +1 @@
-0.6.16.1007 Beta
\ No newline at end of file
+0.6.16.1008 Beta
\ No newline at end of file