From 4c123d8e3b160b8d53547a704f2b0361bd518ce0 Mon Sep 17 00:00:00 2001 From: Gusted Date: Wed, 5 Mar 2025 18:40:13 +0100 Subject: [PATCH] feat: improve error handling of commit rendering - Simplify if-else expression to `NotFoundOrServerError`. - I cannot find an existing scenario where `Getdiff` returns an error and where it therefore should show a 404 error in the context of rendering a diff of a commit. So simply return it as an Internal Server Error, this also helps with debugging if an actual error occurs here (404 errors are only logged at the DEBUG level). - The first change is already covered under existing testing, the second change is not trivial to test. --- routers/web/repo/commit.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index c2436a1c59..857e34381e 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -315,11 +315,7 @@ func Diff(ctx *context.Context) { commit, err := gitRepo.GetCommit(commitID) if err != nil { - if git.IsErrNotExist(err) { - ctx.NotFound("Repo.GitRepo.GetCommit", err) - } else { - ctx.ServerError("Repo.GitRepo.GetCommit", err) - } + ctx.NotFoundOrServerError("gitRepo.GetCommit", git.IsErrNotExist, err) return } if len(commitID) != commit.ID.Type().FullLength() { @@ -343,7 +339,7 @@ func Diff(ctx *context.Context) { FileOnly: fileOnly, }, files...) if err != nil { - ctx.NotFound("GetDiff", err) + ctx.ServerError("GetDiff", err) return }