Skip to content

Commit

Permalink
autoupdater: fix: updating branch failed because wrong ExpectedHeadSHA
Browse files Browse the repository at this point in the history
Sometimes updating a PR with the base branch failed with the error message:
  "evaluating if PR is uptodate with base branch failed: context deadline
   exceeded"
where the individual update operation tries failed with
  - "retryable error: PUT https://api.github.com/repos/[..]/update-branch:
    422 expected head sha didn’t match current head ref."

When a PR branch was not up to date with the base branch, and the GitHub Pull Request
API endpoint did not return "behind", BranchIsBehindBase() was called. The
function queries the GitHub Compare Commit endpoint to figure out if the branch
needs to be updated.
The returned HEAD commit of BranchIsBehindBase() was then used as
ExpectedHeadSHA parameter for the UpdateBranch operation.
The HEAD SHA returned by BranchIsBehindBase() was the SHA of the first commit
in the branch instead of HEAD.
This caused that UpdateBranch() was retried with a wrong ExpectedHeadSHA and
failed until the retry timeout was exceeded.

Do not return from BranchIsBehindBase() the branch HEAD SHA anymore, we would
have to handle pagination to get the HEAD SHA.
Use the HEAD SHA that was retrieved by the GitHub Pull-Request endpoint instead
in PRIsUptodate().
  • Loading branch information
fho committed Jan 24, 2022
1 parent b54ba85 commit a468dd6
Showing 1 changed file with 6 additions and 15 deletions.
21 changes: 6 additions & 15 deletions internal/githubclt/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,22 +62,13 @@ type Client struct {

// BranchIsBehindBase returns true if branch is based on the most recent commit of baseBranch.
// If it is based on older commit, false is returned.
func (clt *Client) BranchIsBehindBase(ctx context.Context, owner, repo, baseBranch, branch string) (behind bool, branchHEADSHA string, err error) {
func (clt *Client) BranchIsBehindBase(ctx context.Context, owner, repo, baseBranch, branch string) (behind bool, err error) {
cmp, _, err := clt.restClt.Repositories.CompareCommits(ctx, owner, repo, baseBranch, branch, &github.ListOptions{PerPage: 1})
if err != nil {
return false, "", clt.wrapRetryableErrors(err)
}

if len(cmp.Commits) == 0 {
return false, "", errors.New("compare response contains 0 commits")
}

branchHEADSHA = cmp.Commits[len(cmp.Commits)-1].GetSHA()
if branchHEADSHA == "" {
return false, "", errors.New("sha field of last commit is empty")
return false, clt.wrapRetryableErrors(err)
}

return cmp.GetBehindBy() > 0, branchHEADSHA, nil
return cmp.GetBehindBy() > 0, nil
}

const (
Expand Down Expand Up @@ -155,12 +146,12 @@ func (clt *Client) PRIsUptodate(ctx context.Context, owner, repo string, pullReq
return false, "", errors.New("got pull request object with empty base ref field")
}

isBehind, branchHEADSHA, err := clt.BranchIsBehindBase(ctx, owner, repo, baseBranch, prBranch)
isBehind, err := clt.BranchIsBehindBase(ctx, owner, repo, baseBranch, prBranch)
if err != nil {
return false, branchHEADSHA, fmt.Errorf("evaluating if branch is behind base failed: %w", err)
return false, "", fmt.Errorf("evaluating if branch is behind base failed: %w", err)
}

return !isBehind, branchHEADSHA, nil
return !isBehind, prHeadSHA, nil
}

// CreateIssueComment creates a comment in a issue or pull request
Expand Down

0 comments on commit a468dd6

Please sign in to comment.