Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor error system #33626

Merged
merged 2 commits into from
Feb 17, 2025
Merged

Refactor error system #33626

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e35482d
fix
wxiaoguang Feb 17, 2025
d829487
Merge branch 'main' into refactor-err
GiteaBot Feb 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions modules/util/error.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,16 @@ import (

// Common Errors forming the base of our error system
//
// Many Errors returned by Gitea can be tested against these errors
// using errors.Is.
// Many Errors returned by Gitea can be tested against these errors using "errors.Is".
var (
ErrInvalidArgument = errors.New("invalid argument")
ErrPermissionDenied = errors.New("permission denied")
ErrAlreadyExist = errors.New("resource already exists")
ErrNotExist = errors.New("resource does not exist")
ErrInvalidArgument = errors.New("invalid argument") // also implies HTTP 400
ErrPermissionDenied = errors.New("permission denied") // also implies HTTP 403
ErrNotExist = errors.New("resource does not exist") // also implies HTTP 404
ErrAlreadyExist = errors.New("resource already exists") // also implies HTTP 409

// ErrUnprocessableContent implies HTTP 422, syntax of the request content was correct,
// but server was unable to process the contained instructions
ErrUnprocessableContent = errors.New("unprocessable content")
)

// SilentWrap provides a simple wrapper for a wrapped error where the wrapped error message plays no part in the error message
Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/admin/email.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func GetAllEmails(ctx *context.APIContext) {
ListOptions: listOptions,
})
if err != nil {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
return
}

Expand Down
10 changes: 5 additions & 5 deletions routers/api/v1/admin/hooks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,14 @@ func ListHooks(ctx *context.APIContext) {

sysHooks, err := webhook.GetSystemOrDefaultWebhooks(ctx, isSystemWebhook)
if err != nil {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
return
}
hooks := make([]*api.Hook, len(sysHooks))
for i, hook := range sysHooks {
h, err := webhook_service.ToHook(setting.AppURL+"/-/admin", hook)
if err != nil {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
return
}
hooks[i] = h
Expand Down Expand Up @@ -98,13 +98,13 @@ func GetHook(ctx *context.APIContext) {
if errors.Is(err, util.ErrNotExist) {
ctx.APIErrorNotFound()
} else {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
}
return
}
h, err := webhook_service.ToHook("/-/admin/", hook)
if err != nil {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
return
}
ctx.JSON(http.StatusOK, h)
Expand Down Expand Up @@ -188,7 +188,7 @@ func DeleteHook(ctx *context.APIContext) {
if errors.Is(err, util.ErrNotExist) {
ctx.APIErrorNotFound()
} else {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
}
return
}
Expand Down
4 changes: 2 additions & 2 deletions routers/api/v1/admin/org.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func CreateOrg(ctx *context.APIContext) {
db.IsErrNamePatternNotAllowed(err) {
ctx.APIError(http.StatusUnprocessableEntity, err)
} else {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
}
return
}
Expand Down Expand Up @@ -109,7 +109,7 @@ func GetAllOrgs(ctx *context.APIContext) {
Visible: []api.VisibleType{api.VisibleTypePublic, api.VisibleTypeLimited, api.VisibleTypePrivate},
})
if err != nil {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
return
}
orgs := make([]*api.Organization, len(users))
Expand Down
16 changes: 8 additions & 8 deletions routers/api/v1/admin/user.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func parseAuthSource(ctx *context.APIContext, u *user_model.User, sourceID int64
if auth.IsErrSourceNotExist(err) {
ctx.APIError(http.StatusUnprocessableEntity, err)
} else {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
}
return
}
Expand Down Expand Up @@ -145,7 +145,7 @@ func CreateUser(ctx *context.APIContext) {
db.IsErrNamePatternNotAllowed(err) {
ctx.APIError(http.StatusUnprocessableEntity, err)
} else {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
}
return
}
Expand Down Expand Up @@ -210,7 +210,7 @@ func EditUser(ctx *context.APIContext) {
case errors.Is(err, password.ErrIsPwned), password.IsErrIsPwnedRequest(err):
ctx.APIError(http.StatusBadRequest, err)
default:
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
}
return
}
Expand All @@ -223,7 +223,7 @@ func EditUser(ctx *context.APIContext) {
case user_model.IsErrEmailAlreadyUsed(err):
ctx.APIError(http.StatusBadRequest, err)
default:
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
}
return
}
Expand Down Expand Up @@ -252,7 +252,7 @@ func EditUser(ctx *context.APIContext) {
if user_model.IsErrDeleteLastAdminUser(err) {
ctx.APIError(http.StatusBadRequest, err)
} else {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
}
return
}
Expand Down Expand Up @@ -307,7 +307,7 @@ func DeleteUser(ctx *context.APIContext) {
user_model.IsErrDeleteLastAdminUser(err) {
ctx.APIError(http.StatusUnprocessableEntity, err)
} else {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
}
return
}
Expand Down Expand Up @@ -381,7 +381,7 @@ func DeleteUserPublicKey(ctx *context.APIContext) {
} else if asymkey_model.IsErrKeyAccessDenied(err) {
ctx.APIError(http.StatusForbidden, "You do not have access to this key")
} else {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
}
return
}
Expand Down Expand Up @@ -432,7 +432,7 @@ func SearchUsers(ctx *context.APIContext) {
ListOptions: listOptions,
})
if err != nil {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
return
}

