refactor: unify error handling in routing layer

6 files changed, 90 insertions, 101 deletions

diff --git a/internal/context/api.go b/internal/context/api.go
index dfea976c..938e207f 100644
--- a/internal/context/api.go
+++ b/internal/context/api.go
@@ -14,6 +14,7 @@ import (
 	log "unknwon.dev/clog/v2"
 
 	"gogs.io/gogs/internal/conf"
+	"gogs.io/gogs/internal/errutil"
 )
 
 type APIContext struct {
@@ -28,26 +29,6 @@ type APIContext struct {
 // FIXME: move this constant to github.com/gogs/go-gogs-client
 const DocURL = "https://github.com/gogs/docs-api"
 
-// Error responses error message to client with given message.
-// If status is 500, also it prints error to log.
-func (c *APIContext) Error(status int, title string, obj interface{}) {
-	var message string
-	if err, ok := obj.(error); ok {
-		message = err.Error()
-	} else {
-		message = obj.(string)
-	}
-
-	if status == http.StatusInternalServerError {
-		log.ErrorDepth(5, "%s: %s", title, message)
-	}
-
-	c.JSON(status, map[string]string{
-		"message": message,
-		"url":     DocURL,
-	})
-}
-
 // NoContent renders the 204 response.
 func (c *APIContext) NoContent() {
 	c.Status(http.StatusNoContent)
@@ -58,25 +39,34 @@ func (c *APIContext) NotFound() {
 	c.Status(http.StatusNotFound)
 }
 
-// ServerError renders the 500 response.
-func (c *APIContext) ServerError(title string, err error) {
-	c.Error(http.StatusInternalServerError, title, err)
+// ErrorStatus renders error with given status code.
+func (c *APIContext) ErrorStatus(status int, err error) {
+	c.JSON(status, map[string]string{
+		"message": err.Error(),
+		"url":     DocURL,
+	})
+}
+
+// Error renders the 500 response.
+func (c *APIContext) Error(err error, msg string) {
+	log.ErrorDepth(5, "%s: %v", msg, err)
+	c.ErrorStatus(http.StatusInternalServerError, err)
 }
 
 // Errorf renders the 500 response with formatted message.
 func (c *APIContext) Errorf(err error, format string, args ...interface{}) {
-	c.Error(http.StatusInternalServerError, fmt.Sprintf(format, args...), err)
+	c.Error(err, fmt.Sprintf(format, args...))
 }
 
-// NotFoundOrServerError use error check function to determine if the error
+// NotFoundOrError use error check function to determine if the error
 // is about not found. It responses with 404 status code for not found error,
 // or error context description for logging purpose of 500 server error.
-func (c *APIContext) NotFoundOrServerError(title string, errck func(error) bool, err error) {
-	if errck(err) {
+func (c *APIContext) NotFoundOrError(err error, msg string) {
+	if errutil.IsNotFound(err) {
 		c.NotFound()
 		return
 	}
-	c.ServerError(title, err)
+	c.Error(err, msg)
 }
 
 // SetLinkHeader sets pagination link header by given total number and page size.
diff --git a/internal/context/auth.go b/internal/context/auth.go
index f99a0bb2..2a7a1aef 100644
--- a/internal/context/auth.go
+++ b/internal/context/auth.go
@@ -28,26 +28,26 @@ func Toggle(options *ToggleOptions) macaron.Handler {
 	return func(c *Context) {
 		// Cannot view any page before installation.
 		if !conf.Security.InstallLock {
-			c.Redirect(conf.Server.Subpath + "/install")
+			c.RedirectSubpath("/install")
 			return
 		}
 
 		// Check prohibit login users.
 		if c.IsLogged && c.User.ProhibitLogin {
 			c.Data["Title"] = c.Tr("auth.prohibit_login")
-			c.HTML(200, "user/auth/prohibit_login")
+			c.Success( "user/auth/prohibit_login")
 			return
 		}
 
 		// Check non-logged users landing page.
 		if !c.IsLogged && c.Req.RequestURI == "/" && conf.Server.LandingURL != "/" {
-			c.SubURLRedirect(conf.Server.LandingURL)
+			c.RedirectSubpath(conf.Server.LandingURL)
 			return
 		}
 
 		// Redirect to dashboard if user tries to visit any non-login page.
 		if options.SignOutRequired && c.IsLogged && c.Req.RequestURI != "/" {
-			c.Redirect(conf.Server.Subpath + "/")
+			c.RedirectSubpath("/")
 			return
 		}
 
@@ -62,18 +62,18 @@ func Toggle(options *ToggleOptions) macaron.Handler {
 			if !c.IsLogged {
 				// Restrict API calls with error message.
 				if auth.IsAPIPath(c.Req.URL.Path) {
-					c.JSON(403, map[string]string{
-						"message": "Only signed in user is allowed to call APIs.",
+					c.JSON(http.StatusForbidden, map[string]string{
+						"message": "Only authenticated user is allowed to call APIs.",
 					})
 					return
 				}
 
 				c.SetCookie("redirect_to", url.QueryEscape(conf.Server.Subpath+c.Req.RequestURI), 0, conf.Server.Subpath)
-				c.Redirect(conf.Server.Subpath + "/user/login")
+				c.RedirectSubpath("/user/login")
 				return
 			} else if !c.User.IsActive && conf.Auth.RequireEmailConfirmation {
-				c.Data["Title"] = c.Tr("auth.active_your_account")
-				c.HTML(200, "user/auth/activate")
+				c.Title("auth.active_your_account")
+				c.Success("user/auth/activate")
 				return
 			}
 		}
@@ -82,21 +82,21 @@ func Toggle(options *ToggleOptions) macaron.Handler {
 		if !options.SignOutRequired && !c.IsLogged && !auth.IsAPIPath(c.Req.URL.Path) &&
 			len(c.GetCookie(conf.Security.CookieUsername)) > 0 {
 			c.SetCookie("redirect_to", url.QueryEscape(conf.Server.Subpath+c.Req.RequestURI), 0, conf.Server.Subpath)
-			c.Redirect(conf.Server.Subpath + "/user/login")
+			c.RedirectSubpath("/user/login")
 			return
 		}
 
 		if options.AdminRequired {
 			if !c.User.IsAdmin {
-				c.Error(403)
+				c.Status(http.StatusForbidden)
 				return
 			}
-			c.Data["PageIsAdmin"] = true
+			c.PageIs("Admin")
 		}
 	}
 }
 
-// RequireBasicAuth verifies HTTP Basic Authentication header with given credentials
+// RequireBasicAuth verifies HTTP Basic Authentication header with given credentials.
 func (c *Context) RequireBasicAuth(username, password string) {
 	fields := strings.Fields(c.Req.Header.Get("Authorization"))
 	if len(fields) != 2 || fields[0] != "Basic" {
diff --git a/internal/context/context.go b/internal/context/context.go
index b4809ec7..142fd241 100644
--- a/internal/context/context.go
+++ b/internal/context/context.go
@@ -23,7 +23,7 @@ import (
 	"gogs.io/gogs/internal/auth"
 	"gogs.io/gogs/internal/conf"
 	"gogs.io/gogs/internal/db"
-	"gogs.io/gogs/internal/db/errors"
+	"gogs.io/gogs/internal/errutil"
 	"gogs.io/gogs/internal/form"
 	"gogs.io/gogs/internal/template"
 )
@@ -149,15 +149,15 @@ func (c *Context) RawRedirect(location string, status ...int) {
 	c.Context.Redirect(location, status...)
 }
 
-// Redirect responses redirection wtih given location and status.
+// Redirect responses redirection with given location and status.
 // It escapes special characters in the location string.
 func (c *Context) Redirect(location string, status ...int) {
 	c.Context.Redirect(template.EscapePound(location), status...)
 }
 
-// SubURLRedirect responses redirection wtih given location and status.
+// RedirectSubpath responses redirection with given location and status.
 // It prepends setting.Server.Subpath to the location string.
-func (c *Context) SubURLRedirect(location string, status ...int) {
+func (c *Context) RedirectSubpath(location string, status ...int) {
 	c.Redirect(conf.Server.Subpath+location, status...)
 }
 
@@ -171,44 +171,46 @@ func (c *Context) RenderWithErr(msg, tpl string, f interface{}) {
 	c.HTML(http.StatusOK, tpl)
 }
 
-// Handle handles and logs error by given status.
-func (c *Context) Handle(status int, msg string, err error) {
-	switch status {
-	case http.StatusNotFound:
-		c.Data["Title"] = "Page Not Found"
-	case http.StatusInternalServerError:
-		c.Data["Title"] = "Internal Server Error"
-		log.ErrorDepth(5, "%s: %v", msg, err)
-		if !conf.IsProdMode() || (c.IsLogged && c.User.IsAdmin) {
-			c.Data["ErrorMsg"] = err
-		}
-	}
-	c.HTML(status, fmt.Sprintf("status/%d", status))
-}
-
 // NotFound renders the 404 page.
 func (c *Context) NotFound() {
-	c.Handle(http.StatusNotFound, "", nil)
+	c.Title("status.page_not_found")
+	c.HTML(http.StatusNotFound, fmt.Sprintf("status/%d", http.StatusNotFound))
 }
 
-// ServerError renders the 500 page.
-func (c *Context) ServerError(msg string, err error) {
-	c.Handle(http.StatusInternalServerError, msg, err)
+// Error renders the 500 page.
+func (c *Context) Error(err error, msg string) {
+	log.ErrorDepth(5, "%s: %v", msg, err)
+
+	c.Title("status.internal_server_error")
+
+	// Only in non-production mode or admin can see the actual error message.
+	if !conf.IsProdMode() || (c.IsLogged && c.User.IsAdmin) {
+		c.Data["ErrorMsg"] = err
+	}
+	c.HTML(http.StatusInternalServerError, fmt.Sprintf("status/%d", http.StatusInternalServerError))
+}
+
+// Errorf renders the 500 response with formatted message.
+func (c *Context) Errorf(err error, format string, args ...interface{}) {
+	c.Error(err, fmt.Sprintf(format, args...))
 }
 
-// NotFoundOrServerError use error check function to determine if the error
-// is about not found. It responses with 404 status code for not found error,
-// or error context description for logging purpose of 500 server error.
-func (c *Context) NotFoundOrServerError(msg string, errck func(error) bool, err error) {
-	if errck(err) {
+// NotFoundOrError responses with 404 page for not found error and 500 page otherwise.
+func (c *Context) NotFoundOrError(err error, msg string) {
+	if errutil.IsNotFound(err) {
 		c.NotFound()
 		return
 	}
-	c.ServerError(msg, err)
+	c.Error(err, msg)
+}
+
+// NotFoundOrErrorf is same as NotFoundOrError but with formatted message.
+func (c *Context) NotFoundOrErrorf(err error, format string, args ...interface{}) {
+	c.NotFoundOrError(err, fmt.Sprintf(format, args...))
 }
 
-func (c *Context) HandleText(status int, msg string) {
-	c.PlainText(status, []byte(msg))
+func (c *Context) PlainText(status int, msg string) {
+	c.Render.PlainText(status, []byte(msg))
 }
 
 func (c *Context) ServeContent(name string, r io.ReadSeeker, params ...interface{}) {
@@ -259,7 +261,7 @@ func Contexter() macaron.Handler {
 
 			owner, err := db.GetUserByName(ownerName)
 			if err != nil {
-				c.NotFoundOrServerError("GetUserByName", errors.IsUserNotExist, err)
+				c.NotFoundOrError(err, "get user by name")
 				return
 			}
 
@@ -273,7 +275,7 @@ func Contexter() macaron.Handler {
 			if !strings.HasPrefix(conf.Server.ExternalURL, "https://") {
 				insecureFlag = "--insecure "
 			}
-			c.PlainText(http.StatusOK, []byte(com.Expand(`<!doctype html>
+			c.PlainText(http.StatusOK, com.Expand(`<!doctype html>
 <html>
 	<head>
 		<meta name="go-import" content="{GoGetImport} git {CloneLink}">
@@ -284,12 +286,12 @@ func Contexter() macaron.Handler {
 	</body>
 </html>
 `, map[string]string{
-				"GoGetImport":    path.Join(conf.Server.URL.Host, conf.Server.Subpath, repo.FullName()),
+				"GoGetImport":    path.Join(conf.Server.URL.Host, conf.Server.Subpath, ownerName, repoName),
 				"CloneLink":      db.ComposeHTTPSCloneURL(ownerName, repoName),
 				"GoDocDirectory": prefix + "{/dir}",
 				"GoDocFile":      prefix + "{/dir}/{file}#L{line}",
 				"InsecureFlag":   insecureFlag,
-			})))
+			}))
 			return
 		}
 
@@ -318,7 +320,7 @@ func Contexter() macaron.Handler {
 		// If request sends files, parse them here otherwise the Query() can't be parsed and the CsrfToken will be invalid.
 		if c.Req.Method == "POST" && strings.Contains(c.Req.Header.Get("Content-Type"), "multipart/form-data") {
 			if err := c.Req.ParseMultipartForm(conf.Attachment.MaxSize << 20); err != nil && !strings.Contains(err.Error(), "EOF") { // 32MB max size
-				c.ServerError("ParseMultipartForm", err)
+				c.Error(err, "parse multipart form")
 				return
 			}
 		}
diff --git a/internal/context/org.go b/internal/context/org.go
index 02e154ca..6ed35835 100644
--- a/internal/context/org.go
+++ b/internal/context/org.go
@@ -11,7 +11,6 @@ import (
 
 	"gogs.io/gogs/internal/conf"
 	"gogs.io/gogs/internal/db"
-	"gogs.io/gogs/internal/db/errors"
 )
 
 type Organization struct {
@@ -50,7 +49,7 @@ func HandleOrgAssignment(c *Context, args ...bool) {
 	var err error
 	c.Org.Organization, err = db.GetUserByName(orgName)
 	if err != nil {
-		c.NotFoundOrServerError("GetUserByName", errors.IsUserNotExist, err)
+		c.NotFoundOrError(err, "get organization by name")
 		return
 	}
 	org := c.Org.Organization
@@ -85,7 +84,7 @@ func HandleOrgAssignment(c *Context, args ...bool) {
 	}
 	if (requireMember && !c.Org.IsMember) ||
 		(requireOwner && !c.Org.IsOwner) {
-		c.Handle(404, "OrgAssignment", err)
+		c.NotFound()
 		return
 	}
 	c.Data["IsOrganizationOwner"] = c.Org.IsOwner
@@ -98,13 +97,13 @@ func HandleOrgAssignment(c *Context, args ...bool) {
 	if c.Org.IsMember {
 		if c.Org.IsOwner {
 			if err := org.GetTeams(); err != nil {
-				c.Handle(500, "GetTeams", err)
+				c.Error(err, "get teams")
 				return
 			}
 		} else {
 			org.Teams, err = org.GetUserTeams(c.User.ID)
 			if err != nil {
-				c.Handle(500, "GetUserTeams", err)
+				c.Error(err, "get user teams")
 				return
 			}
 		}
@@ -124,20 +123,20 @@ func HandleOrgAssignment(c *Context, args ...bool) {
 		}
 
 		if !teamExists {
-			c.Handle(404, "OrgAssignment", err)
+			c.NotFound()
 			return
 		}
 
 		c.Data["IsTeamMember"] = c.Org.IsTeamMember
 		if requireTeamMember && !c.Org.IsTeamMember {
-			c.Handle(404, "OrgAssignment", err)
+			c.NotFound()
 			return
 		}
 
 		c.Org.IsTeamAdmin = c.Org.Team.IsOwnerTeam() || c.Org.Team.Authorize >= db.ACCESS_MODE_ADMIN
 		c.Data["IsTeamAdmin"] = c.Org.IsTeamAdmin
 		if requireTeamAdmin && !c.Org.IsTeamAdmin {
-			c.Handle(404, "OrgAssignment", err)
+			c.NotFound()
 			return
 		}
 	}
diff --git a/internal/context/repo.go b/internal/context/repo.go
index 73352e9c..f2cac277 100644
--- a/internal/context/repo.go
+++ b/internal/context/repo.go
@@ -18,7 +18,6 @@ import (
 
 	"gogs.io/gogs/internal/conf"
 	"gogs.io/gogs/internal/db"
-	dberrors "gogs.io/gogs/internal/db/errors"
 )
 
 type PullRequest struct {
@@ -147,7 +146,7 @@ func RepoAssignment(pages ...bool) macaron.Handler {
 		} else {
 			owner, err = db.GetUserByName(ownerName)
 			if err != nil {
-				c.NotFoundOrServerError("GetUserByName", dberrors.IsUserNotExist, err)
+				c.NotFoundOrError(err, "get user by name")
 				return
 			}
 		}
@@ -156,7 +155,7 @@ func RepoAssignment(pages ...bool) macaron.Handler {
 
 		repo, err := db.GetRepositoryByName(owner.ID, repoName)
 		if err != nil {
-			c.NotFoundOrServerError("GetRepositoryByName", dberrors.IsRepoNotExist, err)
+			c.NotFoundOrError(err, "get repository by name")
 			return
 		}
 
@@ -173,7 +172,7 @@ func RepoAssignment(pages ...bool) macaron.Handler {
 		} else {
 			mode, err := db.UserAccessMode(c.UserID(), repo)
 			if err != nil {
-				c.ServerError("UserAccessMode", err)
+				c.Error(err, "get user access mode")
 				return
 			}
 			c.Repo.AccessMode = mode
@@ -212,7 +211,7 @@ func RepoAssignment(pages ...bool) macaron.Handler {
 		if repo.IsMirror {
 			c.Repo.Mirror, err = db.GetMirrorByRepoID(repo.ID)
 			if err != nil {
-				c.ServerError("GetMirror", err)
+				c.Error(err, "get mirror by repository ID")
 				return
 			}
 			c.Data["MirrorEnablePrune"] = c.Repo.Mirror.EnablePrune
@@ -222,14 +221,14 @@ func RepoAssignment(pages ...bool) macaron.Handler {
 
 		gitRepo, err := git.Open(db.RepoPath(ownerName, repoName))
 		if err != nil {
-			c.ServerError("open repository", err)
+			c.Error(err, "open repository")
 			return
 		}
 		c.Repo.GitRepo = gitRepo
 
 		tags, err := c.Repo.GitRepo.Tags()
 		if err != nil {
-			c.ServerError("get tags", err)
+			c.Error(err, "get tags")
 			return
 		}
 		c.Data["Tags"] = tags
@@ -260,7 +259,7 @@ func RepoAssignment(pages ...bool) macaron.Handler {
 		c.Data["TagName"] = c.Repo.TagName
 		branches, err := c.Repo.GitRepo.Branches()
 		if err != nil {
-			c.ServerError("get branches", err)
+			c.Error(err, "get branches")
 			return
 		}
 		c.Data["Branches"] = branches
@@ -300,7 +299,7 @@ func RepoRef() macaron.Handler {
 			repoPath := db.RepoPath(c.Repo.Owner.Name, c.Repo.Repository.Name)
 			c.Repo.GitRepo, err = git.Open(repoPath)
 			if err != nil {
-				c.Handle(500, "RepoRef Invalid repo "+repoPath, err)
+				c.Error(err, "open repository")
 				return
 			}
 		}
@@ -311,14 +310,14 @@ func RepoRef() macaron.Handler {
 			if !c.Repo.GitRepo.HasBranch(refName) {
 				branches, err := c.Repo.GitRepo.Branches()
 				if err != nil {
-					c.ServerError("get branches", err)
+					c.Error(err, "get branches")
 					return
 				}
 				refName = branches[0]
 			}
 			c.Repo.Commit, err = c.Repo.GitRepo.BranchCommit(refName)
 			if err != nil {
-				c.ServerError("get branch commit", err)
+				c.Error(err, "get branch commit")
 				return
 			}
 			c.Repo.CommitID = c.Repo.Commit.ID.String()
@@ -349,7 +348,7 @@ func RepoRef() macaron.Handler {
 
 				c.Repo.Commit, err = c.Repo.GitRepo.BranchCommit(refName)
 				if err != nil {
-					c.ServerError("get branch commit", err)
+					c.Error(err, "get branch commit")
 					return
 				}
 				c.Repo.CommitID = c.Repo.Commit.ID.String()
@@ -358,7 +357,7 @@ func RepoRef() macaron.Handler {
 				c.Repo.IsViewTag = true
 				c.Repo.Commit, err = c.Repo.GitRepo.TagCommit(refName)
 				if err != nil {
-					c.ServerError("get tag commit", err)
+					c.Error(err, "get tag commit")
 					return
 				}
 				c.Repo.CommitID = c.Repo.Commit.ID.String()
@@ -372,7 +371,7 @@ func RepoRef() macaron.Handler {
 					return
 				}
 			} else {
-				c.Handle(404, "RepoRef invalid repo", fmt.Errorf("branch or tag not exist: %s", refName))
+				c.NotFound()
 				return
 			}
 		}
diff --git a/internal/context/user.go b/internal/context/user.go
index d16b93b7..0d6ad67d 100644
--- a/internal/context/user.go
+++ b/internal/context/user.go
@@ -8,7 +8,6 @@ import (
 	"gopkg.in/macaron.v1"
 
 	"gogs.io/gogs/internal/db"
-	"gogs.io/gogs/internal/db/errors"
 )
 
 // ParamsUser is the wrapper type of the target user defined by URL parameter, namely ':username'.
@@ -22,7 +21,7 @@ func InjectParamsUser() macaron.Handler {
 	return func(c *Context) {
 		user, err := db.GetUserByName(c.Params(":username"))
 		if err != nil {
-			c.NotFoundOrServerError("GetUserByName", errors.IsUserNotExist, err)
+			c.NotFoundOrError(err, "get user by name")
 			return
 		}
 		c.Map(&ParamsUser{user})