Merge branch 'eb/fix-internal-errors' of _OKE5H2PQKOUfzFFDuD4FA/default/CODE/gitness (#332)

gitrpc/commit.go

@@ -219,7 +219,7 @@ func (c *Client) GetCommitDivergences(ctx context.Context,
 
 	divergences := resp.GetDivergences()
 	if divergences == nil {
-		return nil, fmt.Errorf("server response divergences were nil")
+		return nil, NewError(StatusInternal, "server response divergences were nil")
 	}
 
 	// build output
@@ -228,7 +228,7 @@ func (c *Client) GetCommitDivergences(ctx context.Context,
 	}
 	for i := range divergences {
 		if divergences[i] == nil {
-			return nil, fmt.Errorf("server returned nil divergence")
+			return nil, NewError(StatusInternal, "server returned nil divergence")
 		}
 
 		output.Divergences[i] = CommitDivergence{

gitrpc/diff.go

@@ -82,7 +82,8 @@ func (c *Client) DiffShortStat(ctx context.Context, params *DiffParams) (DiffSho
 		MergeBase: params.MergeBase,
 	})
 	if err != nil {
-		return DiffShortStatOutput{}, err
+		return DiffShortStatOutput{}, processRPCErrorf(err, "failed to get diff data between '%s' and '%s'",
+			params.BaseRef, params.HeadRef)
 	}
 	return DiffShortStatOutput{
 		Files:     int(stat.GetFiles()),
@@ -122,7 +123,8 @@ func (c *Client) DiffStats(ctx context.Context, params *DiffParams) (DiffStatsOu
 
 		rpcOutput, err := c.GetCommitDivergences(groupCtx, options)
 		if err != nil {
-			return fmt.Errorf("failed to count pull request commits: %w", err)
+			return processRPCErrorf(err, "failed to count pull request commits between '%s' and '%s'",
+				params.BaseRef, params.HeadRef)
 		}
 		if len(rpcOutput.Divergences) > 0 {
 			totalCommits = int(rpcOutput.Divergences[0].Ahead)
@@ -139,7 +141,7 @@ func (c *Client) DiffStats(ctx context.Context, params *DiffParams) (DiffStatsOu
 			MergeBase:  true, // must be true, because commitDivergences use tripple dot notation
 		})
 		if err != nil {
-			return fmt.Errorf("failed to count pull request file changes: %w", err)
+			return err
 		}
 		totalFiles = stat.Files
 		return nil

gitrpc/errors.go

@@ -127,6 +127,9 @@ func ErrInvalidArgumentf(format string, args ...interface{}) *Error {
 }
 
 func processRPCErrorf(err error, format string, args ...interface{}) error {
+	if errors.Is(err, &Error{}) {
+		return err
+	}
 	// create fallback error returned if we can't map it
 	fallbackMsg := fmt.Sprintf(format, args...)
 	fallbackErr := NewError(StatusInternal, fallbackMsg)

gitrpc/internal/gitea/diff.go

@@ -64,8 +64,8 @@ func (g Adapter) DiffShortStat(
 	}
 	numFiles, totalAdditions, totalDeletions, err := git.GetDiffShortStat(ctx, repoPath, shortstatArgs...)
 	if err != nil {
-		return types.DiffShortStat{}, fmt.Errorf("failed to get diff short stat between %s and %s with err: %w",
+		return types.DiffShortStat{}, processGiteaErrorf(err, "failed to get diff short stat between %s and %s",
-			baseRef, headRef, err)
+			baseRef, headRef)
 	}
 	return types.DiffShortStat{
 		Files:     numFiles,

gitrpc/internal/gitea/mapping.go

@@ -63,7 +63,16 @@ func mapGiteaRunStdError(err gitea.RunStdError, fallback error) error {
 
 	// exit status 128 - fatal: ambiguous argument 'branch1...branch2': unknown revision or path not in the working tree.
 	case err.IsExitCode(128) && strings.Contains(err.Stderr(), "unknown revision"):
-		return types.ErrNotFound
+		msg := "unknown revision or path not in the working tree"
+		// parse the error response from git output
+		lines := strings.Split(err.Error(), "\n")
+		if len(lines) > 0 {
+			cols := strings.Split(lines[0], ": ")
+			if len(cols) >= 2 {
+				msg = cols[1] + ", " + cols[2]
+			}
+		}
+		return fmt.Errorf("%v err: %w", msg, types.ErrNotFound)
 
 	// exit status 128 - fatal: couldn't find remote ref v1.
 	case err.IsExitCode(128) && strings.Contains(err.Stderr(), "couldn't find"):

gitrpc/internal/gitea/merge.go

@@ -106,7 +106,7 @@ func (g Adapter) CreateTemporaryRepoForPR(
 	// Fetch base branch
 	baseCommit, err := g.GetCommit(ctx, pr.BaseRepoPath, pr.BaseBranch)
 	if err != nil {
-		return types.TempRepository{}, fmt.Errorf("failed to get commit of %s branch: %w", baseBranch, err)
+		return types.TempRepository{}, fmt.Errorf("failed to get commit of base branch '%s', error: %w", pr.BaseBranch, err)
 	}
 	baseID := baseCommit.SHA
 	if err = git.NewCommand(ctx, "fetch", "origin", "--no-tags", "--",
@@ -162,7 +162,7 @@ func (g Adapter) CreateTemporaryRepoForPR(
 
 	headCommit, err := g.GetCommit(ctx, pr.HeadRepoPath, pr.HeadBranch)
 	if err != nil {
-		return types.TempRepository{}, fmt.Errorf("failed to get commit of %s branch: %w", trackingBranch, err)
+		return types.TempRepository{}, fmt.Errorf("failed to get commit of head branch '%s', error: %w", pr.HeadBranch, err)
 	}
 	headID := headCommit.SHA
 	if err = git.NewCommand(ctx, "fetch", "--no-tags", remoteRepoName, headID+":"+trackingBranch).

gitrpc/internal/service/diff.go

@@ -77,8 +77,7 @@ func validateDiffRequest(in *rpc.DiffRequest) error {
 func (s DiffService) DiffShortStat(ctx context.Context, r *rpc.DiffRequest) (*rpc.DiffShortStatResponse, error) {
 	err := validateDiffRequest(r)
 	if err != nil {
-		return nil, fmt.Errorf("failed to validate request for short diff statistic "+
+		return nil, fmt.Errorf("failed to validate request for short diff statistic, error: %v", err)
-			"between %s and %s with err: %w", r.GetBaseRef(), r.GetHeadRef(), err)
 	}
 
 	base := r.GetBase()
@@ -91,8 +90,8 @@ func (s DiffService) DiffShortStat(ctx context.Context, r *rpc.DiffRequest) (*rp
 
 	stat, err := s.adapter.DiffShortStat(ctx, repoPath, r.GetBaseRef(), r.GetHeadRef(), direct)
 	if err != nil {
-		return nil, fmt.Errorf("failed to fetch short statistics "+
+		return nil, processGitErrorf(err, "failed to fetch short statistics "+
-			"between %s and %s with err: %w", r.GetBaseRef(), r.GetHeadRef(), err)
+			"between %s and %s", r.GetBaseRef(), r.GetHeadRef())
 	}
 
 	return &rpc.DiffShortStatResponse{

gitrpc/internal/service/errors.go

@@ -201,11 +201,11 @@ func processGitErrorf(err error, format string, args ...interface{}) error {
 		errors.Is(err, types.ErrSHADoesNotMatch),
 		errors.Is(err, types.ErrHunkNotFound),
 		errors.Is(err, types.ErrPathNotFound):
-		return ErrNotFoundf(format, args...)
+		return ErrNotFound(err)
 	case errors.Is(err, types.ErrAlreadyExists):
-		return ErrAlreadyExistsf(format, args...)
+		return ErrAlreadyExists(err)
 	case errors.Is(err, types.ErrInvalidArgument):
-		return ErrInvalidArgumentf(format, args...)
+		return ErrInvalidArgument(err)
 	case errors.As(err, &cferr):
 		stdout := strings.Trim(cferr.StdOut, nl)
 		conflictingFiles := strings.Split(stdout, nl)
@@ -214,9 +214,9 @@ func processGitErrorf(err error, format string, args ...interface{}) error {
 		}
 		return ErrFailedPreconditionf("merging failed due to conflicting changes with the target branch", files, err)
 	case types.IsMergeUnrelatedHistoriesError(err):
-		return ErrFailedPreconditionf(format, args...)
+		return ErrFailedPrecondition(err)
 	case errors.Is(err, types.ErrFailedToConnect):
-		return ErrInvalidArgumentf(format, args...)
+		return ErrInvalidArgument(err)
 	default:
 		return ErrInternalf(format, args...)
 	}

gitrpc/internal/service/merge.go

@@ -64,7 +64,7 @@ func (s MergeService) Merge(
 	// Clone base repo.
 	tmpRepo, err := s.adapter.CreateTemporaryRepoForPR(ctx, s.reposTempDir, pr, baseBranch, trackingBranch)
 	if err != nil {
-		return nil, err
+		return nil, processGitErrorf(err, "failed to initialize temporary repo")
 	}
 	defer func() {
 		rmErr := tempdir.RemoveTemporaryPath(tmpRepo.Path)