Skip to content

Commit 15e020e

Browse files
wxiaoguangwxiaoguang
authored
Refactor error system (#33626)
1 parent 7df09e3 commit 15e020e

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

75 files changed

+703
-693
lines changed

Diff for: ‎modules/util/error.go

+9-6
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,16 @@ import (
1010

1111
// Common Errors forming the base of our error system
1212
//
13-
// Many Errors returned by Gitea can be tested against these errors
14-
// using errors.Is.
13+
// Many Errors returned by Gitea can be tested against these errors using "errors.Is".
1514
var (
16-
ErrInvalidArgument = errors.New("invalid argument")
17-
ErrPermissionDenied = errors.New("permission denied")
18-
ErrAlreadyExist = errors.New("resource already exists")
19-
ErrNotExist = errors.New("resource does not exist")
15+
ErrInvalidArgument = errors.New("invalid argument") // also implies HTTP 400
16+
ErrPermissionDenied = errors.New("permission denied") // also implies HTTP 403
17+
ErrNotExist = errors.New("resource does not exist") // also implies HTTP 404
18+
ErrAlreadyExist = errors.New("resource already exists") // also implies HTTP 409
19+
20+
// ErrUnprocessableContent implies HTTP 422, syntax of the request content was correct,
21+
// but server was unable to process the contained instructions
22+
ErrUnprocessableContent = errors.New("unprocessable content")
2023
)
2124

2225
// SilentWrap provides a simple wrapper for a wrapped error where the wrapped error message plays no part in the error message

Diff for: ‎routers/api/v1/admin/email.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func GetAllEmails(ctx *context.APIContext) {
4242
ListOptions: listOptions,
4343
})
4444
if err != nil {
45-
ctx.APIError(http.StatusInternalServerError, err)
45+
ctx.APIErrorInternal(err)
4646
return
4747
}
4848

Diff for: ‎routers/api/v1/admin/hooks.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,14 @@ func ListHooks(ctx *context.APIContext) {
5959

6060
sysHooks, err := webhook.GetSystemOrDefaultWebhooks(ctx, isSystemWebhook)
6161
if err != nil {
62-
ctx.APIError(http.StatusInternalServerError, err)
62+
ctx.APIErrorInternal(err)
6363
return
6464
}
6565
hooks := make([]*api.Hook, len(sysHooks))
6666
for i, hook := range sysHooks {
6767
h, err := webhook_service.ToHook(setting.AppURL+"/-/admin", hook)
6868
if err != nil {
69-
ctx.APIError(http.StatusInternalServerError, err)
69+
ctx.APIErrorInternal(err)
7070
return
7171
}
7272
hooks[i] = h
@@ -98,13 +98,13 @@ func GetHook(ctx *context.APIContext) {
9898
if errors.Is(err, util.ErrNotExist) {
9999
ctx.APIErrorNotFound()
100100
} else {
101-
ctx.APIError(http.StatusInternalServerError, err)
101+
ctx.APIErrorInternal(err)
102102
}
103103
return
104104
}
105105
h, err := webhook_service.ToHook("/-/admin/", hook)
106106
if err != nil {
107-
ctx.APIError(http.StatusInternalServerError, err)
107+
ctx.APIErrorInternal(err)
108108
return
109109
}
110110
ctx.JSON(http.StatusOK, h)
@@ -188,7 +188,7 @@ func DeleteHook(ctx *context.APIContext) {
188188
if errors.Is(err, util.ErrNotExist) {
189189
ctx.APIErrorNotFound()
190190
} else {
191-
ctx.APIError(http.StatusInternalServerError, err)
191+
ctx.APIErrorInternal(err)
192192
}
193193
return
194194
}

Diff for: ‎routers/api/v1/admin/org.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ func CreateOrg(ctx *context.APIContext) {
6969
db.IsErrNamePatternNotAllowed(err) {
7070
ctx.APIError(http.StatusUnprocessableEntity, err)
7171
} else {
72-
ctx.APIError(http.StatusInternalServerError, err)
72+
ctx.APIErrorInternal(err)
7373
}
7474
return
7575
}
@@ -109,7 +109,7 @@ func GetAllOrgs(ctx *context.APIContext) {
109109
Visible: []api.VisibleType{api.VisibleTypePublic, api.VisibleTypeLimited, api.VisibleTypePrivate},
110110
})
111111
if err != nil {
112-
ctx.APIError(http.StatusInternalServerError, err)
112+
ctx.APIErrorInternal(err)
113113
return
114114
}
115115
orgs := make([]*api.Organization, len(users))

Diff for: ‎routers/api/v1/admin/user.go

+8-8
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func parseAuthSource(ctx *context.APIContext, u *user_model.User, sourceID int64
4242
if auth.IsErrSourceNotExist(err) {
4343
ctx.APIError(http.StatusUnprocessableEntity, err)
4444
} else {
45-
ctx.APIError(http.StatusInternalServerError, err)
45+
ctx.APIErrorInternal(err)
4646
}
4747
return
4848
}
@@ -145,7 +145,7 @@ func CreateUser(ctx *context.APIContext) {
145145
db.IsErrNamePatternNotAllowed(err) {
146146
ctx.APIError(http.StatusUnprocessableEntity, err)
147147
} else {
148-
ctx.APIError(http.StatusInternalServerError, err)
148+
ctx.APIErrorInternal(err)
149149
}
150150
return
151151
}
@@ -210,7 +210,7 @@ func EditUser(ctx *context.APIContext) {
210210
case errors.Is(err, password.ErrIsPwned), password.IsErrIsPwnedRequest(err):
211211
ctx.APIError(http.StatusBadRequest, err)
212212
default:
213-
ctx.APIError(http.StatusInternalServerError, err)
213+
ctx.APIErrorInternal(err)
214214
}
215215
return
216216
}
@@ -223,7 +223,7 @@ func EditUser(ctx *context.APIContext) {
223223
case user_model.IsErrEmailAlreadyUsed(err):
224224
ctx.APIError(http.StatusBadRequest, err)
225225
default:
226-
ctx.APIError(http.StatusInternalServerError, err)
226+
ctx.APIErrorInternal(err)
227227
}
228228
return
229229
}
@@ -252,7 +252,7 @@ func EditUser(ctx *context.APIContext) {
252252
if user_model.IsErrDeleteLastAdminUser(err) {
253253
ctx.APIError(http.StatusBadRequest, err)
254254
} else {
255-
ctx.APIError(http.StatusInternalServerError, err)
255+
ctx.APIErrorInternal(err)
256256
}
257257
return
258258
}
@@ -307,7 +307,7 @@ func DeleteUser(ctx *context.APIContext) {
307307
user_model.IsErrDeleteLastAdminUser(err) {
308308
ctx.APIError(http.StatusUnprocessableEntity, err)
309309
} else {
310-
ctx.APIError(http.StatusInternalServerError, err)
310+
ctx.APIErrorInternal(err)
311311
}
312312
return
313313
}
@@ -381,7 +381,7 @@ func DeleteUserPublicKey(ctx *context.APIContext) {
381381
} else if asymkey_model.IsErrKeyAccessDenied(err) {
382382
ctx.APIError(http.StatusForbidden, "You do not have access to this key")
383383
} else {
384-
ctx.APIError(http.StatusInternalServerError, err)
384+
ctx.APIErrorInternal(err)
385385
}
386386
return
387387
}
@@ -432,7 +432,7 @@ func SearchUsers(ctx *context.APIContext) {
432432
ListOptions: listOptions,
433433
})
434434
if err != nil {
435-
ctx.APIError(http.StatusInternalServerError, err)
435+
ctx.APIErrorInternal(err)
436436
return
437437
}
438438

