From 8defca6d39c6e83cf29cd77918db687fe2fe1802 Mon Sep 17 00:00:00 2001 From: Giteabot Date: Thu, 5 Jun 2025 15:15:32 +0800 Subject: [PATCH] Fix possible pull request broken when leave the page immediately after clicking the update button (#34509) (#34607) Backport #34509 by @lunny If user leaves the page, the context will become cancelled, so that the update process maybe terminal in an unexpected status. This PR haven't resolve the problem totally. It uses a background context to not cancel the update process even if the user leaved the pull request view page. Fix #31779 Co-authored-by: Lunny Xiao --- routers/api/v1/repo/pull.go | 3 ++- routers/web/repo/pull.go | 5 ++++- services/pull/update.go | 45 +++++++++++++------------------------ 3 files changed, 22 insertions(+), 31 deletions(-) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index d1bcaefb52..2c194f9253 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -23,6 +23,7 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" + "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" @@ -1301,7 +1302,7 @@ func UpdatePullRequest(ctx *context.APIContext) { // default merge commit message message := fmt.Sprintf("Merge branch '%s' into %s", pr.BaseBranch, pr.HeadBranch) - if err = pull_service.Update(ctx, pr, ctx.Doer, message, rebase); err != nil { + if err = pull_service.Update(graceful.GetManager().ShutdownContext(), pr, ctx.Doer, message, rebase); err != nil { if pull_service.IsErrMergeConflicts(err) { ctx.APIError(http.StatusConflict, "merge failed because of conflict") return diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 58c052db9e..86f4f0167f 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -26,6 +26,7 @@ import ( "code.gitea.io/gitea/modules/emoji" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" + "code.gitea.io/gitea/modules/graceful" issue_template "code.gitea.io/gitea/modules/issue/template" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" @@ -980,7 +981,9 @@ func UpdatePullRequest(ctx *context.Context) { // default merge commit message message := fmt.Sprintf("Merge branch '%s' into %s", issue.PullRequest.BaseBranch, issue.PullRequest.HeadBranch) - if err = pull_service.Update(ctx, issue.PullRequest, ctx.Doer, message, rebase); err != nil { + // The update process should not be cancelled by the user + // so we set the context to be a background context + if err = pull_service.Update(graceful.GetManager().ShutdownContext(), issue.PullRequest, ctx.Doer, message, rebase); err != nil { if pull_service.IsErrMergeConflicts(err) { conflictError := err.(pull_service.ErrMergeConflicts) flashError, err := ctx.RenderToHTML(tplAlertDetails, map[string]any{ diff --git a/services/pull/update.go b/services/pull/update.go index 5cc5e2b134..b8f84e3d65 100644 --- a/services/pull/update.go +++ b/services/pull/update.go @@ -41,22 +41,6 @@ func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model. return fmt.Errorf("HeadBranch of PR %d is up to date", pr.Index) } - if rebase { - defer func() { - go AddTestPullRequestTask(TestPullRequestOptions{ - RepoID: pr.BaseRepo.ID, - Doer: doer, - Branch: pr.BaseBranch, - IsSync: false, - IsForcePush: false, - OldCommitID: "", - NewCommitID: "", - }) - }() - - return updateHeadByRebaseOnToBase(ctx, pr, doer) - } - if err := pr.LoadBaseRepo(ctx); err != nil { log.Error("unable to load BaseRepo for %-v during update-by-merge: %v", pr, err) return fmt.Errorf("unable to load BaseRepo for PR[%d] during update-by-merge: %w", pr.ID, err) @@ -74,6 +58,22 @@ func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model. return fmt.Errorf("unable to load HeadRepo for PR[%d] during update-by-merge: %w", pr.ID, err) } + defer func() { + go AddTestPullRequestTask(TestPullRequestOptions{ + RepoID: pr.BaseRepo.ID, + Doer: doer, + Branch: pr.BaseBranch, + IsSync: false, + IsForcePush: false, + OldCommitID: "", + NewCommitID: "", + }) + }() + + if rebase { + return updateHeadByRebaseOnToBase(ctx, pr, doer) + } + // TODO: FakePR: it is somewhat hacky, but it is the only way to "merge" at the moment // ideally in the future the "merge" functions should be refactored to decouple from the PullRequest // now use a fake reverse PR to switch head&base repos/branches @@ -90,19 +90,6 @@ func Update(ctx context.Context, pr *issues_model.PullRequest, doer *user_model. } _, err = doMergeAndPush(ctx, reversePR, doer, repo_model.MergeStyleMerge, "", message, repository.PushTriggerPRUpdateWithBase) - - defer func() { - go AddTestPullRequestTask(TestPullRequestOptions{ - RepoID: reversePR.HeadRepo.ID, - Doer: doer, - Branch: reversePR.HeadBranch, - IsSync: false, - IsForcePush: false, - OldCommitID: "", - NewCommitID: "", - }) - }() - return err }