Add context around checkout failures #210

vshah23 · 2023-03-08T10:02:59Z

Checklist

I've read and followed the Contribution Guidelines
step.yml and README.md is updated with the changes (if needed)

Version

Context

[CI-620] We want to improve the readability of error messages and suggest to users actions they can take to more quickly resolve build failures.

Changes

Add context around checkout failures
Added suggestions for fetch failures

Investigation details

Decisions

gitclone/checkout_method_pr_auto_diff.go

gitclone/checkout_method_pr_auto_merge_branch.go

gitclone/checkout_method_pr_manual.go

ofalvai

Looks good so far, just a few minor issues!

ofalvai · 2023-03-08T16:26:03Z

gitclone/checkout_helper.go

@@ -138,7 +139,8 @@ func mergeWithCustomRetry(gitCmd git.Git, arg string, retry fallbackRetry) error
 			return runner.Run(gitCmd.Merge(arg))
 		}

-		return fmt.Errorf("merge failed (%s): %v", arg, mErr)
+		wErr := fmt.Errorf("merge failed (%s): %w%w", arg, mErr)


I incidentally stumbled into exactly this kind of error in CI a few hours ago 😆 Looks like the mErr variable is empty, while you are at it, can you look into why this is printed like this? Don't spend too much time on it if it's not an obvious issue, I just wanted to highlight 😇

Hmmm we're trying to read the error output from the command, but I guess git doesn't treat this scenario as an error in its logging even though it exits with exit status 1

I added additional messaging for this scenario: 244e94f

The output looks like:

gitclone/checkout_helper.go

vshah23 · 2023-03-15T22:38:13Z

gitclone/checkout_method_pr_auto_merge_branch.go