Diff for: ‎routers/api/v1/admin/user_badge.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ func ListUserBadges(ctx *context.APIContext) {
3333

3434
badges, maxResults, err := user_model.GetUserBadges(ctx, ctx.ContextUser)
3535
if err != nil {
36-
ctx.APIError(http.StatusInternalServerError, err)
36+
ctx.APIErrorInternal(err)
3737
return
3838
}
3939

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

7272
if err := user_model.AddUserBadges(ctx, ctx.ContextUser, badges); err != nil {
73-
ctx.APIError(http.StatusInternalServerError, err)
73+
ctx.APIErrorInternal(err)
7474
return
7575
}
7676

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

108108
if err := user_model.RemoveUserBadges(ctx, ctx.ContextUser, badges); err != nil {
109-
ctx.APIError(http.StatusInternalServerError, err)
109+
ctx.APIErrorInternal(err)
110110
return
111111
}
112112

Diff for: ‎routers/api/v1/api.go

+23-19
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@
6666
package v1
6767

6868
import (
69+
"errors"
6970
"fmt"
7071
"net/http"
7172
"strings"
@@ -118,7 +119,7 @@ func sudo() func(ctx *context.APIContext) {
118119
if user_model.IsErrUserNotExist(err) {
119120
ctx.APIErrorNotFound()
120121
} else {
121-
ctx.APIError(http.StatusInternalServerError, err)
122+
ctx.APIErrorInternal(err)
122123
}
123124
return
124125
}
@@ -156,10 +157,10 @@ func repoAssignment() func(ctx *context.APIContext) {
156157
} else if user_model.IsErrUserRedirectNotExist(err) {
157158
ctx.APIErrorNotFound("GetUserByName", err)
158159
} else {
159-
ctx.APIError(http.StatusInternalServerError, err)
160+
ctx.APIErrorInternal(err)
160161
}
161162
} else {
162-
ctx.APIError(http.StatusInternalServerError, err)
163+
ctx.APIErrorInternal(err)
163164
}
164165
return
165166
}
@@ -177,10 +178,10 @@ func repoAssignment() func(ctx *context.APIContext) {
177178
} else if repo_model.IsErrRedirectNotExist(err) {
178179
ctx.APIErrorNotFound()
179180
} else {
180-
ctx.APIError(http.StatusInternalServerError, err)
181+
ctx.APIErrorInternal(err)
181182
}
182183
} else {
183-
ctx.APIError(http.StatusInternalServerError, err)
184+
ctx.APIErrorInternal(err)
184185
}
185186
return
186187
}
@@ -192,7 +193,7 @@ func repoAssignment() func(ctx *context.APIContext) {
192193
taskID := ctx.Data["ActionsTaskID"].(int64)
193194
task, err := actions_model.GetTaskByID(ctx, taskID)
194195
if err != nil {
195-
ctx.APIError(http.StatusInternalServerError, err)
196+
ctx.APIErrorInternal(err)
196197
return
197198
}
198199
if task.RepoID != repo.ID {
@@ -207,14 +208,14 @@ func repoAssignment() func(ctx *context.APIContext) {
207208
}
208209

209210
if err := ctx.Repo.Repository.LoadUnits(ctx); err != nil {
210-
ctx.APIError(http.StatusInternalServerError, err)
211+
ctx.APIErrorInternal(err)
211212
return
212213
}
213214
ctx.Repo.Permission.SetUnitsWithDefaultAccessMode(ctx.Repo.Repository.Units, ctx.Repo.Permission.AccessMode)
214215
} else {
215216
ctx.Repo.Permission, err = access_model.GetUserRepoPermission(ctx, repo, ctx.Doer)
216217
if err != nil {
217-
ctx.APIError(http.StatusInternalServerError, err)
218+
ctx.APIErrorInternal(err)
218219
return
219220
}
220221
}
@@ -474,13 +475,14 @@ func reqOrgOwnership() func(ctx *context.APIContext) {
474475
} else if ctx.Org.Team != nil {
475476
orgID = ctx.Org.Team.OrgID
476477
} else {
477-
ctx.APIError(http.StatusInternalServerError, "reqOrgOwnership: unprepared context")
478+
setting.PanicInDevOrTesting("reqOrgOwnership: unprepared context")
479+
ctx.APIErrorInternal(errors.New("reqOrgOwnership: unprepared context"))
478480
return
479481
}
480482

481483
isOwner, err := organization.IsOrganizationOwner(ctx, orgID, ctx.Doer.ID)
482484
if err != nil {
483-
ctx.APIError(http.StatusInternalServerError, err)
485+
ctx.APIErrorInternal(err)
484486
return
485487
} else if !isOwner {
486488
if ctx.Org.Organization != nil {
@@ -500,26 +502,27 @@ func reqTeamMembership() func(ctx *context.APIContext) {
500502
return
501503
}
502504
if ctx.Org.Team == nil {
503-
ctx.APIError(http.StatusInternalServerError, "reqTeamMembership: unprepared context")
505+
setting.PanicInDevOrTesting("reqTeamMembership: unprepared context")
506+
ctx.APIErrorInternal(errors.New("reqTeamMembership: unprepared context"))
504507
return
505508
}
506509

507510
orgID := ctx.Org.Team.OrgID
508511
isOwner, err := organization.IsOrganizationOwner(ctx, orgID, ctx.Doer.ID)
509512
if err != nil {
510-
ctx.APIError(http.StatusInternalServerError, err)
513+
ctx.APIErrorInternal(err)
511514
return
512515
} else if isOwner {
513516
return
514517
}
515518

516519
if isTeamMember, err := organization.IsTeamMember(ctx, orgID, ctx.Org.Team.ID, ctx.Doer.ID); err != nil {
517-
ctx.APIError(http.StatusInternalServerError, err)
520+
ctx.APIErrorInternal(err)
518521
return
519522
} else if !isTeamMember {
520523
isOrgMember, err := organization.IsOrganizationMember(ctx, orgID, ctx.Doer.ID)
521524
if err != nil {
522-
ctx.APIError(http.StatusInternalServerError, err)
525+
ctx.APIErrorInternal(err)
523526
} else if isOrgMember {
524527
ctx.APIError(http.StatusForbidden, "Must be a team member")
525528
} else {
@@ -543,12 +546,13 @@ func reqOrgMembership() func(ctx *context.APIContext) {
543546
} else if ctx.Org.Team != nil {
544547
orgID = ctx.Org.Team.OrgID
545548
} else {
546-
ctx.APIError(http.StatusInternalServerError, "reqOrgMembership: unprepared context")
549+
setting.PanicInDevOrTesting("reqOrgMembership: unprepared context")
550+
ctx.APIErrorInternal(errors.New("reqOrgMembership: unprepared context"))
547551
return
548552
}
549553

550554
if isMember, err := organization.IsOrganizationMember(ctx, orgID, ctx.Doer.ID); err != nil {
551-
ctx.APIError(http.StatusInternalServerError, err)
555+
ctx.APIErrorInternal(err)
552556
return
553557
} else if !isMember {
554558
if ctx.Org.Organization != nil {
@@ -615,10 +619,10 @@ func orgAssignment(args ...bool) func(ctx *context.APIContext) {
615619
} else if user_model.IsErrUserRedirectNotExist(err) {
616620
ctx.APIErrorNotFound("GetOrgByName", err)
617621
} else {
618-
ctx.APIError(http.StatusInternalServerError, err)
622+
ctx.APIErrorInternal(err)
619623
}
620624
} else {
621-
ctx.APIError(http.StatusInternalServerError, err)
625+
ctx.APIErrorInternal(err)
622626
}
623627
return
624628
}
@@ -631,7 +635,7 @@ func orgAssignment(args ...bool) func(ctx *context.APIContext) {
631635
if organization.IsErrTeamNotExist(err) {
632636
ctx.APIErrorNotFound()
633637
} else {
634-
ctx.APIError(http.StatusInternalServerError, err)
638+
ctx.APIErrorInternal(err)
635639
}
636640
return
637641
}

Diff for: ‎routers/api/v1/misc/signing.go

+2-3
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ package misc
55

66
import (
77
"fmt"
8-
"net/http"
98

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

5453
content, err := asymkey_service.PublicSigningKey(ctx, path)
5554
if err != nil {
56-
ctx.APIError(http.StatusInternalServerError, err)
55+
ctx.APIErrorInternal(err)
5756
return
5857
}
5958
_, err = ctx.Write([]byte(content))
6059
if err != nil {
61-
ctx.APIError(http.StatusInternalServerError, fmt.Errorf("Error writing key content %w", err))
60+
ctx.APIErrorInternal(fmt.Errorf("Error writing key content %w", err))
6261
}
6362
}

0 commit comments

Comments
 (0)