Skip to content

Ignore error when retrieving changed PR review files (#21487) #21524

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 20, 2022
Merged

Ignore error when retrieving changed PR review files (#21487) #21524

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion services/gitdiff/gitdiff.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1505,8 +1505,13 @@ 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
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)
Expand Down