Make error rendering context-aware

modules/context/api.go

@@ -8,6 +8,8 @@ package context
 import (
 	"context"
 	"fmt"
+	"html/template"
+	"io"
 	"net/http"
 	"net/url"
 	"strings"
@@ -21,6 +23,8 @@ import (
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/web/middleware"
 	auth_service "code.gitea.io/gitea/services/auth"
+
+	"github.com/unrolled/render"
 )
 
 // APIContext is a specific context for API service
@@ -89,25 +93,19 @@ func (ctx *APIContext) ServerError(title string, err error) {
 // Error responds with an error message to client with given obj as the message.
 // If status is 500, also it prints error to log.
 func (ctx *APIContext) Error(status int, title string, obj interface{}) {
-	var message string
+	var foundError error
+	message := title
 	if err, ok := obj.(error); ok {
-		message = err.Error()
+		foundError = err
 	} else {
 		message = fmt.Sprintf("%s", obj)
 	}
 
 	if status == http.StatusInternalServerError {
-		log.ErrorWithSkip(1, "%s: %s", title, message)
-
-		if setting.IsProd && !(ctx.Doer != nil && ctx.Doer.IsAdmin) {
-			message = ""
-		}
+		log.ErrorWithSkip(1, "%s: %v", title, foundError)
 	}
 
-	ctx.JSON(status, APIError{
-		Message: message,
-		URL:     setting.API.SwaggerURL,
-	})
+	ctx.ErrorHandler.Error(ctx.Context, status, message, foundError)
 }
 
 // InternalServerError responds with an error message to the client with the error as a message
@@ -239,17 +237,31 @@ func APIAuth(authMethod auth_service.Method) func(*APIContext) {
 	}
 }
 
+// Implements a stub Render implementation to demonstrate that the API context lacks this feature.
+// Instead of displaying a vague NPE panic when called, it now displays a clear panic message.
+type panicRender struct{}
+
+func (panicRender) TemplateLookup(string) *template.Template {
+	panic("Render.TemplateLookup is not implemented for API context.")
+}
+
+func (panicRender) HTML(w io.Writer, status int, name string, binding interface{}, htmlOpt ...render.HTMLOptions) error {
+	panic("Render.HTML is not implemented for API context.")
+}
+
 // APIContexter returns apicontext as middleware
 func APIContexter() func(http.Handler) http.Handler {
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
 			locale := middleware.Locale(w, req)
 			ctx := APIContext{
 				Context: &Context{
-					Resp:   NewResponse(w),
-					Data:   map[string]interface{}{},
-					Locale: locale,
-					Cache:  cache.GetCache(),
+					Resp:         NewResponse(w),
+					Render:       panicRender{},
+					ErrorHandler: apiErrorHandler{},
+					Data:         map[string]interface{}{},
+					Locale:       locale,
+					Cache:        cache.GetCache(),
 					Repo: &Repository{
 						PullRequest: &PullRequest{},
 					},
@@ -294,24 +306,64 @@ func APIContexter() func(http.Handler) http.Handler {
 // String will replace message, errors will be added to a slice
 func (ctx *APIContext) NotFound(objs ...interface{}) {
 	message := ctx.Tr("error.not_found")
-	var errors []string
+	var foundError error
 	for _, obj := range objs {
-		// Ignore nil
 		if obj == nil {
 			continue
 		}
 
 		if err, ok := obj.(error); ok {
-			errors = append(errors, err.Error())
+			foundError = err
 		} else {
 			message = obj.(string)
 		}
 	}
 
+	ctx.ErrorHandler.NotFound(ctx.Context, message, foundError)
+}
+
+type apiErrorHandler struct{}
+
+var _ ErrorHandler = apiErrorHandler{}
+
+func (apiErrorHandler) NotFound(ctx *Context, logMessage string, logErr error) {
 	ctx.JSON(http.StatusNotFound, map[string]interface{}{
+		"message": logMessage,
+		"url":     setting.API.SwaggerURL,
+		"error":   logErr,
+	})
+}
+
+func (apiErrorHandler) ServerError(ctx *Context, logMessage string, logErr error) {
+	message := logMessage
+
+	if setting.IsProd && (ctx.Doer == nil || !ctx.Doer.IsAdmin) {
+		message = ""
+		logErr = nil
+	}
+
+	ctx.JSON(http.StatusInternalServerError, map[string]interface{}{
+		"message": message,
+		"url":     setting.API.SwaggerURL,
+		"error":   logErr,
+	})
+}
+
+func (apiErrorHandler) Error(ctx *Context, status int, logMessage string, logErr error) {
+	message := logMessage
+
+	// Errors are given as message and priority over logMessage.
+	if logErr != nil {
+		message = logErr.Error()
+	}
+
+	if status == http.StatusInternalServerError && setting.IsProd && (ctx.Doer == nil || !ctx.Doer.IsAdmin) {
+		message = ""
+	}
+
+	ctx.JSON(status, map[string]interface{}{
 		"message": message,
 		"url":     setting.API.SwaggerURL,
-		"errors":  errors,
 	})
 }
 

modules/context/context.go

@@ -52,13 +52,23 @@ type Render interface {
 	HTML(w io.Writer, status int, name string, binding interface{}, htmlOpt ...render.HTMLOptions) error
 }
 
+type ErrorHandler interface {
+	// Logs and display an 404 error.
+	NotFound(ctx *Context, logMessage string, logErr error)
+	// Logs and display an 500 error.
+	ServerError(ctx *Context, logMessage string, logErr error)
+	// Logs and display the specified status, logErr can be nil.
+	Error(ctx *Context, status int, logMessage string, logErr error)
+}
+
 // Context represents context of a request.
 type Context struct {
-	Resp     ResponseWriter
-	Req      *http.Request
-	Data     map[string]interface{} // data used by MVC templates
-	PageData map[string]interface{} // data used by JavaScript modules in one page, it's `window.config.pageData`
-	Render   Render
+	Resp         ResponseWriter
+	Req          *http.Request
+	Data         map[string]interface{} // data used by MVC templates
+	PageData     map[string]interface{} // data used by JavaScript modules in one page, it's `window.config.pageData`
+	Render       Render
+	ErrorHandler ErrorHandler
 	translation.Locale
 	Cache   cache.Cache
 	csrf    CSRFProtector
@@ -253,68 +263,23 @@ func (ctx *Context) RenderWithErr(msg string, tpl base.TplName, form interface{}
 
 // NotFound displays a 404 (Not Found) page and prints the given error, if any.
 func (ctx *Context) NotFound(logMsg string, logErr error) {
-	ctx.notFoundInternal(logMsg, logErr)
-}
-
-func (ctx *Context) notFoundInternal(logMsg string, logErr error) {
-	if logErr != nil {
-		log.Log(2, log.DEBUG, "%s: %v", logMsg, logErr)
-		if !setting.IsProd {
-			ctx.Data["ErrorMsg"] = logErr
-		}
-	}
-
-	// response simple message if Accept isn't text/html
-	showHTML := false
-	for _, part := range ctx.Req.Header["Accept"] {
-		if strings.Contains(part, "text/html") {
-			showHTML = true
-			break
-		}
-	}
-
-	if !showHTML {
-		ctx.plainTextInternal(3, http.StatusNotFound, []byte("Not found.\n"))
-		return
-	}
-
-	ctx.Data["IsRepo"] = ctx.Repo.Repository != nil
-	ctx.Data["Title"] = "Page Not Found"
-	ctx.HTML(http.StatusNotFound, base.TplName("status/404"))
+	ctx.ErrorHandler.NotFound(ctx, logMsg, logErr)
 }
 
 // ServerError displays a 500 (Internal Server Error) page and prints the given error, if any.
 func (ctx *Context) ServerError(logMsg string, logErr error) {
-	ctx.serverErrorInternal(logMsg, logErr)
-}
-
-func (ctx *Context) serverErrorInternal(logMsg string, logErr error) {
-	if logErr != nil {
-		log.ErrorWithSkip(2, "%s: %v", logMsg, logErr)
-		if _, ok := logErr.(*net.OpError); ok || errors.Is(logErr, &net.OpError{}) {
-			// This is an error within the underlying connection
-			// and further rendering will not work so just return
-			return
-		}
-
-		if !setting.IsProd {
-			ctx.Data["ErrorMsg"] = logErr
-		}
-	}
-
-	ctx.Data["Title"] = "Internal Server Error"
-	ctx.HTML(http.StatusInternalServerError, base.TplName("status/500"))
+	ctx.ErrorHandler.ServerError(ctx, logMsg, logErr)
 }
 
 // NotFoundOrServerError use error check function to determine if the error
 // is about not found. It responds with 404 status code for not found error,
 // or error context description for logging purpose of 500 server error.
 func (ctx *Context) NotFoundOrServerError(logMsg string, errCheck func(error) bool, err error) {
 	if errCheck(err) {
-		ctx.notFoundInternal(logMsg, err)
+		ctx.NotFound(logMsg, err)
 		return
 	}
-	ctx.serverErrorInternal(logMsg, err)
+	ctx.ServerError(logMsg, err)
 }
 
 // PlainTextBytes renders bytes as plain text
@@ -427,11 +392,11 @@ func (ctx *Context) UploadStream() (rd io.ReadCloser, needToClose bool, err erro
 
 // Error returned an error to web browser
 func (ctx *Context) Error(status int, contents ...string) {
-	v := http.StatusText(status)
+	message := http.StatusText(status)
 	if len(contents) > 0 {
-		v = contents[0]
+		message = contents[0]
 	}
-	http.Error(ctx.Resp, v, status)
+	ctx.ErrorHandler.Error(ctx, status, message, nil)
 }
 
 // JSON render content as JSON
@@ -618,6 +583,59 @@ func (ctx *Context) AppendAccessControlExposeHeaders(names ...string) {
 	}
 }
 
+type webErrorHandler struct{}
+
+var _ ErrorHandler = webErrorHandler{}
+
+func (webErrorHandler) NotFound(ctx *Context, logMsg string, logErr error) {
+	if logErr != nil {
+		log.Log(2, log.DEBUG, "%s: %v", logMsg, logErr)
+		if !setting.IsProd {
+			ctx.Data["ErrorMsg"] = logErr
+		}
+	}
+
+	// response simple message if Accept isn't text/html
+	showHTML := false
+	for _, part := range ctx.Req.Header["Accept"] {
+		if strings.Contains(part, "text/html") {
+			showHTML = true
+			break
+		}
+	}
+
+	if !showHTML {
+		ctx.plainTextInternal(3, http.StatusNotFound, []byte("Not found.\n"))
+		return
+	}
+
+	ctx.Data["IsRepo"] = ctx.Repo.Repository != nil
+	ctx.Data["Title"] = "Page Not Found"
+	ctx.HTML(http.StatusNotFound, base.TplName("status/404"))
+}
+
+func (webErrorHandler) ServerError(ctx *Context, logMsg string, logErr error) {
+	if logErr != nil {
+		log.ErrorWithSkip(2, "%s: %v", logMsg, logErr)
+		if _, ok := logErr.(*net.OpError); ok || errors.Is(logErr, &net.OpError{}) {
+			// This is an error within the underlying connection
+			// and further rendering will not work so just return
+			return
+		}
+
+		if !setting.IsProd {
+			ctx.Data["ErrorMsg"] = logErr
+		}
+	}
+
+	ctx.Data["Title"] = "Internal Server Error"
+	ctx.HTML(http.StatusInternalServerError, base.TplName("status/500"))
+}
+
+func (webErrorHandler) Error(ctx *Context, status int, logMsg string, _ error) {
+	http.Error(ctx.Resp, logMsg, status)
+}
+
 // Handler represents a custom handler
 type Handler func(*Context)
 
@@ -699,12 +717,13 @@ func Contexter(ctx context.Context) func(next http.Handler) http.Handler {
 			link := setting.AppSubURL + strings.TrimSuffix(req.URL.EscapedPath(), "/")
 
 			ctx := Context{
-				Resp:    NewResponse(resp),
-				Cache:   mc.GetCache(),
-				Locale:  locale,
-				Link:    link,
-				Render:  rnd,
-				Session: session.GetSession(req),
+				Resp:         NewResponse(resp),
+				Cache:        mc.GetCache(),
+				Locale:       locale,
+				Link:         link,
+				Render:       rnd,
+				ErrorHandler: webErrorHandler{},
+				Session:      session.GetSession(req),
 				Repo: &Repository{
 					PullRequest: &PullRequest{},
 				},

modules/context/package.go

@@ -144,9 +144,10 @@ func PackageContexter(ctx gocontext.Context) func(next http.Handler) http.Handle
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) {
 			ctx := Context{
-				Resp:   NewResponse(resp),
-				Data:   map[string]interface{}{},
-				Render: rnd,
+				Resp:         NewResponse(resp),
+				Data:         map[string]interface{}{},
+				Render:       rnd,
+				ErrorHandler: apiErrorHandler{},
 			}
 			defer ctx.Close()