From b27fe05af02b157e58b7e47f08941826835bedcc Mon Sep 17 00:00:00 2001 From: delvh Date: Mon, 17 Oct 2022 15:38:10 +0200 Subject: [PATCH 1/2] Ignore error when retrieving changed PR review files Fixes #21392 --- services/gitdiff/gitdiff.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index e7362cdcdddcd..f74a14bb3c333 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -1235,8 +1235,14 @@ func SyncAndGetUserSpecificDiff(ctx context.Context, userID int64, pull *issues_ } changedFiles, err := gitRepo.GetFilesChangedBetween(review.CommitSHA, latestCommit) + // There are way too many possible errors. + // Examples are various git errors such as the commit the review was based on was gc'ed and hence doesn't exist anymore as well as unrecoverable errors where we should serve a 500 response + // Due to the current architecture and physical limitation of needing to compare explicit error messages, we can only choose one approach without the code getting ugly + // For SOME of the errors such as the gc'ed commit, it would be best to mark all files as changed + // But as that does not work for all potential errors, we simply mark all files as unchanged and drop the error which always works, even if not as good as possible if err != nil { - return diff, err + changedFiles = []string{} + log.Error("Could not get changed files between %s and %s for pull request %d in repo with path %s. Assuming no changes. Error: %w", review.CommitSHA, latestCommit, pull.Index, gitRepo.Path, err) } filesChangedSinceLastDiff := make(map[string]pull_model.ViewedState) From 1b13090fef31b0c15982a651f322528472bbdd16 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 19 Oct 2022 22:51:35 +0800 Subject: [PATCH 2/2] Update services/gitdiff/gitdiff.go Co-authored-by: delvh --- services/gitdiff/gitdiff.go | 1 - 1 file changed, 1 deletion(-) diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index f74a14bb3c333..e687d9ae9163b 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -1241,7 +1241,6 @@ func SyncAndGetUserSpecificDiff(ctx context.Context, userID int64, pull *issues_ // For SOME of the errors such as the gc'ed commit, it would be best to mark all files as changed // But as that does not work for all potential errors, we simply mark all files as unchanged and drop the error which always works, even if not as good as possible if err != nil { - changedFiles = []string{} log.Error("Could not get changed files between %s and %s for pull request %d in repo with path %s. Assuming no changes. Error: %w", review.CommitSHA, latestCommit, pull.Index, gitRepo.Path, err) }