Improve package API log handling (#35100)

wxiaoguang · web-flow · commit f0da1de7e30e · 2025-07-16T12:25:49.000-04:00
Simplify code and fix log processing logic
diff --git a/routers/api/packages/alpine/alpine.go b/routers/api/packages/alpine/alpine.go
@@ -25,9 +25,8 @@ import (
 )
 
 func apiError(ctx *context.Context, status int, obj any) {
-	helper.LogAndProcessError(ctx, status, obj, func(message string) {
-		ctx.PlainText(status, message)
-	})
+	message := helper.ProcessErrorForUser(ctx, status, obj)
+	ctx.PlainText(status, message)
 }
 
 func GetRepositoryKey(ctx *context.Context) {
diff --git a/routers/api/packages/arch/arch.go b/routers/api/packages/arch/arch.go
@@ -24,9 +24,8 @@ import (
 )
 
 func apiError(ctx *context.Context, status int, obj any) {
-	helper.LogAndProcessError(ctx, status, obj, func(message string) {
-		ctx.PlainText(status, message)
-	})
+	message := helper.ProcessErrorForUser(ctx, status, obj)
+	ctx.PlainText(status, message)
 }
 
 func GetRepositoryKey(ctx *context.Context) {
diff --git a/routers/api/packages/cargo/cargo.go b/routers/api/packages/cargo/cargo.go
@@ -37,15 +37,14 @@ type StatusMessage struct {
 }
 
 func apiError(ctx *context.Context, status int, obj any) {
-	helper.LogAndProcessError(ctx, status, obj, func(message string) {
-		ctx.JSON(status, StatusResponse{
-			OK: false,
-			Errors: []StatusMessage{
-				{
-					Message: message,
-				},
+	message := helper.ProcessErrorForUser(ctx, status, obj)
+	ctx.JSON(status, StatusResponse{
+		OK: false,
+		Errors: []StatusMessage{
+			{
+				Message: message,
 			},
-		})
+		},
 	})
 }
 
diff --git a/routers/api/packages/chef/chef.go b/routers/api/packages/chef/chef.go
@@ -30,10 +30,9 @@ func apiError(ctx *context.Context, status int, obj any) {
 		ErrorMessages []string `json:"error_messages"`
 	}
 
-	helper.LogAndProcessError(ctx, status, obj, func(message string) {
-		ctx.JSON(status, Error{
-			ErrorMessages: []string{message},
-		})
+	message := helper.ProcessErrorForUser(ctx, status, obj)
+	ctx.JSON(status, Error{
+		ErrorMessages: []string{message},
 	})
 }
 
diff --git a/routers/api/packages/composer/composer.go b/routers/api/packages/composer/composer.go
@@ -28,18 +28,17 @@ import (
 )
 
 func apiError(ctx *context.Context, status int, obj any) {
-	helper.LogAndProcessError(ctx, status, obj, func(message string) {
-		type Error struct {
-			Status  int    `json:"status"`
-			Message string `json:"message"`
-		}
-		ctx.JSON(status, struct {
-			Errors []Error `json:"errors"`
-		}{
-			Errors: []Error{
-				{Status: status, Message: message},
-			},
-		})
+	message := helper.ProcessErrorForUser(ctx, status, obj)
+	type Error struct {
+		Status  int    `json:"status"`
+		Message string `json:"message"`
+	}
+	ctx.JSON(status, struct {
+		Errors []Error `json:"errors"`
+	}{
+		Errors: []Error{
+			{Status: status, Message: message},
+		},
 	})
 }
 
diff --git a/routers/api/packages/conan/conan.go b/routers/api/packages/conan/conan.go
@@ -61,10 +61,9 @@ func jsonResponse(ctx *context.Context, status int, obj any) {
 }
 
 func apiError(ctx *context.Context, status int, obj any) {
-	helper.LogAndProcessError(ctx, status, obj, func(message string) {
-		jsonResponse(ctx, status, map[string]string{
-			"message": message,
-		})
+	message := helper.ProcessErrorForUser(ctx, status, obj)
+	jsonResponse(ctx, status, map[string]string{
+		"message": message,
 	})
 }
 
diff --git a/routers/api/packages/conda/conda.go b/routers/api/packages/conda/conda.go
@@ -25,14 +25,13 @@ import (
 )
 
 func apiError(ctx *context.Context, status int, obj any) {
-	helper.LogAndProcessError(ctx, status, obj, func(message string) {
-		ctx.JSON(status, struct {
-			Reason  string `json:"reason"`
-			Message string `json:"message"`
-		}{
-			Reason:  http.StatusText(status),
-			Message: message,
-		})
+	message := helper.ProcessErrorForUser(ctx, status, obj)
+	ctx.JSON(status, struct {
+		Reason  string `json:"reason"`
+		Message string `json:"message"`
+	}{
+		Reason:  http.StatusText(status),
+		Message: message,
 	})
 }
 
diff --git a/routers/api/packages/container/container.go b/routers/api/packages/container/container.go
@@ -93,10 +93,9 @@ func jsonResponse(ctx *context.Context, status int, obj any) {
 }
 
 func apiError(ctx *context.Context, status int, err error) {
-	helper.LogAndProcessError(ctx, status, err, func(message string) {
-		setResponseHeaders(ctx.Resp, &containerHeaders{
-			Status: status,
-		})
+	_ = helper.ProcessErrorForUser(ctx, status, err)
+	setResponseHeaders(ctx.Resp, &containerHeaders{
+		Status: status,
 	})
 }
 
diff --git a/routers/api/packages/cran/cran.go b/routers/api/packages/cran/cran.go
@@ -22,9 +22,8 @@ import (
 )
 
 func apiError(ctx *context.Context, status int, obj any) {
-	helper.LogAndProcessError(ctx, status, obj, func(message string) {
-		ctx.PlainText(status, message)
-	})
+	message := helper.ProcessErrorForUser(ctx, status, obj)
+	ctx.PlainText(status, message)
 }
 
 func EnumerateSourcePackages(ctx *context.Context) {
diff --git a/routers/api/packages/debian/debian.go b/routers/api/packages/debian/debian.go
@@ -24,9 +24,8 @@ import (
 )
 
 func apiError(ctx *context.Context, status int, obj any) {
-	helper.LogAndProcessError(ctx, status, obj, func(message string) {
-		ctx.PlainText(status, message)
-	})
+	message := helper.ProcessErrorForUser(ctx, status, obj)
+	ctx.PlainText(status, message)
 }
 
 func GetRepositoryKey(ctx *context.Context) {
diff --git a/routers/api/packages/generic/generic.go b/routers/api/packages/generic/generic.go
@@ -24,9 +24,8 @@ var (
 )
 
 func apiError(ctx *context.Context, status int, obj any) {
-	helper.LogAndProcessError(ctx, status, obj, func(message string) {
-		ctx.PlainText(status, message)
-	})
+	message := helper.ProcessErrorForUser(ctx, status, obj)
+	ctx.PlainText(status, message)
 }
 
 // DownloadPackageFile serves the specific generic package.
diff --git a/routers/api/packages/goproxy/goproxy.go b/routers/api/packages/goproxy/goproxy.go
@@ -22,9 +22,8 @@ import (
 )
 
 func apiError(ctx *context.Context, status int, obj any) {
-	helper.LogAndProcessError(ctx, status, obj, func(message string) {
-		ctx.PlainText(status, message)
-	})
+	message := helper.ProcessErrorForUser(ctx, status, obj)
+	ctx.PlainText(status, message)
 }
 
 func EnumeratePackageVersions(ctx *context.Context) {
diff --git a/routers/api/packages/helm/helm.go b/routers/api/packages/helm/helm.go
@@ -28,13 +28,12 @@ import (
 )
 
 func apiError(ctx *context.Context, status int, obj any) {
-	helper.LogAndProcessError(ctx, status, obj, func(message string) {
-		type Error struct {
-			Error string `json:"error"`
-		}
-		ctx.JSON(status, Error{
-			Error: message,
-		})
+	message := helper.ProcessErrorForUser(ctx, status, obj)
+	type Error struct {
+		Error string `json:"error"`
+	}
+	ctx.JSON(status, Error{
+		Error: message,
 	})
 }
 
diff --git a/routers/api/packages/helper/helper.go b/routers/api/packages/helper/helper.go
@@ -15,31 +15,29 @@ import (
 	"code.gitea.io/gitea/services/context"
 )
 
-// LogAndProcessError logs an error and calls a custom callback with the processed error message.
-// If the error is an InternalServerError the message is stripped if the user is not an admin.
-func LogAndProcessError(ctx *context.Context, status int, obj any, cb func(string)) {
+// ProcessErrorForUser logs the error and returns a user-error message for the end user.
+// If the status is http.StatusInternalServerError, the message is stripped for non-admin users in production.
+func ProcessErrorForUser(ctx *context.Context, status int, errObj any) string {
 	var message string
-	if err, ok := obj.(error); ok {
+	if err, ok := errObj.(error); ok {
 		message = err.Error()
-	} else if obj != nil {
-		message = fmt.Sprintf("%s", obj)
+	} else if errObj != nil {
+		message = fmt.Sprint(errObj)
 	}
-	if status == http.StatusInternalServerError {
-		log.ErrorWithSkip(1, message)
 
+	if status == http.StatusInternalServerError {
+		log.Log(2, log.ERROR, "Package registry API internal error: %d %s", status, message)
 		if setting.IsProd && (ctx.Doer == nil || !ctx.Doer.IsAdmin) {
-			message = ""
+			message = "internal server error"
 		}
-	} else {
-		log.Debug(message)
+		return message
 	}
 
-	if cb != nil {
-		cb(message)
-	}
+	log.Log(2, log.DEBUG, "Package registry API user error: %d %s", status, message)
+	return message
 }
 
-// Serves the content of the package file
+// ServePackageFile the content of the package file
 // If the url is set it will redirect the request, otherwise the content is copied to the response.
 func ServePackageFile(ctx *context.Context, s io.ReadSeekCloser, u *url.URL, pf *packages_model.PackageFile, forceOpts ...*context.ServeHeaderOptions) {
 	if u != nil {
diff --git a/routers/api/packages/maven/maven.go b/routers/api/packages/maven/maven.go
@@ -22,7 +22,6 @@ import (
 	packages_model "code.gitea.io/gitea/models/packages"
 	"code.gitea.io/gitea/modules/globallock"
 	"code.gitea.io/gitea/modules/json"
-	"code.gitea.io/gitea/modules/log"
 	packages_module "code.gitea.io/gitea/modules/packages"
 	maven_module "code.gitea.io/gitea/modules/packages/maven"
 	"code.gitea.io/gitea/modules/util"
@@ -49,14 +48,9 @@ var (
 )
 
 func apiError(ctx *context.Context, status int, obj any) {
-	helper.LogAndProcessError(ctx, status, obj, func(message string) {
-		// The maven client does not present the error message to the user. Log it for users with access to server logs.
-		if status == http.StatusBadRequest || status == http.StatusInternalServerError {
-			log.Error(message)
-		}
-
-		ctx.PlainText(status, message)
-	})
+	message := helper.ProcessErrorForUser(ctx, status, obj)
+	// Maven client doesn't present the error message to end users; site admin can check the server logs that outputted by ProcessErrorForUser
+	ctx.PlainText(status, message)
 }
 
 // DownloadPackageFile serves the content of a package
diff --git a/routers/api/packages/npm/npm.go b/routers/api/packages/npm/npm.go
@@ -33,10 +33,9 @@ import (
 var errInvalidTagName = errors.New("The tag name is invalid")
 
 func apiError(ctx *context.Context, status int, obj any) {
-	helper.LogAndProcessError(ctx, status, obj, func(message string) {
-		ctx.JSON(status, map[string]string{
-			"error": message,
-		})
+	message := helper.ProcessErrorForUser(ctx, status, obj)
+	ctx.JSON(status, map[string]string{
+		"error": message,
 	})
 }
 
diff --git a/routers/api/packages/nuget/nuget.go b/routers/api/packages/nuget/nuget.go
@@ -29,10 +29,9 @@ import (
 )
 
 func apiError(ctx *context.Context, status int, obj any) {
-	helper.LogAndProcessError(ctx, status, obj, func(message string) {
-		ctx.JSON(status, map[string]string{
-			"Message": message,
-		})
+	message := helper.ProcessErrorForUser(ctx, status, obj)
+	ctx.JSON(status, map[string]string{
+		"Message": message,
 	})
 }
 
diff --git a/routers/api/packages/pub/pub.go b/routers/api/packages/pub/pub.go
@@ -43,13 +43,12 @@ func apiError(ctx *context.Context, status int, obj any) {
 		Error Error `json:"error"`
 	}
 
-	helper.LogAndProcessError(ctx, status, obj, func(message string) {
-		jsonResponse(ctx, status, ErrorWrapper{
-			Error: Error{
-				Code:    http.StatusText(status),
-				Message: message,
-			},
-		})
+	message := helper.ProcessErrorForUser(ctx, status, obj)
+	jsonResponse(ctx, status, ErrorWrapper{
+		Error: Error{
+			Code:    http.StatusText(status),
+			Message: message,
+		},
 	})
 }
 
diff --git a/routers/api/packages/pypi/pypi.go b/routers/api/packages/pypi/pypi.go
@@ -40,9 +40,8 @@ var versionMatcher = regexp.MustCompile(`\Av?` +
 	`\z`)
 
 func apiError(ctx *context.Context, status int, obj any) {
-	helper.LogAndProcessError(ctx, status, obj, func(message string) {
-		ctx.PlainText(status, message)
-	})
+	message := helper.ProcessErrorForUser(ctx, status, obj)
+	ctx.PlainText(status, message)
 }
 
 // PackageMetadata returns the metadata for a single package
diff --git a/routers/api/packages/rpm/rpm.go b/routers/api/packages/rpm/rpm.go
@@ -26,9 +26,8 @@ import (
 )
 
 func apiError(ctx *context.Context, status int, obj any) {
-	helper.LogAndProcessError(ctx, status, obj, func(message string) {
-		ctx.PlainText(status, message)
-	})
+	message := helper.ProcessErrorForUser(ctx, status, obj)
+	ctx.PlainText(status, message)
 }
 
 // https://dnf.readthedocs.io/en/latest/conf_ref.html
diff --git a/routers/api/packages/rubygems/rubygems.go b/routers/api/packages/rubygems/rubygems.go
@@ -25,9 +25,8 @@ import (
 )
 
 func apiError(ctx *context.Context, status int, obj any) {
-	helper.LogAndProcessError(ctx, status, obj, func(message string) {
-		ctx.PlainText(status, message)
-	})
+	message := helper.ProcessErrorForUser(ctx, status, obj)
+	ctx.PlainText(status, message)
 }
 
 // EnumeratePackages serves the package list
diff --git a/routers/api/packages/swift/swift.go b/routers/api/packages/swift/swift.go
@@ -77,17 +77,14 @@ func apiError(ctx *context.Context, status int, obj any) {
 		Detail string `json:"detail"`
 	}
 
-	helper.LogAndProcessError(ctx, status, obj, func(message string) {
-		setResponseHeaders(ctx.Resp, &headers{
-			Status:      status,
-			ContentType: "application/problem+json",
-		})
-		if err := json.NewEncoder(ctx.Resp).Encode(Problem{
-			Status: status,
-			Detail: message,
-		}); err != nil {
-			log.Error("JSON encode: %v", err)
-		}
+	message := helper.ProcessErrorForUser(ctx, status, obj)
+	setResponseHeaders(ctx.Resp, &headers{
+		Status:      status,
+		ContentType: "application/problem+json",
+	})
+	_ = json.NewEncoder(ctx.Resp).Encode(Problem{
+		Status: status,
+		Detail: message,
 	})
 }
 
diff --git a/routers/api/packages/vagrant/vagrant.go b/routers/api/packages/vagrant/vagrant.go

Original file line number	Diff line number	Diff line change
`@@ -25,9 +25,8 @@ import (`
`25`	`25`	`)`
`26`	`26`
`27`	`27`	`func apiError(ctx *context.Context, status int, obj any) {`
`28`		`- helper.LogAndProcessError(ctx, status, obj, func(message string) {`
`29`		`- ctx.PlainText(status, message)`
`30`		`- })`
	`28`	`+ message := helper.ProcessErrorForUser(ctx, status, obj)`
	`29`	`+ ctx.PlainText(status, message)`
`31`	`30`	`}`
`32`	`31`
`33`	`32`	`func GetRepositoryKey(ctx *context.Context) {`
Original file line number	Diff line number	Diff line change
`@@ -24,9 +24,8 @@ import (`
`24`	`24`	`)`
`25`	`25`
`26`	`26`	`func apiError(ctx *context.Context, status int, obj any) {`
`27`		`- helper.LogAndProcessError(ctx, status, obj, func(message string) {`
`28`		`- ctx.PlainText(status, message)`
`29`		`- })`
	`27`	`+ message := helper.ProcessErrorForUser(ctx, status, obj)`
	`28`	`+ ctx.PlainText(status, message)`
`30`	`29`	`}`
`31`	`30`
`32`	`31`	`func GetRepositoryKey(ctx *context.Context) {`
Original file line number	Diff line number	Diff line change
`@@ -30,10 +30,9 @@ func apiError(ctx *context.Context, status int, obj any) {`
`30`	`30`	ErrorMessages []string `json:"error_messages"`
`31`	`31`	`}`
`32`	`32`
`33`		`- helper.LogAndProcessError(ctx, status, obj, func(message string) {`
`34`		`- ctx.JSON(status, Error{`
`35`		`- ErrorMessages: []string{message},`
`36`		`- })`
	`33`	`+ message := helper.ProcessErrorForUser(ctx, status, obj)`
	`34`	`+ ctx.JSON(status, Error{`
	`35`	`+ ErrorMessages: []string{message},`
`37`	`36`	`})`
`38`	`37`	`}`
`39`	`38`
Original file line number	Diff line number	Diff line change
`@@ -61,10 +61,9 @@ func jsonResponse(ctx *context.Context, status int, obj any) {`
`61`	`61`	`}`
`62`	`62`
`63`	`63`	`func apiError(ctx *context.Context, status int, obj any) {`
`64`		`- helper.LogAndProcessError(ctx, status, obj, func(message string) {`
`65`		`- jsonResponse(ctx, status, map[string]string{`
`66`		`- "message": message,`
`67`		`- })`
	`64`	`+ message := helper.ProcessErrorForUser(ctx, status, obj)`
	`65`	`+ jsonResponse(ctx, status, map[string]string{`
	`66`	`+ "message": message,`
`68`	`67`	`})`
`69`	`68`	`}`
`70`	`69`
Original file line number	Diff line number	Diff line change
`@@ -93,10 +93,9 @@ func jsonResponse(ctx *context.Context, status int, obj any) {`
`93`	`93`	`}`
`94`	`94`
`95`	`95`	`func apiError(ctx *context.Context, status int, err error) {`
`96`		`- helper.LogAndProcessError(ctx, status, err, func(message string) {`
`97`		`- setResponseHeaders(ctx.Resp, &containerHeaders{`
`98`		`- Status: status,`
`99`		`- })`
	`96`	`+ _ = helper.ProcessErrorForUser(ctx, status, err)`
	`97`	`+ setResponseHeaders(ctx.Resp, &containerHeaders{`
	`98`	`+ Status: status,`
`100`	`99`	`})`
`101`	`100`	`}`
`102`	`101`
Original file line number	Diff line number	Diff line change
`@@ -22,9 +22,8 @@ import (`
`22`	`22`	`)`
`23`	`23`
`24`	`24`	`func apiError(ctx *context.Context, status int, obj any) {`
`25`		`- helper.LogAndProcessError(ctx, status, obj, func(message string) {`
`26`		`- ctx.PlainText(status, message)`
`27`		`- })`
	`25`	`+ message := helper.ProcessErrorForUser(ctx, status, obj)`
	`26`	`+ ctx.PlainText(status, message)`
`28`	`27`	`}`
`29`	`28`
`30`	`29`	`func EnumerateSourcePackages(ctx *context.Context) {`