Skip to content

Commit 6a03309

Browse files
delvhlunnywxiaoguang
authored
Ignore error when retrieving changed PR review files (#21487)
When a PR reviewer reviewed a file on a commit that was later gc'ed, they would always get a `500` response from then on when loading the PR. This PR simply ignores that error and instead marks all files as unchanged. This approach was chosen as the only feasible option without diving into **a lot** of error handling. Fixes #21392 Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
1 parent bd272e4 commit 6a03309

File tree

1 file changed

+6
-1
lines changed

1 file changed

+6
-1
lines changed

services/gitdiff/gitdiff.go

+6-1
Original file line numberDiff line numberDiff line change
@@ -1235,8 +1235,13 @@ func SyncAndGetUserSpecificDiff(ctx context.Context, userID int64, pull *issues_
12351235
}
12361236

12371237
changedFiles, err := gitRepo.GetFilesChangedBetween(review.CommitSHA, latestCommit)
1238+
// There are way too many possible errors.
1239+
// 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
1240+
// 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
1241+
// For SOME of the errors such as the gc'ed commit, it would be best to mark all files as changed
1242+
// 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
12381243
if err != nil {
1239-
return diff, err
1244+
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)
12401245
}
12411246

12421247
filesChangedSinceLastDiff := make(map[string]pull_model.ViewedState)

0 commit comments

Comments
 (0)