Expand Down
6 changes: 3 additions & 3 deletions routers/api/v1/admin/user_badge.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func ListUserBadges(ctx *context.APIContext) {

badges, maxResults, err := user_model.GetUserBadges(ctx, ctx.ContextUser)
if err != nil {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
return
}

Expand Down Expand Up @@ -70,7 +70,7 @@ func AddUserBadges(ctx *context.APIContext) {
badges := prepareBadgesForReplaceOrAdd(*form)

if err := user_model.AddUserBadges(ctx, ctx.ContextUser, badges); err != nil {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
return
}

Expand Down Expand Up @@ -106,7 +106,7 @@ func DeleteUserBadges(ctx *context.APIContext) {
badges := prepareBadgesForReplaceOrAdd(*form)

if err := user_model.RemoveUserBadges(ctx, ctx.ContextUser, badges); err != nil {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
return
}

Expand Down
42 changes: 23 additions & 19 deletions routers/api/v1/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
package v1

import (
"errors"
"fmt"
"net/http"
"strings"
Expand Down Expand Up @@ -118,7 +119,7 @@ func sudo() func(ctx *context.APIContext) {
if user_model.IsErrUserNotExist(err) {
ctx.APIErrorNotFound()
} else {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
}
return
}
Expand Down Expand Up @@ -156,10 +157,10 @@ func repoAssignment() func(ctx *context.APIContext) {
} else if user_model.IsErrUserRedirectNotExist(err) {
ctx.APIErrorNotFound("GetUserByName", err)
} else {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
}
} else {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
}
return
}
Expand All @@ -177,10 +178,10 @@ func repoAssignment() func(ctx *context.APIContext) {
} else if repo_model.IsErrRedirectNotExist(err) {
ctx.APIErrorNotFound()
} else {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
}
} else {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
}
return
}
Expand All @@ -192,7 +193,7 @@ func repoAssignment() func(ctx *context.APIContext) {
taskID := ctx.Data["ActionsTaskID"].(int64)
task, err := actions_model.GetTaskByID(ctx, taskID)
if err != nil {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
return
}
if task.RepoID != repo.ID {
Expand All @@ -207,14 +208,14 @@ func repoAssignment() func(ctx *context.APIContext) {
}

if err := ctx.Repo.Repository.LoadUnits(ctx); err != nil {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
return
}
ctx.Repo.Permission.SetUnitsWithDefaultAccessMode(ctx.Repo.Repository.Units, ctx.Repo.Permission.AccessMode)
} else {
ctx.Repo.Permission, err = access_model.GetUserRepoPermission(ctx, repo, ctx.Doer)
if err != nil {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
return
}
}
Expand Down Expand Up @@ -474,13 +475,14 @@ func reqOrgOwnership() func(ctx *context.APIContext) {
} else if ctx.Org.Team != nil {
orgID = ctx.Org.Team.OrgID
} else {
ctx.APIError(http.StatusInternalServerError, "reqOrgOwnership: unprepared context")
setting.PanicInDevOrTesting("reqOrgOwnership: unprepared context")
ctx.APIErrorInternal(errors.New("reqOrgOwnership: unprepared context"))
return
}

isOwner, err := organization.IsOrganizationOwner(ctx, orgID, ctx.Doer.ID)
if err != nil {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
return
} else if !isOwner {
if ctx.Org.Organization != nil {
Expand All @@ -500,26 +502,27 @@ func reqTeamMembership() func(ctx *context.APIContext) {
return
}
if ctx.Org.Team == nil {
ctx.APIError(http.StatusInternalServerError, "reqTeamMembership: unprepared context")
setting.PanicInDevOrTesting("reqTeamMembership: unprepared context")
ctx.APIErrorInternal(errors.New("reqTeamMembership: unprepared context"))
return
}

orgID := ctx.Org.Team.OrgID
isOwner, err := organization.IsOrganizationOwner(ctx, orgID, ctx.Doer.ID)
if err != nil {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
return
} else if isOwner {
return
}

if isTeamMember, err := organization.IsTeamMember(ctx, orgID, ctx.Org.Team.ID, ctx.Doer.ID); err != nil {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
return
} else if !isTeamMember {
isOrgMember, err := organization.IsOrganizationMember(ctx, orgID, ctx.Doer.ID)
if err != nil {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
} else if isOrgMember {
ctx.APIError(http.StatusForbidden, "Must be a team member")
} else {
Expand All @@ -543,12 +546,13 @@ func reqOrgMembership() func(ctx *context.APIContext) {
} else if ctx.Org.Team != nil {
orgID = ctx.Org.Team.OrgID
} else {
ctx.APIError(http.StatusInternalServerError, "reqOrgMembership: unprepared context")
setting.PanicInDevOrTesting("reqOrgMembership: unprepared context")
ctx.APIErrorInternal(errors.New("reqOrgMembership: unprepared context"))
return
}

if isMember, err := organization.IsOrganizationMember(ctx, orgID, ctx.Doer.ID); err != nil {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
return
} else if !isMember {
if ctx.Org.Organization != nil {
Expand Down Expand Up @@ -615,10 +619,10 @@ func orgAssignment(args ...bool) func(ctx *context.APIContext) {
} else if user_model.IsErrUserRedirectNotExist(err) {
ctx.APIErrorNotFound("GetOrgByName", err)
} else {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
}
} else {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
}
return
}
Expand All @@ -631,7 +635,7 @@ func orgAssignment(args ...bool) func(ctx *context.APIContext) {
if organization.IsErrTeamNotExist(err) {
ctx.APIErrorNotFound()
} else {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
}
return
}
Expand Down
5 changes: 2 additions & 3 deletions routers/api/v1/misc/signing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package misc

import (
"fmt"
"net/http"

asymkey_service "code.gitea.io/gitea/services/asymkey"
"code.gitea.io/gitea/services/context"
Expand Down Expand Up @@ -53,11 +52,11 @@ func SigningKey(ctx *context.APIContext) {

content, err := asymkey_service.PublicSigningKey(ctx, path)
if err != nil {
ctx.APIError(http.StatusInternalServerError, err)
ctx.APIErrorInternal(err)
return
}
_, err = ctx.Write([]byte(content))
if err != nil {
ctx.APIError(http.StatusInternalServerError, fmt.Errorf("Error writing key content %w", err))
ctx.APIErrorInternal(fmt.Errorf("Error writing key content %w", err))
}
}
Loading
Loading