Fix git error handling (#32401)

This commit is contained in:
wxiaoguang 2024-11-02 19:20:22 +08:00 committed by GitHub
parent 13a203828c
commit e524f63d58
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 35 additions and 89 deletions

View File

@ -4,28 +4,14 @@
package git
import (
"context"
"errors"
"fmt"
"strings"
"time"
"code.gitea.io/gitea/modules/util"
)
// ErrExecTimeout error when exec timed out
type ErrExecTimeout struct {
Duration time.Duration
}
// IsErrExecTimeout if some error is ErrExecTimeout
func IsErrExecTimeout(err error) bool {
_, ok := err.(ErrExecTimeout)
return ok
}
func (err ErrExecTimeout) Error() string {
return fmt.Sprintf("execution is timeout [duration: %v]", err.Duration)
}
// ErrNotExist commit not exist error
type ErrNotExist struct {
ID string
@ -62,21 +48,6 @@ func IsErrBadLink(err error) bool {
return ok
}
// ErrUnsupportedVersion error when required git version not matched
type ErrUnsupportedVersion struct {
Required string
}
// IsErrUnsupportedVersion if some error is ErrUnsupportedVersion
func IsErrUnsupportedVersion(err error) bool {
_, ok := err.(ErrUnsupportedVersion)
return ok
}
func (err ErrUnsupportedVersion) Error() string {
return fmt.Sprintf("Operation requires higher version [required: %s]", err.Required)
}
// ErrBranchNotExist represents a "BranchNotExist" kind of error.
type ErrBranchNotExist struct {
Name string
@ -185,3 +156,10 @@ func IsErrMoreThanOne(err error) bool {
func (err *ErrMoreThanOne) Error() string {
return fmt.Sprintf("ErrMoreThanOne Error: %v: %s\n%s", err.Err, err.StdErr, err.StdOut)
}
func IsErrCanceledOrKilled(err error) bool {
// When "cancel()" a git command's context, the returned error of "Run()" could be one of them:
// - context.Canceled
// - *exec.ExitError: "signal: killed"
return err != nil && (errors.Is(err, context.Canceled) || err.Error() == "signal: killed")
}

View File

@ -166,9 +166,7 @@ func (c *CheckAttributeReader) Run() error {
Stdout: c.stdOut,
Stderr: stdErr,
})
if err != nil && // If there is an error we need to return but:
c.ctx.Err() != err && // 1. Ignore the context error if the context is cancelled or exceeds the deadline (RunWithContext could return c.ctx.Err() which is Canceled or DeadlineExceeded)
err.Error() != "signal: killed" { // 2. We should not pass up errors due to the program being killed
if err != nil && !IsErrCanceledOrKilled(err) {
return fmt.Errorf("failed to run attr-check. Error: %w\nStderr: %s", err, stdErr.String())
}
return nil

View File

@ -21,7 +21,6 @@ import (
repo_model "code.gitea.io/gitea/models/repo"
unit_model "code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/gitrepo"
"code.gitea.io/gitea/modules/label"
"code.gitea.io/gitea/modules/log"
@ -739,11 +738,9 @@ func updateBasicProperties(ctx *context.APIContext, opts api.EditRepoOption) err
if opts.DefaultBranch != nil && repo.DefaultBranch != *opts.DefaultBranch && (repo.IsEmpty || ctx.Repo.GitRepo.IsBranchExist(*opts.DefaultBranch)) {
if !repo.IsEmpty {
if err := gitrepo.SetDefaultBranch(ctx, ctx.Repo.Repository, *opts.DefaultBranch); err != nil {
if !git.IsErrUnsupportedVersion(err) {
ctx.Error(http.StatusInternalServerError, "SetDefaultBranch", err)
return err
}
}
updateRepoLicense = true
}
repo.DefaultBranch = *opts.DefaultBranch

View File

@ -8,7 +8,6 @@ import (
"net/http"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/gitrepo"
"code.gitea.io/gitea/modules/private"
gitea_context "code.gitea.io/gitea/services/context"
@ -23,13 +22,11 @@ func SetDefaultBranch(ctx *gitea_context.PrivateContext) {
ctx.Repo.Repository.DefaultBranch = branch
if err := gitrepo.SetDefaultBranch(ctx, ctx.Repo.Repository, ctx.Repo.Repository.DefaultBranch); err != nil {
if !git.IsErrUnsupportedVersion(err) {
ctx.JSON(http.StatusInternalServerError, private.Response{
Err: fmt.Sprintf("Unable to set default branch on repository: %s/%s Error: %v", ownerName, repoName, err),
})
return
}
}
if err := repo_model.UpdateDefaultBranch(ctx, ctx.Repo.Repository); err != nil {
ctx.JSON(http.StatusInternalServerError, private.Response{

View File

@ -467,7 +467,7 @@ func serviceRPC(ctx *context.Context, h *serviceHandler, service string) {
Stderr: &stderr,
UseContextTimeout: true,
}); err != nil {
if err.Error() != "signal: killed" {
if !git.IsErrCanceledOrKilled(err) {
log.Error("Fail to serve RPC(%s) in %s: %v - %s", service, h.getRepoDir(), err, stderr.String())
}
return

View File

@ -1162,7 +1162,7 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi
Dir: repoPath,
Stdout: writer,
Stderr: stderr,
}); err != nil && err.Error() != "signal: killed" {
}); err != nil && !git.IsErrCanceledOrKilled(err) {
log.Error("error during GetDiff(git diff dir: %s): %v, stderr: %s", repoPath, err, stderr.String())
}

View File

@ -613,15 +613,9 @@ func checkAndUpdateEmptyRepository(ctx context.Context, m *repo_model.Mirror, re
}
// Update the git repository default branch
if err := gitrepo.SetDefaultBranch(ctx, m.Repo, m.Repo.DefaultBranch); err != nil {
if !git.IsErrUnsupportedVersion(err) {
log.Error("Failed to update default branch of underlying git repository %-v. Error: %v", m.Repo, err)
desc := fmt.Sprintf("Failed to update default branch of underlying git repository '%s': %v", m.Repo.RepoPath(), err)
if err = system_model.CreateRepositoryNotice(desc); err != nil {
log.Error("CreateRepositoryNotice: %v", err)
}
return false
}
}
m.Repo.IsEmpty = false
// Update the is empty and default_branch columns
if err := repo_model.UpdateRepositoryCols(ctx, m.Repo, "default_branch", "is_empty"); err != nil {

View File

@ -602,12 +602,7 @@ func SetRepoDefaultBranch(ctx context.Context, repo *repo_model.Repository, gitR
log.Error("CancelPreviousJobs: %v", err)
}
if err := gitrepo.SetDefaultBranch(ctx, repo, newBranchName); err != nil {
if !git.IsErrUnsupportedVersion(err) {
return err
}
}
return nil
return gitrepo.SetDefaultBranch(ctx, repo, newBranchName)
}); err != nil {
return err
}

View File

@ -339,8 +339,7 @@ func (t *TemporaryUploadRepository) Push(doer *user_model.User, commitHash, bran
func (t *TemporaryUploadRepository) DiffIndex() (*gitdiff.Diff, error) {
stdoutReader, stdoutWriter, err := os.Pipe()
if err != nil {
log.Error("Unable to open stdout pipe: %v", err)
return nil, fmt.Errorf("Unable to open stdout pipe: %w", err)
return nil, fmt.Errorf("unable to open stdout pipe: %w", err)
}
defer func() {
_ = stdoutReader.Close()
@ -348,9 +347,7 @@ func (t *TemporaryUploadRepository) DiffIndex() (*gitdiff.Diff, error) {
}()
stderr := new(bytes.Buffer)
var diff *gitdiff.Diff
var finalErr error
if err := git.NewCommand(t.ctx, "diff-index", "--src-prefix=\\a/", "--dst-prefix=\\b/", "--cached", "-p", "HEAD").
err = git.NewCommand(t.ctx, "diff-index", "--src-prefix=\\a/", "--dst-prefix=\\b/", "--cached", "-p", "HEAD").
Run(&git.RunOpts{
Timeout: 30 * time.Second,
Dir: t.basePath,
@ -359,27 +356,19 @@ func (t *TemporaryUploadRepository) DiffIndex() (*gitdiff.Diff, error) {
PipelineFunc: func(ctx context.Context, cancel context.CancelFunc) error {
_ = stdoutWriter.Close()
defer cancel()
diff, finalErr = gitdiff.ParsePatch(t.ctx, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, stdoutReader, "")
if finalErr != nil {
log.Error("ParsePatch: %v", finalErr)
cancel()
}
var diffErr error
diff, diffErr = gitdiff.ParsePatch(t.ctx, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, stdoutReader, "")
_ = stdoutReader.Close()
return finalErr
if diffErr != nil {
// if the diffErr is not nil, it will be returned as the error of "Run()"
return fmt.Errorf("ParsePatch: %w", diffErr)
}
return nil
},
}); err != nil {
if finalErr != nil {
log.Error("Unable to ParsePatch in temporary repo %s (%s). Error: %v", t.repo.FullName(), t.basePath, finalErr)
return nil, finalErr
}
// If the process exited early, don't error
if err != context.Canceled {
log.Error("Unable to run diff-index pipeline in temporary repo %s (%s). Error: %v\nStderr: %s",
t.repo.FullName(), t.basePath, err, stderr)
return nil, fmt.Errorf("Unable to run diff-index pipeline in temporary repo %s. Error: %w\nStderr: %s",
t.repo.FullName(), err, stderr)
}
})
if err != nil && !git.IsErrCanceledOrKilled(err) {
log.Error("Unable to diff-index in temporary repo %s (%s). Error: %v\nStderr: %s", t.repo.FullName(), t.basePath, err, stderr)
return nil, fmt.Errorf("unable to run diff-index pipeline in temporary repo: %w", err)
}
diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(t.ctx, t.basePath, git.TrustedCmdArgs{"--cached"}, "HEAD")

View File

@ -183,11 +183,9 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error {
repo.IsEmpty = false
if repo.DefaultBranch != setting.Repository.DefaultBranch {
if err := gitrepo.SetDefaultBranch(ctx, repo, repo.DefaultBranch); err != nil {
if !git.IsErrUnsupportedVersion(err) {
return err
}
}
}
// Update the is empty and default_branch columns
if err := repo_model.UpdateRepositoryCols(ctx, repo, "default_branch", "is_empty"); err != nil {
return fmt.Errorf("UpdateRepositoryCols: %w", err)