Fix error logs and improve some comments/messages (#35105)

14 files changed, 19 insertions, 41 deletions

diff --git a/modules/storage/storage.go b/modules/storage/storage.go
index 2313951d8a..1868817c05 100644
--- a/modules/storage/storage.go
+++ b/modules/storage/storage.go
@@ -59,8 +59,12 @@ type Object interface {
 // ObjectStorage represents an object storage to handle a bucket and files
 type ObjectStorage interface {
 	Open(path string) (Object, error)
-	// Save store a object, if size is unknown set -1
+
+	// Save store an object, if size is unknown set -1
+	// NOTICE: Some storage SDK will close the Reader after saving if it is also a Closer,
+	// DO NOT use the reader anymore after Save, or wrap it to a non-Closer reader.
 	Save(path string, r io.Reader, size int64) (int64, error)
+
 	Stat(path string) (os.FileInfo, error)
 	Delete(path string) error
 	URL(path, name, method string, reqParams url.Values) (*url.URL, error)
diff --git a/routers/api/packages/api.go b/routers/api/packages/api.go
index d914856706..f6ee5958b5 100644
--- a/routers/api/packages/api.go
+++ b/routers/api/packages/api.go
@@ -99,7 +99,7 @@ func verifyAuth(r *web.Router, authMethods []auth.Method) {
 		ctx.Doer, err = authGroup.Verify(ctx.Req, ctx.Resp, ctx, ctx.Session)
 		if err != nil {
 			log.Error("Failed to verify user: %v", err)
-			ctx.HTTPError(http.StatusUnauthorized, "authGroup.Verify")
+			ctx.HTTPError(http.StatusUnauthorized, "Failed to authenticate user")
 			return
 		}
 		ctx.IsSigned = ctx.Doer != nil
diff --git a/routers/api/packages/conan/auth.go b/routers/api/packages/conan/auth.go
index 9c03d01391..bce3235a2e 100644
--- a/routers/api/packages/conan/auth.go
+++ b/routers/api/packages/conan/auth.go
@@ -34,7 +34,6 @@ func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataS
 
 	u, err := user_model.GetUserByID(req.Context(), packageMeta.UserID)
 	if err != nil {
-		log.Error("GetUserByID:  %v", err)
 		return nil, err
 	}
 	if packageMeta.Scope != "" {
diff --git a/routers/api/packages/conan/conan.go b/routers/api/packages/conan/conan.go
index 15f5276487..126b1593cd 100644
--- a/routers/api/packages/conan/conan.go
+++ b/routers/api/packages/conan/conan.go
@@ -55,9 +55,7 @@ func jsonResponse(ctx *context.Context, status int, obj any) {
 	// https://github.com/conan-io/conan/issues/6613
 	ctx.Resp.Header().Set("Content-Type", "application/json")
 	ctx.Status(status)
-	if err := json.NewEncoder(ctx.Resp).Encode(obj); err != nil {
-		log.Error("JSON encode: %v", err)
-	}
+	_ = json.NewEncoder(ctx.Resp).Encode(obj)
 }
 
 func apiError(ctx *context.Context, status int, obj any) {
@@ -392,7 +390,6 @@ func uploadFile(ctx *context.Context, fileFilter container.Set[string], fileKey
 		if isConanfileFile {
 			metadata, err := conan_module.ParseConanfile(buf)
 			if err != nil {
-				log.Error("Error parsing package metadata: %v", err)
 				apiError(ctx, http.StatusInternalServerError, err)
 				return
 			}
@@ -418,7 +415,6 @@ func uploadFile(ctx *context.Context, fileFilter container.Set[string], fileKey
 		} else {
 			info, err := conan_module.ParseConaninfo(buf)
 			if err != nil {
-				log.Error("Error parsing conan info: %v", err)
 				apiError(ctx, http.StatusInternalServerError, err)
 				return
 			}
diff --git a/routers/api/packages/conda/conda.go b/routers/api/packages/conda/conda.go
index 9b41745feb..f496002bb5 100644
--- a/routers/api/packages/conda/conda.go
+++ b/routers/api/packages/conda/conda.go
@@ -13,7 +13,6 @@ import (
 	packages_model "code.gitea.io/gitea/models/packages"
 	conda_model "code.gitea.io/gitea/models/packages/conda"
 	"code.gitea.io/gitea/modules/json"
-	"code.gitea.io/gitea/modules/log"
 	packages_module "code.gitea.io/gitea/modules/packages"
 	conda_module "code.gitea.io/gitea/modules/packages/conda"
 	"code.gitea.io/gitea/modules/util"
@@ -184,10 +183,7 @@ func EnumeratePackages(ctx *context.Context) {
 	}
 
 	resp.WriteHeader(http.StatusOK)
-
-	if err := json.NewEncoder(w).Encode(repoData); err != nil {
-		log.Error("JSON encode: %v", err)
-	}
+	_ = json.NewEncoder(w).Encode(repoData)
 }
 
 func UploadPackageFile(ctx *context.Context) {
diff --git a/routers/api/packages/container/auth.go b/routers/api/packages/container/auth.go
index 1e1b87eb79..19a931c405 100644
--- a/routers/api/packages/container/auth.go
+++ b/routers/api/packages/container/auth.go
@@ -35,7 +35,6 @@ func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataS
 
 	u, err := user_model.GetPossibleUserByID(req.Context(), packageMeta.UserID)
 	if err != nil {
-		log.Error("GetPossibleUserByID:  %v", err)
 		return nil, err
 	}
 
diff --git a/routers/api/packages/container/container.go b/routers/api/packages/container/container.go
index b0c605f828..db81dd13c2 100644
--- a/routers/api/packages/container/container.go
+++ b/routers/api/packages/container/container.go
@@ -448,9 +448,9 @@ func PutBlobsUpload(ctx *context.Context) {
 		return
 	}
 
-	// There was a strange bug: the "Close" fails with error "close .../tmp/package-upload/....: file already closed"
-	// AFAIK there should be no other "Close" call to the uploader between NewBlobUploader and this line.
-	// At least it's safe to call Close twice, so ignore the error.
+	// Some SDK (e.g.: minio) will close the Reader if it is also a Closer after "uploading".
+	// And we don't need to wrap the reader to anything else because the SDK will benefit from other interfaces like Seeker.
+	// It's safe to call Close twice, so ignore the error.
 	_ = uploader.Close()
 
 	if err := container_service.RemoveBlobUploadByID(ctx, uploader.ID); err != nil {
@@ -733,9 +733,7 @@ func serveBlob(ctx *context.Context, pfd *packages_model.PackageFileDescriptor)
 	defer s.Close()
 
 	setResponseHeaders(ctx.Resp, headers)
-	if _, err := io.Copy(ctx.Resp, s); err != nil {
-		log.Error("Error whilst copying content to response: %v", err)
-	}
+	_, _ = io.Copy(ctx.Resp, s)
 }
 
 // https://github.com/opencontainers/distribution-spec/blob/main/spec.md#content-discovery
diff --git a/routers/api/packages/generic/generic.go b/routers/api/packages/generic/generic.go
index efc58206f4..5eb189e6d9 100644
--- a/routers/api/packages/generic/generic.go
+++ b/routers/api/packages/generic/generic.go
@@ -11,7 +11,6 @@ import (
 	"unicode"
 
 	packages_model "code.gitea.io/gitea/models/packages"
-	"code.gitea.io/gitea/modules/log"
 	packages_module "code.gitea.io/gitea/modules/packages"
 	"code.gitea.io/gitea/routers/api/packages/helper"
 	"code.gitea.io/gitea/services/context"
@@ -101,7 +100,6 @@ func UploadPackage(ctx *context.Context) {
 
 	buf, err := packages_module.CreateHashedBufferFromReader(upload)
 	if err != nil {
-		log.Error("Error creating hashed buffer: %v", err)
 		apiError(ctx, http.StatusInternalServerError, err)
 		return
 	}
diff --git a/routers/api/packages/helm/helm.go b/routers/api/packages/helm/helm.go
index e509f6cd09..4c1b72d5c0 100644
--- a/routers/api/packages/helm/helm.go
+++ b/routers/api/packages/helm/helm.go
@@ -14,7 +14,6 @@ import (
 
 	packages_model "code.gitea.io/gitea/models/packages"
 	"code.gitea.io/gitea/modules/json"
-	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/optional"
 	packages_module "code.gitea.io/gitea/modules/packages"
 	helm_module "code.gitea.io/gitea/modules/packages/helm"
@@ -86,16 +85,14 @@ func Index(ctx *context.Context) {
 	}
 
 	ctx.Resp.WriteHeader(http.StatusOK)
-	if err := yaml.NewEncoder(ctx.Resp).Encode(&Index{
+	_ = yaml.NewEncoder(ctx.Resp).Encode(&Index{
 		APIVersion: "v1",
 		Entries:    entries,
 		Generated:  time.Now(),
 		ServerInfo: &ServerInfo{
 			ContextPath: setting.AppSubURL + "/api/packages/" + url.PathEscape(ctx.Package.Owner.Name) + "/helm",
 		},
-	}); err != nil {
-		log.Error("YAML encode failed: %v", err)
-	}
+	})
 }
 
 // DownloadPackageFile serves the content of a package
diff --git a/routers/api/packages/nuget/auth.go b/routers/api/packages/nuget/auth.go
index e81ad01b2b..ce7df0ce0a 100644
--- a/routers/api/packages/nuget/auth.go
+++ b/routers/api/packages/nuget/auth.go
@@ -26,7 +26,6 @@ func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataS
 	token, err := auth_model.GetAccessTokenBySHA(req.Context(), req.Header.Get("X-NuGet-ApiKey"))
 	if err != nil {
 		if !(auth_model.IsErrAccessTokenNotExist(err) || auth_model.IsErrAccessTokenEmpty(err)) {
-			log.Error("GetAccessTokenBySHA: %v", err)
 			return nil, err
 		}
 		return nil, nil
@@ -34,7 +33,6 @@ func (a *Auth) Verify(req *http.Request, w http.ResponseWriter, store auth.DataS
 
 	u, err := user_model.GetUserByID(req.Context(), token.UID)
 	if err != nil {
-		log.Error("GetUserByID:  %v", err)
 		return nil, err
 	}
 
diff --git a/routers/api/packages/nuget/nuget.go b/routers/api/packages/nuget/nuget.go
index 4e096305a6..c42fdd7db5 100644
--- a/routers/api/packages/nuget/nuget.go
+++ b/routers/api/packages/nuget/nuget.go
@@ -17,7 +17,6 @@ import (
 	"code.gitea.io/gitea/models/db"
 	packages_model "code.gitea.io/gitea/models/packages"
 	nuget_model "code.gitea.io/gitea/models/packages/nuget"
-	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/optional"
 	packages_module "code.gitea.io/gitea/modules/packages"
 	nuget_module "code.gitea.io/gitea/modules/packages/nuget"
@@ -38,12 +37,8 @@ func apiError(ctx *context.Context, status int, obj any) {
 func xmlResponse(ctx *context.Context, status int, obj any) { //nolint:unparam // status is always StatusOK
 	ctx.Resp.Header().Set("Content-Type", "application/atom+xml; charset=utf-8")
 	ctx.Resp.WriteHeader(status)
-	if _, err := ctx.Resp.Write([]byte(xml.Header)); err != nil {
-		log.Error("Write failed: %v", err)
-	}
-	if err := xml.NewEncoder(ctx.Resp).Encode(obj); err != nil {
-		log.Error("XML encode failed: %v", err)
-	}
+	_, _ = ctx.Resp.Write([]byte(xml.Header))
+	_ = xml.NewEncoder(ctx.Resp).Encode(obj)
 }
 
 // https://github.com/NuGet/NuGet.Client/blob/dev/src/NuGet.Core/NuGet.Protocol/LegacyFeed/V2FeedQueryBuilder.cs
diff --git a/routers/api/packages/pub/pub.go b/routers/api/packages/pub/pub.go
index 661960a40c..7564e14d0e 100644
--- a/routers/api/packages/pub/pub.go
+++ b/routers/api/packages/pub/pub.go
@@ -15,7 +15,6 @@ import (
 
 	packages_model "code.gitea.io/gitea/models/packages"
 	"code.gitea.io/gitea/modules/json"
-	"code.gitea.io/gitea/modules/log"
 	packages_module "code.gitea.io/gitea/modules/packages"
 	pub_module "code.gitea.io/gitea/modules/packages/pub"
 	"code.gitea.io/gitea/modules/setting"
@@ -29,9 +28,7 @@ func jsonResponse(ctx *context.Context, status int, obj any) {
 	resp := ctx.Resp
 	resp.Header().Set("Content-Type", "application/vnd.pub.v2+json")
 	resp.WriteHeader(status)
-	if err := json.NewEncoder(resp).Encode(obj); err != nil {
-		log.Error("JSON encode: %v", err)
-	}
+	_ = json.NewEncoder(resp).Encode(obj)
 }
 
 func apiError(ctx *context.Context, status int, obj any) {
diff --git a/services/auth/interface.go b/services/auth/interface.go
index e7eccecea0..c4bed2b640 100644
--- a/services/auth/interface.go
+++ b/services/auth/interface.go
@@ -20,7 +20,7 @@ type SessionStore session.Store
 // Method represents an authentication method (plugin) for HTTP requests.
 type Method interface {
 	// Verify tries to verify the authentication data contained in the request.
-	// If verification is successful returns either an existing user object (with id > 0)
+	// If verification succeeds, it returns either an existing user object (with id > 0)
 	// or a new user object (with id = 0) populated with the information that was found
 	// in the authentication data (username or email).
 	// Second argument returns err if verification fails, otherwise
diff --git a/services/context/base.go b/services/context/base.go
index f3f92b7eeb..28d6656fd1 100644
--- a/services/context/base.go
+++ b/services/context/base.go
@@ -83,6 +83,7 @@ func (b *Base) RespHeader() http.Header {
 }
 
 // HTTPError returned an error to web browser
+// FIXME: many calls to this HTTPError are not right: it shouldn't expose err.Error() directly, it doesn't accept more than one content
 func (b *Base) HTTPError(status int, contents ...string) {
 	v := http.StatusText(status)
 	if len(contents) > 0 {