@@ -43,7 +43,7 @@ func (c checkoutPRMergeRef) do(gitCmd git.Git, fetchOpts fetchOptions, fallback
 	// $ git fetch origin refs/remotes/pull/7/merge:refs/pull/7/merge
 	err := fetch(gitCmd, originRemoteName, refSpec, fetchOpts)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to fetch merge ref: %w", err)


vshah23 · 2023-03-15T22:39:11Z

gitclone/checkout_method_pr_auto_diff.go

@@ -43,7 +43,7 @@ type checkoutPRDiffFile struct {
 func (c checkoutPRDiffFile) do(gitCmd git.Git, fetchOptions fetchOptions, fallback fallbackRetry) error {
 	destBranchRef := refsHeadsPrefix + c.params.DestinationBranch
 	if err := fetch(gitCmd, originRemoteName, destBranchRef, fetchOptions); err != nil {
-		return err
+		return fmt.Errorf("failed to fetch base branch: %w", err)


vshah23 · 2023-03-15T22:45:48Z

gitclone/checkout_method_pr_manual.go

@@ -62,7 +62,7 @@ func (c checkoutPRManualMerge) do(gitCmd git.Git, fetchOptions fetchOptions, fal
 	// Fetch and checkout destinations branch
 	destBranchRef := refsHeadsPrefix + c.params.DestinationBranch
 	if err := fetchInitialBranch(gitCmd, originRemoteName, destBranchRef, fetchOptions); err != nil {
-		return err
+		return fmt.Errorf("failed to fetch base branch: %w", err)


vshah23 · 2023-03-15T22:53:28Z

gitclone/checkout_method_pr_manual.go

@@ -87,7 +87,7 @@ func (c checkoutPRManualMerge) do(gitCmd git.Git, fetchOptions fetchOptions, fal
 	// Fetch and merge
 	sourceBranchRef := refsHeadsPrefix + c.params.SourceBranch
 	if err := fetch(gitCmd, remoteName, sourceBranchRef, fetchOptions); err != nil {
-		return err
+		return fmt.Errorf("failed to fetch compare branch: %w", err)


vshah23 · 2023-03-15T22:56:25Z

Note: The formatting is a bit messed up because the steperror package in go-steputils v1 does not have the error unwrapping interface implemented.

This is how it looks with those changes in place:

I opened a PR in go-steputils to add this: bitrise-io/go-steputils#77

vshah23 · 2023-03-17T03:14:15Z

gitclone/checkout_method_simple.go

-		return err
+		err = fmt.Errorf("failed to checkout commit: %w", err)
+		newErr := fmt.Errorf("please check if the provided commit hash (%s) is valid", c.params.Commit)
+		return fmt.Errorf("%v: %w", err, newErr)


vshah23 · 2023-03-17T10:52:37Z

gitclone/checkout_helper.go

@@ -72,7 +73,7 @@ func fetch(gitCmd git.Git, remote string, ref string, options fetchOptions) erro
 		return handleCheckoutError(
 			listBranches(gitCmd),
 			fetchFailedTag,
-			fmt.Errorf("fetch failed: %v", err),


Removing fetch failed: from here since the caller of this method has more context on what went wrong we add additional error messaging at the various call sites instead.

vshah23 · 2023-03-17T10:53:45Z

gitclone/checkout_helper.go

@@ -101,7 +102,8 @@ func checkoutWithCustomRetry(gitCmd git.Git, arg string, retry fallbackRetry) er
 func fetchInitialBranch(gitCmd git.Git, remote string, branchRef string, fetchTraits fetchOptions) error {
 	branch := strings.TrimPrefix(branchRef, refsHeadsPrefix)
 	if err := fetch(gitCmd, remote, branchRef, fetchTraits); err != nil {
-		return err
+		wErr := fmt.Errorf("failed to fetch branch (%s): %w", branchRef, err)
+		return fmt.Errorf("%v: %w", wErr, errors.New("please make sure the branch still exists"))


vshah23 · 2023-03-17T11:18:59Z

gitclone/checkout_helper.go

@@ -119,7 +121,7 @@ func fetchInitialBranch(gitCmd git.Git, remote string, branchRef string, fetchTr
 	if err := runner.Run(gitCmd.Merge(remoteBranch)); err != nil {
 		return newStepError(
 			"update_branch_failed",
-			fmt.Errorf("updating branch (merge) failed %s: %v", branch, err),
+			fmt.Errorf("updating branch (merge) failed %s: %w", branch, err),


…ethod

Co-authored-by: Olivér Falvai <oliver.falvai@bitrise.io>

Co-authored-by: lpusok <7979773+lpusok@users.noreply.github.com>

gitclone/checkout_helper.go

Co-authored-by: Olivér Falvai <oliver.falvai@bitrise.io>

vshah23 requested review from tothszabi, ofalvai, lpusok and godrei March 8, 2023 10:03

ofalvai reviewed Mar 8, 2023

View reviewed changes

gitclone/checkout_method_pr_auto_diff.go Outdated Show resolved Hide resolved

ofalvai reviewed Mar 8, 2023

View reviewed changes

gitclone/checkout_method_pr_auto_merge_branch.go Outdated Show resolved Hide resolved

ofalvai reviewed Mar 8, 2023

View reviewed changes

gitclone/checkout_method_pr_manual.go Outdated Show resolved Hide resolved

ofalvai reviewed Mar 8, 2023

View reviewed changes

gitclone/checkout_method_pr_manual.go Outdated Show resolved Hide resolved

ofalvai requested changes Mar 8, 2023

View reviewed changes

lpusok reviewed Mar 8, 2023

View reviewed changes

gitclone/checkout_helper.go Outdated Show resolved Hide resolved

vshah23 marked this pull request as ready for review March 9, 2023 13:54

godrei previously approved these changes Mar 13, 2023

View reviewed changes

gitclone/checkout_helper.go Outdated Show resolved Hide resolved

vshah23 dismissed godrei’s stale review via c6249f9 March 14, 2023 06:01

vshah23 commented Mar 15, 2023

View reviewed changes

vshah23 commented Mar 17, 2023

View reviewed changes

vshah23 and others added 9 commits March 20, 2023 08:47

Added context around failure to fetch branch for different checkout m…

2e9087f

…ethod

Added suggestions for fetch failures

61f24f7

Update gitclone/checkout_method_pr_auto_diff.go

873f30e

Co-authored-by: Olivér Falvai <oliver.falvai@bitrise.io>

Update gitclone/checkout_method_pr_auto_merge_branch.go

682654e

Co-authored-by: Olivér Falvai <oliver.falvai@bitrise.io>

Update gitclone/checkout_method_pr_manual.go

b2beb71

Co-authored-by: Olivér Falvai <oliver.falvai@bitrise.io>

Update gitclone/checkout_method_pr_manual.go

669347b

Co-authored-by: Olivér Falvai <oliver.falvai@bitrise.io>

Update gitclone/checkout_helper.go

33826b3

Co-authored-by: lpusok <7979773+lpusok@users.noreply.github.com>

Fix error formatting

5a0e930

Fixed failing unit test

3b61b86

vshah23 added 9 commits March 20, 2023 08:47

Fixed error wrapping for fetch errors

a8a4d1b

Remove steperror changes

6584480

Added additional context before commit checkout

202642a

Added a suggestion for simple commit checkout

bbf42d7

Added context and suggestion for initial branch fetch

b07b7f5

Wrap errors

aea4fb8

Add context when tag fetch failed

c2079e5

Fix tests

e274638

Handled empty error scenario

e3fee88

vshah23 force-pushed the CI-620 branch from 244e94f to e3fee88 Compare March 20, 2023 13:48

vshah23 requested review from ofalvai and lpusok March 20, 2023 14:10

ofalvai reviewed Mar 20, 2023

View reviewed changes

gitclone/checkout_helper.go Outdated Show resolved Hide resolved

Update gitclone/checkout_helper.go

3892320

Co-authored-by: Olivér Falvai <oliver.falvai@bitrise.io>

vshah23 requested a review from ofalvai March 20, 2023 15:50

Updated go-steputils dep

bd8708a

vshah23 force-pushed the CI-620 branch from b8ad735 to bd8708a Compare March 20, 2023 16:26

ofalvai approved these changes Mar 20, 2023

View reviewed changes

vshah23 merged commit f29bfca into master Mar 20, 2023

vshah23 deleted the CI-620 branch March 20, 2023 16:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add context around checkout failures #210

Add context around checkout failures #210

vshah23 commented Mar 8, 2023 •

edited by jira bot

Loading

ofalvai left a comment

ofalvai Mar 8, 2023

vshah23 Mar 20, 2023 •

edited

Loading

vshah23 Mar 20, 2023

vshah23 Mar 15, 2023

vshah23 Mar 15, 2023

vshah23 Mar 15, 2023

vshah23 Mar 15, 2023

vshah23 commented Mar 15, 2023 •

edited

Loading

vshah23 Mar 17, 2023

vshah23 Mar 17, 2023

vshah23 Mar 17, 2023

vshah23 Mar 17, 2023

Add context around checkout failures #210

Add context around checkout failures #210

Conversation

vshah23 commented Mar 8, 2023 • edited by jira bot Loading

Checklist

Version

Context

Changes

Investigation details

Decisions

ofalvai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vshah23 Mar 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vshah23 commented Mar 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vshah23 commented Mar 8, 2023 •

edited by jira bot

Loading

vshah23 Mar 20, 2023 •

edited

Loading

vshah23 commented Mar 15, 2023 •

edited

Loading