Handle HTTP 404 when deleting remote branch in pr merge

pkg/cmd/pr/merge/merge.go

@@ -460,10 +460,22 @@ func (m *mergeContext) deleteRemoteBranch() error {
 	if !m.merged {
  apiClient := api.NewClientFromHTTP(m.httpClient)
  err := api.BranchDeleteRemote(apiClient, m.baseRepo, m.pr.HeadRefName)
- var httpErr api.HTTPError
- // The ref might have already been deleted by GitHub
- if err != nil && (!errors.As(err, &httpErr) || httpErr.StatusCode != 422) {
- return fmt.Errorf("failed to delete remote branch %s: %w", m.cs.Cyan(m.pr.HeadRefName), err)
+ if err != nil {
+ // Normally, the API returns 422, with the message "Reference does not exist"
+ // when the branch has already been deleted. It also returns 404 with the same
+ // message, but that rarely happens. In both cases, we should not return an
+ // error because the goal is already achieved.
+
+ var isAlreadyDeletedError bool
+ if httpErr := (api.HTTPError{}); errors.As(err, &httpErr) {
+ // TODO: since the API returns 422 for a couple of other reasons, for more accuracy
+ // we might want to check the error message against "Reference does not exist".
+ isAlreadyDeletedError = httpErr.StatusCode == http.StatusUnprocessableEntity || httpErr.StatusCode == http.StatusNotFound
+ }
+
+ if !isAlreadyDeletedError {
+ return fmt.Errorf("failed to delete remote branch %s: %w", m.cs.Cyan(m.pr.HeadRefName), err)
+ }
  }
 	}
 

pkg/cmd/pr/merge/merge_test.go

@@ -24,6 +24,7 @@ import (
 	"github.com/cli/cli/v2/pkg/httpmock"
 	"github.com/cli/cli/v2/pkg/iostreams"
 	"github.com/cli/cli/v2/test"
+	ghapi "github.com/cli/go-gh/v2/pkg/api"
 	"github.com/google/shlex"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -659,6 +660,103 @@ func TestPrMerge_deleteBranch(t *testing.T) {
 	`), output.Stderr())
 }
 
+func TestPrMerge_deleteBranch_apiError(t *testing.T) {
+	tests := []struct {
+ name string
+ apiError ghapi.HTTPError
+ wantErr string
+ wantStderr string
+	}{
+ {
+ name: "branch already deleted (422: Reference does not exist)",
+ apiError: ghapi.HTTPError{
+ Message: "Reference does not exist",
+ StatusCode: http.StatusUnprocessableEntity, // 422
+ },
+ wantStderr: heredoc.Doc(`
+ ✓ Merged pull request OWNER/REPO#10 (Blueberries are a good fruit)
+ ✓ Deleted local branch blueberries and switched to branch main
+ ✓ Deleted remote branch blueberries
+ `),
+ },
+ {
+ name: "branch already deleted (404: Reference does not exist) (#11187)",
+ apiError: ghapi.HTTPError{
+ Message: "Reference does not exist",
+ StatusCode: http.StatusNotFound, // 404
+ },
+ wantStderr: heredoc.Doc(`
+ ✓ Merged pull request OWNER/REPO#10 (Blueberries are a good fruit)
+ ✓ Deleted local branch blueberries and switched to branch main
+ ✓ Deleted remote branch blueberries
+ `),
+ },
+ {
+ name: "unknown API error",
+ apiError: ghapi.HTTPError{
+ Message: "blah blah",
+ StatusCode: http.StatusInternalServerError, // 500
+ },
+ wantStderr: heredoc.Doc(`
+ ✓ Merged pull request OWNER/REPO#10 (Blueberries are a good fruit)
+ ✓ Deleted local branch blueberries and switched to branch main
+ `),
+ wantErr: "failed to delete remote branch blueberries: HTTP 500: blah blah (https://api.github.com/repos/OWNER/REPO/git/refs/heads/blueberries)",
+ },
+	}
+
+	for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ http := initFakeHTTP()
+ defer http.Verify(t)
+
+ shared.StubFinderForRunCommandStyleTests(t,
+ "",
+ &api.PullRequest{
+ ID: "PR_10",
+ Number: 10,
+ State: "OPEN",
+ Title: "Blueberries are a good fruit",
+ HeadRefName: "blueberries",
+ BaseRefName: "main",
+ MergeStateStatus: "CLEAN",
+ },
+ baseRepo("OWNER", "REPO", "main"),
+ )
+
+ http.Register(
+ httpmock.GraphQL(`mutation PullRequestMerge\b`),
+ httpmock.GraphQLMutation(`{}`, func(input map[string]interface{}) {
+ assert.Equal(t, "PR_10", input["pullRequestId"].(string))
+ assert.Equal(t, "MERGE", input["mergeMethod"].(string))
+ assert.NotContains(t, input, "commitHeadline")
+ }))
+ http.Register(
+ httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"),
+ httpmock.JSONErrorResponse(tt.apiError.StatusCode, tt.apiError))
+
+ cs, cmdTeardown := run.Stub()
+ defer cmdTeardown(t)
+
+ cs.Register(`git rev-parse --verify refs/heads/main`, 0, "")
+ cs.Register(`git checkout main`, 0, "")
+ cs.Register(`git rev-parse --verify refs/heads/blueberries`, 0, "")
+ cs.Register(`git branch -D blueberries`, 0, "")
+ cs.Register(`git pull --ff-only`, 0, "")
+
+ output, err := runCommand(http, nil, "blueberries", true, `pr merge --merge --delete-branch`)
+ assert.Equal(t, "", output.String())
+ assert.Equal(t, tt.wantStderr, output.Stderr())
+
+ if tt.wantErr != "" {
+ assert.EqualError(t, err, tt.wantErr)
+ return
+ }
+ assert.NoError(t, err)
+ })
+	}
+}
+
 func TestPrMerge_deleteBranch_mergeQueue(t *testing.T) {
 	http := initFakeHTTP()
 	defer http.Verify(t)