Clean up log messages (#30313)

wxiaoguang · GiteaBot · web-flow · commit 83f83019ef34 · 2024-04-07T19:17:06.000+08:00
`log.Xxx("%v")` is not ideal, this PR adds necessary context messages.
Remove some unnecessary logs.

Co-authored-by: Giteabot &lt;teabot@gitea.io&gt;
diff --git a/cmd/web.go b/cmd/web.go
@@ -114,7 +114,7 @@ func showWebStartupMessage(msg string) {
 	log.Info("* WorkPath: %s", setting.AppWorkPath)
 	log.Info("* CustomPath: %s", setting.CustomPath)
 	log.Info("* ConfigFile: %s", setting.CustomConf)
-	log.Info("%s", msg)
+	log.Info("%s", msg) // show startup message
 }
 
 func serveInstall(ctx *cli.Context) error {
diff --git a/models/asymkey/ssh_key_fingerprint.go b/models/asymkey/ssh_key_fingerprint.go
@@ -76,23 +76,14 @@ func calcFingerprintNative(publicKeyContent string) (string, error) {
 // CalcFingerprint calculate public key's fingerprint
 func CalcFingerprint(publicKeyContent string) (string, error) {
 	// Call the method based on configuration
-	var (
-		fnName, fp string
-		err        error
-	)
-	if len(setting.SSH.KeygenPath) == 0 {
-		fnName = "calcFingerprintNative"
-		fp, err = calcFingerprintNative(publicKeyContent)
-	} else {
-		fnName = "calcFingerprintSSHKeygen"
-		fp, err = calcFingerprintSSHKeygen(publicKeyContent)
-	}
+	useNative := setting.SSH.KeygenPath == ""
+	calcFn := util.Iif(useNative, calcFingerprintNative, calcFingerprintSSHKeygen)
+	fp, err := calcFn(publicKeyContent)
 	if err != nil {
 		if IsErrKeyUnableVerify(err) {
-			log.Info("%s", publicKeyContent)
 			return "", err
 		}
-		return "", fmt.Errorf("%s: %w", fnName, err)
+		return "", fmt.Errorf("CalcFingerprint(%s): %w", util.Iif(useNative, "native", "ssh-keygen"), err)
 	}
 	return fp, nil
 }
diff --git a/models/repo/issue.go b/models/repo/issue.go
@@ -53,7 +53,7 @@ func (repo *Repository) IsDependenciesEnabled(ctx context.Context) bool {
 	var u *RepoUnit
 	var err error
 	if u, err = repo.GetUnit(ctx, unit.TypeIssues); err != nil {
-		log.Trace("%s", err)
+		log.Trace("IsDependenciesEnabled: %v", err)
 		return setting.Service.DefaultEnableDependencies
 	}
 	return u.IssuesConfig().EnableDependencies
diff --git a/modules/util/util.go b/modules/util/util.go
@@ -214,7 +214,7 @@ func ToPointer[T any](val T) *T {
 }
 
 // Iif is an "inline-if", it returns "trueVal" if "condition" is true, otherwise "falseVal"
-func Iif[T comparable](condition bool, trueVal, falseVal T) T {
+func Iif[T any](condition bool, trueVal, falseVal T) T {
 	if condition {
 		return trueVal
 	}
diff --git a/routers/api/v1/notify/repo.go b/routers/api/v1/notify/repo.go
@@ -10,7 +10,6 @@ import (
 
 	activities_model "code.gitea.io/gitea/models/activities"
 	"code.gitea.io/gitea/models/db"
-	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/services/context"
 	"code.gitea.io/gitea/services/convert"
@@ -201,7 +200,6 @@ func ReadRepoNotifications(ctx *context.APIContext) {
 	if !ctx.FormBool("all") {
 		statuses := ctx.FormStrings("status-types")
 		opts.Status = statusStringsToNotificationStatuses(statuses, []string{"unread"})
-		log.Error("%v", opts.Status)
 	}
 	nl, err := db.Find[activities_model.Notification](ctx, opts)
 	if err != nil {
diff --git a/routers/private/actions.go b/routers/private/actions.go
@@ -26,7 +26,7 @@ func GenerateActionsRunnerToken(ctx *context.PrivateContext) {
 	defer rd.Close()
 
 	if err := json.NewDecoder(rd).Decode(&genRequest); err != nil {
-		log.Error("%v", err)
+		log.Error("JSON Decode failed: %v", err)
 		ctx.JSON(http.StatusInternalServerError, private.Response{
 			Err: err.Error(),
 		})
@@ -35,7 +35,7 @@ func GenerateActionsRunnerToken(ctx *context.PrivateContext) {
 
 	owner, repo, err := parseScope(ctx, genRequest.Scope)
 	if err != nil {
-		log.Error("%v", err)
+		log.Error("parseScope failed: %v", err)
 		ctx.JSON(http.StatusInternalServerError, private.Response{
 			Err: err.Error(),
 		})
@@ -45,18 +45,18 @@ func GenerateActionsRunnerToken(ctx *context.PrivateContext) {
 	if errors.Is(err, util.ErrNotExist) || (token != nil && !token.IsActive) {
 		token, err = actions_model.NewRunnerToken(ctx, owner, repo)
 		if err != nil {
-			err := fmt.Sprintf("error while creating runner token: %v", err)
-			log.Error("%v", err)
+			errMsg := fmt.Sprintf("error while creating runner token: %v", err)
+			log.Error("NewRunnerToken failed: %v", errMsg)
 			ctx.JSON(http.StatusInternalServerError, private.Response{
-				Err: err,
+				Err: errMsg,
 			})
 			return
 		}
 	} else if err != nil {
-		err := fmt.Sprintf("could not get unactivated runner token: %v", err)
-		log.Error("%v", err)
+		errMsg := fmt.Sprintf("could not get unactivated runner token: %v", err)
+		log.Error("GetLatestRunnerToken failed: %v", errMsg)
 		ctx.JSON(http.StatusInternalServerError, private.Response{
-			Err: err,
+			Err: errMsg,
 		})
 		return
 	}
diff --git a/routers/private/hook_verification.go b/routers/private/hook_verification.go
@@ -47,7 +47,7 @@ func verifyCommits(oldCommitID, newCommitID string, repo *git.Repository, env []
 			_ = stdoutWriter.Close()
 			err := readAndVerifyCommitsFromShaReader(stdoutReader, repo, env)
 			if err != nil {
-				log.Error("%v", err)
+				log.Error("readAndVerifyCommitsFromShaReader failed: %v", err)
 				cancel()
 			}
 			_ = stdoutReader.Close()
@@ -66,7 +66,6 @@ func readAndVerifyCommitsFromShaReader(input io.ReadCloser, repo *git.Repository
 		line := scanner.Text()
 		err := readAndVerifyCommit(line, repo, env)
 		if err != nil {
-			log.Error("%v", err)
 			return err
 		}
 	}
diff --git a/routers/private/mail.go b/routers/private/mail.go
@@ -35,7 +35,7 @@ func SendEmail(ctx *context.PrivateContext) {
 	defer rd.Close()
 
 	if err := json.NewDecoder(rd).Decode(&mail); err != nil {
-		log.Error("%v", err)
+		log.Error("JSON Decode failed: %v", err)
 		ctx.JSON(http.StatusInternalServerError, private.Response{
 			Err: err.Error(),
 		})
diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go
@@ -403,7 +403,6 @@ func EditUserPost(ctx *context.Context) {
 			ctx.Data["Err_Password"] = true
 			ctx.RenderWithErr(ctx.Tr("auth.password_pwned"), tplUserEdit, &form)
 		case password.IsErrIsPwnedRequest(err):
-			log.Error("%s", err.Error())
 			ctx.Data["Err_Password"] = true
 			ctx.RenderWithErr(ctx.Tr("auth.password_pwned_err"), tplUserEdit, &form)
 		default:
diff --git a/routers/web/auth/password.go b/routers/web/auth/password.go
@@ -214,7 +214,6 @@ func ResetPasswdPost(ctx *context.Context) {
 		case errors.Is(err, password.ErrIsPwned):
 			ctx.RenderWithErr(ctx.Tr("auth.password_pwned"), tplResetPassword, nil)
 		case password.IsErrIsPwnedRequest(err):
-			log.Error("%s", err.Error())
 			ctx.RenderWithErr(ctx.Tr("auth.password_pwned_err"), tplResetPassword, nil)
 		default:
 			ctx.ServerError("UpdateAuth", err)
@@ -298,7 +297,6 @@ func MustChangePasswordPost(ctx *context.Context) {
 			ctx.Data["Err_Password"] = true
 			ctx.RenderWithErr(ctx.Tr("auth.password_pwned"), tplMustChangePassword, &form)
 		case password.IsErrIsPwnedRequest(err):
-			log.Error("%s", err.Error())
 			ctx.Data["Err_Password"] = true
 			ctx.RenderWithErr(ctx.Tr("auth.password_pwned_err"), tplMustChangePassword, &form)
 		default:
diff --git a/routers/web/user/setting/account.go b/routers/web/user/setting/account.go
@@ -74,7 +74,6 @@ func AccountPost(ctx *context.Context) {
 			case errors.Is(err, password.ErrIsPwned):
 				ctx.Flash.Error(ctx.Tr("auth.password_pwned"))
 			case password.IsErrIsPwnedRequest(err):
-				log.Error("%s", err.Error())
 				ctx.Flash.Error(ctx.Tr("auth.password_pwned_err"))
 			default:
 				ctx.ServerError("UpdateAuth", err)
diff --git a/services/context/captcha.go b/services/context/captcha.go
@@ -79,11 +79,11 @@ func VerifyCaptcha(ctx *Context, tpl base.TplName, form any) {
 	case setting.CfTurnstile:
 		valid, err = turnstile.Verify(ctx, ctx.Req.Form.Get(cfTurnstileResponseField))
 	default:
-		ctx.ServerError("Unknown Captcha Type", fmt.Errorf("Unknown Captcha Type: %s", setting.Service.CaptchaType))
+		ctx.ServerError("Unknown Captcha Type", fmt.Errorf("unknown Captcha Type: %s", setting.Service.CaptchaType))
 		return
 	}
 	if err != nil {
-		log.Debug("%v", err)
+		log.Debug("Captcha Verify failed: %v", err)
 	}
 
 	if !valid {
diff --git a/services/notify/notify.go b/services/notify/notify.go
@@ -91,7 +91,7 @@ func AutoMergePullRequest(ctx context.Context, doer *user_model.User, pr *issues
 // NewPullRequest notifies new pull request to notifiers
 func NewPullRequest(ctx context.Context, pr *issues_model.PullRequest, mentions []*user_model.User) {
 	if err := pr.LoadIssue(ctx); err != nil {
-		log.Error("%v", err)
+		log.Error("LoadIssue failed: %v", err)
 		return
 	}
 	if err := pr.Issue.LoadPoster(ctx); err != nil {
@@ -112,7 +112,7 @@ func PullRequestSynchronized(ctx context.Context, doer *user_model.User, pr *iss
 // PullRequestReview notifies new pull request review
 func PullRequestReview(ctx context.Context, pr *issues_model.PullRequest, review *issues_model.Review, comment *issues_model.Comment, mentions []*user_model.User) {
 	if err := review.LoadReviewer(ctx); err != nil {
-		log.Error("%v", err)
+		log.Error("LoadReviewer failed: %v", err)
 		return
 	}
 	for _, notifier := range notifiers {
diff --git a/services/repository/files/cherry_pick.go b/services/repository/files/cherry_pick.go
@@ -28,7 +28,7 @@ func CherryPick(ctx context.Context, repo *repo_model.Repository, doer *user_mod
 
 	t, err := NewTemporaryUploadRepository(ctx, repo)
 	if err != nil {
-		log.Error("%v", err)
+		log.Error("NewTemporaryUploadRepository failed: %v", err)
 	}
 	defer t.Close()
 	if err := t.Clone(opts.OldBranch, false); err != nil {
diff --git a/services/repository/files/patch.go b/services/repository/files/patch.go
@@ -111,7 +111,7 @@ func ApplyDiffPatch(ctx context.Context, repo *repo_model.Repository, doer *user
 
 	t, err := NewTemporaryUploadRepository(ctx, repo)
 	if err != nil {
-		log.Error("%v", err)
+		log.Error("NewTemporaryUploadRepository failed: %v", err)
 	}
 	defer t.Close()
 	if err := t.Clone(opts.OldBranch, true); err != nil {
diff --git a/services/repository/files/update.go b/services/repository/files/update.go
@@ -143,7 +143,7 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use
 
 	t, err := NewTemporaryUploadRepository(ctx, repo)
 	if err != nil {
-		log.Error("%v", err)
+		log.Error("NewTemporaryUploadRepository failed: %v", err)
 	}
 	defer t.Close()
 	hasOldBranch := true
diff --git a/services/wiki/wiki.go b/services/wiki/wiki.go
@@ -161,7 +161,7 @@ func updateWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model
 		if isOldWikiExist {
 			err := gitRepo.RemoveFilesFromIndex(oldWikiPath)
 			if err != nil {
-				log.Error("%v", err)
+				log.Error("RemoveFilesFromIndex failed: %v", err)
 				return err
 			}
 		}
@@ -171,18 +171,18 @@ func updateWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model
 
 	objectHash, err := gitRepo.HashObject(strings.NewReader(content))
 	if err != nil {
-		log.Error("%v", err)
+		log.Error("HashObject failed: %v", err)
 		return err
 	}
 
 	if err := gitRepo.AddObjectToIndex("100644", objectHash, newWikiPath); err != nil {
-		log.Error("%v", err)
+		log.Error("AddObjectToIndex failed: %v", err)
 		return err
 	}
 
 	tree, err := gitRepo.WriteTree()
 	if err != nil {
-		log.Error("%v", err)
+		log.Error("WriteTree failed: %v", err)
 		return err
 	}
 
@@ -207,7 +207,7 @@ func updateWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model
 
 	commitHash, err := gitRepo.CommitTree(doer.NewGitSig(), committer, tree, commitTreeOpts)
 	if err != nil {
-		log.Error("%v", err)
+		log.Error("CommitTree failed: %v", err)
 		return err
 	}
 
@@ -222,11 +222,11 @@ func updateWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model
 			0,
 		),
 	}); err != nil {
-		log.Error("%v", err)
+		log.Error("Push failed: %v", err)
 		if git.IsErrPushOutOfDate(err) || git.IsErrPushRejected(err) {
 			return err
 		}
-		return fmt.Errorf("Push: %w", err)
+		return fmt.Errorf("failed to push: %w", err)
 	}
 
 	return nil

Original file line number	Diff line number	Diff line change
`@@ -114,7 +114,7 @@ func showWebStartupMessage(msg string) {`
`114`	`114`	`log.Info("* WorkPath: %s", setting.AppWorkPath)`
`115`	`115`	`log.Info("* CustomPath: %s", setting.CustomPath)`
`116`	`116`	`log.Info("* ConfigFile: %s", setting.CustomConf)`
`117`		`- log.Info("%s", msg)`
	`117`	`+ log.Info("%s", msg) // show startup message`
`118`	`118`	`}`
`119`	`119`
`120`	`120`	`func serveInstall(ctx *cli.Context) error {`
Original file line number	Diff line number	Diff line change
`@@ -53,7 +53,7 @@ func (repo *Repository) IsDependenciesEnabled(ctx context.Context) bool {`
`53`	`53`	`var u *RepoUnit`
`54`	`54`	`var err error`
`55`	`55`	`if u, err = repo.GetUnit(ctx, unit.TypeIssues); err != nil {`
`56`		`- log.Trace("%s", err)`
	`56`	`+ log.Trace("IsDependenciesEnabled: %v", err)`
`57`	`57`	`return setting.Service.DefaultEnableDependencies`
`58`	`58`	`}`
`59`	`59`	`return u.IssuesConfig().EnableDependencies`
Original file line number	Diff line number	Diff line change
`@@ -214,7 +214,7 @@ func ToPointer[T any](val T) *T {`
`214`	`214`	`}`
`215`	`215`
`216`	`216`	`// Iif is an "inline-if", it returns "trueVal" if "condition" is true, otherwise "falseVal"`
`217`		`-func Iif[T comparable](condition bool, trueVal, falseVal T) T {`
	`217`	`+func Iif[T any](condition bool, trueVal, falseVal T) T {`
`218`	`218`	`if condition {`
`219`	`219`	`return trueVal`
`220`	`220`	`}`
Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,7 @@ func verifyCommits(oldCommitID, newCommitID string, repo *git.Repository, env []`
`47`	`47`	`_ = stdoutWriter.Close()`
`48`	`48`	`err := readAndVerifyCommitsFromShaReader(stdoutReader, repo, env)`
`49`	`49`	`if err != nil {`
`50`		`- log.Error("%v", err)`
	`50`	`+ log.Error("readAndVerifyCommitsFromShaReader failed: %v", err)`
`51`	`51`	`cancel()`
`52`	`52`	`}`
`53`	`53`	`_ = stdoutReader.Close()`
`@@ -66,7 +66,6 @@ func readAndVerifyCommitsFromShaReader(input io.ReadCloser, repo *git.Repository`
`66`	`66`	`line := scanner.Text()`
`67`	`67`	`err := readAndVerifyCommit(line, repo, env)`
`68`	`68`	`if err != nil {`
`69`		`- log.Error("%v", err)`
`70`	`69`	`return err`
`71`	`70`	`}`
`72`	`71`	`}`