refactor(api): clarify APIError message usage and fix legacy lint error (#38012)

Avoid unclear & fragile "any" tricks, fix various abuses

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
Copilot
2026-06-07 06:19:39 +00:00
committed by GitHub
co-authored by wxiaoguang
parent c43eb7c33a
commit 5fe4f962e8
64 changed files with 395 additions and 384 deletions
+46 -43
View File
@@ -137,16 +137,34 @@ func (ctx *APIContext) apiErrorInternal(skip int, err error) {
})
}
// APIError 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) APIError(status int, obj any) {
// APIErrorNotFound handles 404s for APIContext
// String will replace message, errors will be added to a slice
func (ctx *APIContext) APIErrorNotFound(objs ...any) {
var message string
if err, ok := obj.(error); ok {
message = err.Error()
} else {
message = fmt.Sprintf("%s", obj)
}
var errs []string
for _, obj := range objs {
// Ignore nil
if obj == nil {
continue
}
if err, ok := obj.(error); ok {
errs = append(errs, err.Error())
} else {
message = obj.(string)
}
}
ctx.JSON(http.StatusNotFound, map[string]any{
"message": util.IfZero(message, "not found"), // do not use locale in API
"url": setting.API.SwaggerURL,
"errors": errs,
})
}
// APIError responds with an error message to client.
// If status is 500, also it prints error to log.
func (ctx *APIContext) APIError(status int, msg string) {
message := msg
if status == http.StatusInternalServerError {
log.ErrorWithSkip(1, "APIError: %s", message)
@@ -161,6 +179,26 @@ func (ctx *APIContext) APIError(status int, obj any) {
})
}
// APIErrorAuto use error check function to determine the response code
func (ctx *APIContext) APIErrorAuto(err error) {
switch {
case errors.Is(err, util.ErrInvalidArgument):
ctx.APIError(http.StatusBadRequest, err.Error())
case errors.Is(err, util.ErrPermissionDenied):
ctx.APIError(http.StatusForbidden, err.Error())
case errors.Is(err, util.ErrNotExist):
ctx.APIError(http.StatusNotFound, err.Error())
case errors.Is(err, util.ErrAlreadyExist):
ctx.APIError(http.StatusConflict, err.Error())
case errors.Is(err, util.ErrContentTooLarge):
ctx.APIError(http.StatusRequestEntityTooLarge, err.Error())
case errors.Is(err, util.ErrUnprocessableContent):
ctx.APIError(http.StatusUnprocessableEntity, err.Error())
default:
ctx.apiErrorInternal(1, err)
}
}
type apiContextKeyType struct{}
var apiContextKey = apiContextKeyType{}
@@ -248,30 +286,6 @@ func APIContexter() func(http.Handler) http.Handler {
}
}
// APIErrorNotFound handles 404s for APIContext
// String will replace message, errors will be added to a slice
func (ctx *APIContext) APIErrorNotFound(objs ...any) {
var message string
var errs []string
for _, obj := range objs {
// Ignore nil
if obj == nil {
continue
}
if err, ok := obj.(error); ok {
errs = append(errs, err.Error())
} else {
message = obj.(string)
}
}
ctx.JSON(http.StatusNotFound, map[string]any{
"message": util.IfZero(message, "not found"), // do not use locale in API
"url": setting.API.SwaggerURL,
"errors": errs,
})
}
// ReferencesGitRepo injects the GitRepo into the Context
// you can optional skip the IsEmpty check
func ReferencesGitRepo(allowEmpty ...bool) func(ctx *APIContext) {
@@ -329,17 +343,6 @@ func RepoRefForAPI(next http.Handler) http.Handler {
})
}
// 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 *APIContext) NotFoundOrServerError(err error) {
if errors.Is(err, util.ErrNotExist) {
ctx.JSON(http.StatusNotFound, nil)
return
}
ctx.APIErrorInternal(err)
}
// IsUserSiteAdmin returns true if current user is a site admin
func (ctx *APIContext) IsUserSiteAdmin() bool {
return ctx.IsSigned && ctx.Doer.IsAdmin
+9 -12
View File
@@ -34,11 +34,8 @@ type packageAssignmentCtx struct {
// PackageAssignment returns a middleware to handle Context.Package assignment
func PackageAssignment() func(ctx *Context) {
return func(ctx *Context) {
errorFn := func(status int, obj any) {
err, ok := obj.(error)
if !ok {
err = fmt.Errorf("%s", obj)
}
errorFn := func(status int, msg string) {
err := fmt.Errorf("%s", msg)
if status == http.StatusNotFound {
ctx.NotFound(err)
} else {
@@ -58,11 +55,11 @@ func PackageAssignmentAPI() func(ctx *APIContext) {
}
}
func packageAssignment(ctx *packageAssignmentCtx, errCb func(int, any)) *Package {
func packageAssignment(ctx *packageAssignmentCtx, errCb func(int, string)) *Package {
pkgOwner := ctx.ContextUser
accessMode, err := determineAccessMode(ctx.Base, pkgOwner, ctx.Doer)
if err != nil {
errCb(http.StatusInternalServerError, fmt.Errorf("determineAccessMode: %w", err))
errCb(http.StatusInternalServerError, fmt.Sprintf("determineAccessMode: %v", err))
return nil
}
@@ -81,25 +78,25 @@ func packageAssignment(ctx *packageAssignmentCtx, errCb func(int, any)) *Package
pv, err := packages_model.GetVersionByNameAndVersion(ctx, pkg.Owner.ID, packages_model.Type(packageType), name, version)
if err != nil {
if errors.Is(err, packages_model.ErrPackageNotExist) {
errCb(http.StatusNotFound, fmt.Errorf("GetVersionByNameAndVersion: %w", err))
errCb(http.StatusNotFound, fmt.Sprintf("GetVersionByNameAndVersion: %v", err))
} else {
errCb(http.StatusInternalServerError, fmt.Errorf("GetVersionByNameAndVersion: %w", err))
errCb(http.StatusInternalServerError, fmt.Sprintf("GetVersionByNameAndVersion: %v", err))
}
return pkg
}
pkg.Descriptor, err = packages_model.GetPackageDescriptor(ctx, pv)
if err != nil {
errCb(http.StatusInternalServerError, fmt.Errorf("GetPackageDescriptor: %w", err))
errCb(http.StatusInternalServerError, fmt.Sprintf("GetPackageDescriptor: %v", err))
return pkg
}
} else {
p, err := packages_model.GetPackageByName(ctx, pkg.Owner.ID, packages_model.Type(packageType), name)
if err != nil {
if errors.Is(err, packages_model.ErrPackageNotExist) {
errCb(http.StatusNotFound, fmt.Errorf("GetPackageByName: %w", err))
errCb(http.StatusNotFound, fmt.Sprintf("GetPackageByName: %v", err))
} else {
errCb(http.StatusInternalServerError, fmt.Errorf("GetPackageByName: %w", err))
errCb(http.StatusInternalServerError, fmt.Sprintf("GetPackageByName: %v", err))
}
return pkg
}
+6 -9
View File
@@ -14,11 +14,8 @@ import (
// UserAssignmentWeb returns a middleware to handle context-user assignment for web routes
func UserAssignmentWeb() func(ctx *Context) {
return func(ctx *Context) {
errorFn := func(status int, obj any) {
err, ok := obj.(error)
if !ok {
err = fmt.Errorf("%s", obj)
}
errorFn := func(status int, msg string) {
err := fmt.Errorf("%s", msg)
if status == http.StatusNotFound {
ctx.NotFound(err)
} else {
@@ -37,7 +34,7 @@ func UserAssignmentAPI() func(ctx *APIContext) {
}
}
func userAssignment(ctx *Base, doer *user_model.User, errCb func(int, any)) (contextUser *user_model.User) {
func userAssignment(ctx *Base, doer *user_model.User, errCb func(int, string)) (contextUser *user_model.User) {
username := ctx.PathParam("username")
if doer != nil && strings.EqualFold(doer.LowerName, username) {
@@ -50,12 +47,12 @@ func userAssignment(ctx *Base, doer *user_model.User, errCb func(int, any)) (con
if redirectUserID, err := user_model.LookupUserRedirect(ctx, username); err == nil {
RedirectToUser(ctx, doer, username, redirectUserID)
} else if user_model.IsErrUserRedirectNotExist(err) {
errCb(http.StatusNotFound, err)
errCb(http.StatusNotFound, err.Error())
} else {
errCb(http.StatusInternalServerError, fmt.Errorf("LookupUserRedirect: %w", err))
errCb(http.StatusInternalServerError, fmt.Sprintf("LookupUserRedirect: %v", err))
}
} else {
errCb(http.StatusInternalServerError, fmt.Errorf("GetUserByName: %w", err))
errCb(http.StatusInternalServerError, fmt.Sprintf("GetUserByName: %v", err))
}
}
}