Skip to content

Refactor error system #33771

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.

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

Merged
merged 2 commits into from
Mar 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion models/asymkey/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func (err ErrKeyUnableVerify) Error() string {
}

// ErrKeyIsPrivate is returned when the provided key is a private key not a public key
var ErrKeyIsPrivate = util.NewSilentWrapErrorf(util.ErrInvalidArgument, "the provided key is a private key")
var ErrKeyIsPrivate = util.ErrorWrap(util.ErrInvalidArgument, "the provided key is a private key")

// ErrKeyNotExist represents a "KeyNotExist" kind of error.
type ErrKeyNotExist struct {
Expand Down
2 changes: 1 addition & 1 deletion models/db/name.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func (err ErrNameCharsNotAllowed) Unwrap() error {
func IsUsableName(reservedNames, reservedPatterns []string, name string) error {
name = strings.TrimSpace(strings.ToLower(name))
if utf8.RuneCountInString(name) == 0 {
return util.SilentWrap{Message: "name is empty", Err: util.ErrInvalidArgument}
return util.NewInvalidArgumentErrorf("name is empty")
}

for i := range reservedNames {
Expand Down
6 changes: 3 additions & 3 deletions models/repo/archiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,15 @@ func (archiver *RepoArchiver) RelativePath() string {
func repoArchiverForRelativePath(relativePath string) (*RepoArchiver, error) {
parts := strings.SplitN(relativePath, "/", 3)
if len(parts) != 3 {
return nil, util.SilentWrap{Message: fmt.Sprintf("invalid storage path: %s", relativePath), Err: util.ErrInvalidArgument}
return nil, util.NewInvalidArgumentErrorf("invalid storage path: must have 3 parts")
}
repoID, err := strconv.ParseInt(parts[0], 10, 64)
if err != nil {
return nil, util.SilentWrap{Message: fmt.Sprintf("invalid storage path: %s", relativePath), Err: util.ErrInvalidArgument}
return nil, util.NewInvalidArgumentErrorf("invalid storage path: invalid repo id")
}
commitID, archiveType := git.SplitArchiveNameType(parts[2])
if archiveType == git.ArchiveUnknown {
return nil, util.SilentWrap{Message: fmt.Sprintf("invalid storage path: %s", relativePath), Err: util.ErrInvalidArgument}
return nil, util.NewInvalidArgumentErrorf("invalid storage path: invalid archive type")
}
return &RepoArchiver{RepoID: repoID, CommitID: commitID, Type: archiveType}, nil
}
Expand Down
2 changes: 1 addition & 1 deletion models/user/must_change_password.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func SetMustChangePassword(ctx context.Context, all, mustChangePassword bool, in
if !all {
include = sliceTrimSpaceDropEmpty(include)
if len(include) == 0 {
return 0, util.NewSilentWrapErrorf(util.ErrInvalidArgument, "no users to include provided")
return 0, util.ErrorWrap(util.ErrInvalidArgument, "no users to include provided")
}

cond = cond.And(builder.In("lower_name", include))
Expand Down
6 changes: 3 additions & 3 deletions modules/packages/conda/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ import (
)

var (
ErrInvalidStructure = util.SilentWrap{Message: "package structure is invalid", Err: util.ErrInvalidArgument}
ErrInvalidName = util.SilentWrap{Message: "package name is invalid", Err: util.ErrInvalidArgument}
ErrInvalidVersion = util.SilentWrap{Message: "package version is invalid", Err: util.ErrInvalidArgument}
ErrInvalidStructure = util.NewInvalidArgumentErrorf("package structure is invalid")
ErrInvalidName = util.NewInvalidArgumentErrorf("package name is invalid")
ErrInvalidVersion = util.NewInvalidArgumentErrorf("package version is invalid")
)

const (
Expand Down
13 changes: 0 additions & 13 deletions modules/translation/i18n/errors.go

This file was deleted.

3 changes: 2 additions & 1 deletion modules/translation/i18n/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package i18n

import (
"errors"
"fmt"
"reflect"
)
Expand All @@ -30,7 +31,7 @@ func Format(format string, args ...any) (msg string, err error) {
fmtArgs = append(fmtArgs, val.Index(i).Interface())
}
} else {
err = ErrUncertainArguments
err = errors.New("arguments to i18n should not contain uncertain slices")
break
}
} else {
Expand Down
3 changes: 2 additions & 1 deletion modules/translation/i18n/localestore.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package i18n

import (
"errors"
"fmt"
"html/template"
"slices"
Expand Down Expand Up @@ -41,7 +42,7 @@ func NewLocaleStore() LocaleStore {
// AddLocaleByIni adds locale by ini into the store
func (store *localeStore) AddLocaleByIni(langName, langDesc string, source, moreSource []byte) error {
if _, ok := store.localeMap[langName]; ok {
return ErrLocaleAlreadyExist
return errors.New("lang has already been added")
}

store.langNames = append(store.langNames, langName)
Expand Down
40 changes: 20 additions & 20 deletions modules/util/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,74 +22,74 @@ var (
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
// errorWrapper provides a simple wrapper for a wrapped error where the wrapped error message plays no part in the error message
// Especially useful for "untyped" errors created with "errors.New(…)" that can be classified as 'invalid argument', 'permission denied', 'exists already', or 'does not exist'
type SilentWrap struct {
type errorWrapper struct {
Message string
Err error
}

// Error returns the message
func (w SilentWrap) Error() string {
func (w errorWrapper) Error() string {
return w.Message
}

// Unwrap returns the underlying error
func (w SilentWrap) Unwrap() error {
func (w errorWrapper) Unwrap() error {
return w.Err
}

type LocaleWrap struct {
type LocaleWrapper struct {
err error
TrKey string
TrArgs []any
}

// Error returns the message
func (w LocaleWrap) Error() string {
func (w LocaleWrapper) Error() string {
return w.err.Error()
}

// Unwrap returns the underlying error
func (w LocaleWrap) Unwrap() error {
func (w LocaleWrapper) Unwrap() error {
return w.err
}

// NewSilentWrapErrorf returns an error that formats as the given text but unwraps as the provided error
func NewSilentWrapErrorf(unwrap error, message string, args ...any) error {
// ErrorWrap returns an error that formats as the given text but unwraps as the provided error
func ErrorWrap(unwrap error, message string, args ...any) error {
if len(args) == 0 {
return SilentWrap{Message: message, Err: unwrap}
return errorWrapper{Message: message, Err: unwrap}
}
return SilentWrap{Message: fmt.Sprintf(message, args...), Err: unwrap}
return errorWrapper{Message: fmt.Sprintf(message, args...), Err: unwrap}
}

// NewInvalidArgumentErrorf returns an error that formats as the given text but unwraps as an ErrInvalidArgument
func NewInvalidArgumentErrorf(message string, args ...any) error {
return NewSilentWrapErrorf(ErrInvalidArgument, message, args...)
return ErrorWrap(ErrInvalidArgument, message, args...)
}

// NewPermissionDeniedErrorf returns an error that formats as the given text but unwraps as an ErrPermissionDenied
func NewPermissionDeniedErrorf(message string, args ...any) error {
return NewSilentWrapErrorf(ErrPermissionDenied, message, args...)
return ErrorWrap(ErrPermissionDenied, message, args...)
}

// NewAlreadyExistErrorf returns an error that formats as the given text but unwraps as an ErrAlreadyExist
func NewAlreadyExistErrorf(message string, args ...any) error {
return NewSilentWrapErrorf(ErrAlreadyExist, message, args...)
return ErrorWrap(ErrAlreadyExist, message, args...)
}

// NewNotExistErrorf returns an error that formats as the given text but unwraps as an ErrNotExist
func NewNotExistErrorf(message string, args ...any) error {
return NewSilentWrapErrorf(ErrNotExist, message, args...)
return ErrorWrap(ErrNotExist, message, args...)
}

// ErrWrapLocale wraps an err with a translation key and arguments
func ErrWrapLocale(err error, trKey string, trArgs ...any) error {
return LocaleWrap{err: err, TrKey: trKey, TrArgs: trArgs}
// ErrorWrapLocale wraps an err with a translation key and arguments
func ErrorWrapLocale(err error, trKey string, trArgs ...any) error {
return LocaleWrapper{err: err, TrKey: trKey, TrArgs: trArgs}
}

func ErrAsLocale(err error) *LocaleWrap {
var e LocaleWrap
func ErrorAsLocale(err error) *LocaleWrapper {
var e LocaleWrapper
if errors.As(err, &e) {
return &e
}
Expand Down
2 changes: 1 addition & 1 deletion routers/web/repo/actions/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,7 @@ func Run(ctx *context_module.Context) {
return nil
})
if err != nil {
if errLocale := util.ErrAsLocale(err); errLocale != nil {
if errLocale := util.ErrorAsLocale(err); errLocale != nil {
ctx.Flash.Error(ctx.Tr(errLocale.TrKey, errLocale.TrArgs...))
ctx.Redirect(redirectURL)
} else {
Expand Down
10 changes: 5 additions & 5 deletions services/actions/workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,14 +141,14 @@ func GetActionWorkflow(ctx *context.APIContext, workflowID string) (*api.ActionW

func DispatchActionWorkflow(ctx reqctx.RequestContext, doer *user_model.User, repo *repo_model.Repository, gitRepo *git.Repository, workflowID, ref string, processInputs func(model *model.WorkflowDispatch, inputs map[string]any) error) error {
if workflowID == "" {
return util.ErrWrapLocale(
return util.ErrorWrapLocale(
util.NewNotExistErrorf("workflowID is empty"),
"actions.workflow.not_found", workflowID,
)
}

if ref == "" {
return util.ErrWrapLocale(
return util.ErrorWrapLocale(
util.NewNotExistErrorf("ref is empty"),
"form.target_ref_not_exist", ref,
)
Expand All @@ -158,7 +158,7 @@ func DispatchActionWorkflow(ctx reqctx.RequestContext, doer *user_model.User, re
cfgUnit := repo.MustGetUnit(ctx, unit.TypeActions)
cfg := cfgUnit.ActionsConfig()
if cfg.IsWorkflowDisabled(workflowID) {
return util.ErrWrapLocale(
return util.ErrorWrapLocale(
util.NewPermissionDeniedErrorf("workflow is disabled"),
"actions.workflow.disabled",
)
Expand All @@ -177,7 +177,7 @@ func DispatchActionWorkflow(ctx reqctx.RequestContext, doer *user_model.User, re
runTargetCommit, err = gitRepo.GetBranchCommit(ref)
}
if err != nil {
return util.ErrWrapLocale(
return util.ErrorWrapLocale(
util.NewNotExistErrorf("ref %q doesn't exist", ref),
"form.target_ref_not_exist", ref,
)
Expand Down Expand Up @@ -208,7 +208,7 @@ func DispatchActionWorkflow(ctx reqctx.RequestContext, doer *user_model.User, re
}

if len(workflows) == 0 {
return util.ErrWrapLocale(
return util.ErrorWrapLocale(
util.NewNotExistErrorf("workflow %q doesn't exist", workflowID),
"actions.workflow.not_found", workflowID,
)
Expand Down
10 changes: 2 additions & 8 deletions services/release/release.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,10 +296,7 @@ func UpdateRelease(ctx context.Context, doer *user_model.User, gitRepo *git.Repo
}
for _, attach := range attachments {
if attach.ReleaseID != rel.ID {
return util.SilentWrap{
Message: "delete attachment of release permission denied",
Err: util.ErrPermissionDenied,
}
return util.NewPermissionDeniedErrorf("delete attachment of release permission denied")
}
deletedUUIDs.Add(attach.UUID)
}
Expand All @@ -321,10 +318,7 @@ func UpdateRelease(ctx context.Context, doer *user_model.User, gitRepo *git.Repo
}
for _, attach := range attachments {
if attach.ReleaseID != rel.ID {
return util.SilentWrap{
Message: "update attachment of release permission denied",
Err: util.ErrPermissionDenied,
}
return util.NewPermissionDeniedErrorf("update attachment of release permission denied")
}
}

Expand Down
Loading