diff --git a/pkg/cmd/repo/sync/http.go b/pkg/cmd/repo/sync/http.go index 61a1c3317..3e193daeb 100644 --- a/pkg/cmd/repo/sync/http.go +++ b/pkg/cmd/repo/sync/http.go @@ -32,8 +32,6 @@ func latestCommit(client *api.Client, repo ghrepo.Interface, branch string) (com type upstreamMergeErr struct{ error } -var upstreamMergeUnavailableErr = upstreamMergeErr{errors.New("upstream merge API is unavailable")} - var missingWorkflowScopeRE = regexp.MustCompile("refusing to allow.*without `workflow` scope") var missingWorkflowScopeErr = errors.New("Upstream commits contain workflow changes, which require the `workflow` scope to merge. To request it, run: gh auth refresh -s workflow") @@ -60,8 +58,6 @@ func triggerUpstreamMerge(client *api.Client, repo ghrepo.Interface, branch stri return "", missingWorkflowScopeErr } return "", upstreamMergeErr{errors.New(httpErr.Message)} - case http.StatusNotFound: - return "", upstreamMergeUnavailableErr } } return "", err diff --git a/pkg/cmd/repo/sync/sync.go b/pkg/cmd/repo/sync/sync.go index 1f734b543..dd52868d1 100644 --- a/pkg/cmd/repo/sync/sync.go +++ b/pkg/cmd/repo/sync/sync.go @@ -284,6 +284,13 @@ func executeLocalRepoSync(srcRepo ghrepo.Interface, remote string, opts *SyncOpt return nil } +// ExecuteRemoteRepoSync will take several steps to sync the source and destination repositories. +// First it will try to use the merge-upstream API endpoint. If this fails due to merge conflicts +// or unknown merge issues then it will fallback to using the low level git references API endpoint. +// The reason the fallback is necessary is to better support these error cases. The git references API +// endpoint allows us to sync repositories that are not fast-forward merge compatible. Additionally, +// the git references API endpoint gives more detailed error responses as to why the sync failed. +// Unless the --force flag is specified we will not perform non-fast-forward merges. func executeRemoteRepoSync(client *api.Client, destRepo, srcRepo ghrepo.Interface, opts *SyncOptions) (string, error) { branchName := opts.Branch if branchName == "" { @@ -317,8 +324,9 @@ func executeRemoteRepoSync(client *api.Client, destRepo, srcRepo ghrepo.Interfac return "", err } - // This is not a great way to detect the error returned by the API - // Unfortunately API returns 422 for multiple reasons + // Using string comparison is a brittle way to determine the error returned by the API + // endpoint but unfortunately the API returns 422 for many reasons so we must + // interpret the message provide better error messaging for our users. err = syncFork(client, destRepo, branchName, commit.Object.SHA, opts.Force) var httpErr api.HTTPError if err != nil { diff --git a/pkg/cmd/repo/sync/sync_test.go b/pkg/cmd/repo/sync/sync_test.go index a532f3992..eafc0835e 100644 --- a/pkg/cmd/repo/sync/sync_test.go +++ b/pkg/cmd/repo/sync/sync_test.go @@ -292,7 +292,7 @@ func Test_SyncRun(t *testing.T) { httpmock.StringResponse(`{"data":{"repository":{"defaultBranchRef":{"name": "trunk"}}}}`)) reg.Register( httpmock.REST("POST", "repos/FORKOWNER/REPO-FORK/merge-upstream"), - httpmock.StatusStringResponse(404, `{}`)) + httpmock.StatusStringResponse(422, `{}`)) reg.Register( httpmock.REST("GET", "repos/OWNER/REPO/git/refs/heads/trunk"), httpmock.StringResponse(`{"object":{"sha":"0xDEADBEEF"}}`))