fix: Error on PR creation by juanjosevazquezgil · Pull Request #414 · prefapp/daggerverse

juanjosevazquezgil · 2026-02-25T10:30:04Z

Closes #413

…/prefapp/daggerverse into fix/413-fix-error-on-pr-creation

Copilot

Pull request overview

This PR addresses intermittent errors when creating GitHub PRs via the gh Dagger module by adding retry/delay behavior, and updates the consuming module to a newer pinned version of that dependency.

Changes:

Update update-claims-features to pin a newer gh daggerverse dependency revision.
Add retry loops around gh pr create and PR-number lookup in Gh.CreatePR.
Add a fixed delay before calling CreatePR in CommitAndCreatePR.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
update-claims-features/dagger.json	Bumps the pinned `gh` module revision to pick up the fix.
gh/main.go	Adds retry/sleep logic intended to eliminate false-negative PR creation errors.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

gh/main.go

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

gh/main.go

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

gh/main.go

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-25T15:01:42Z

gh/main.go

+	for i := range MaxRetries {
+		_, err = ctr.Sync(ctx)
+
+		if err == nil {
+			break
+		}
+
+		if i == MaxRetries-1 {
+			return "", errors.New(
+				extractErrorMessage(err),
+			)
+		}
+
+		fmt.Printf("Error creating PR, retrying... (%d/%d)\n", i+1, MaxRetries-1)
+
+		timer := time.NewTimer(WaitTimeBetweenRetries)
+		select {
+		case <-timer.C:
+		case <-ctx.Done():
+			timer.Stop()
+			return "", ctx.Err()
+		}


The retry logic blindly retries all errors without distinguishing between transient failures (which should be retried) and permanent failures (which should fail immediately). For example, authentication errors, invalid repository errors, or permission errors should not be retried. Consider checking the error type or message to determine if the error is retryable before waiting and retrying.

Copilot · 2026-02-25T15:01:43Z

gh/main.go

+	fmt.Println("\nCreating PR...")
+	for i := range MaxRetries {
+		_, err = ctr.Sync(ctx)
+
+		if err == nil {
+			break
+		}
+
+		if i == MaxRetries-1 {
+			return "", errors.New(
+				extractErrorMessage(err),
+			)
+		}
+
+		fmt.Printf("Error creating PR, retrying... (%d/%d)\n", i+1, MaxRetries-1)
+
+		timer := time.NewTimer(WaitTimeBetweenRetries)
+		select {
+		case <-timer.C:
+		case <-ctx.Done():
+			timer.Stop()
+			return "", ctx.Err()
+		}
 	}

-	prId, err := ctr.
-		WithExec([]string{
-			"gh", "pr", "list",
-			"--head", branch,
-			"--json", "number",
-			"--jq", ".[0].number",
-		}).
-		Stdout(ctx)
+	var prId string
+	fmt.Println("Getting PR ID...")
+	for i := range MaxRetries {
+		prId, err = ctr.
+			WithExec([]string{
+				"gh", "pr", "list",
+				"--head", branch,
+				"--json", "number",
+				"--jq", ".[0].number",
+			}).
+			Stdout(ctx)

-	if err != nil {
-		return "", errors.New(
-			extractErrorMessage(err),
-		)
+		// Check if the prId is a valid integer (which means we got the PR ID successfully). If not, we retry.
+		_, convErr := strconv.Atoi(strings.TrimSpace(prId))
+
+		if err == nil && convErr == nil {
+			break
+		}
+
+		if i == MaxRetries-1 {
+			return "", errors.New(
+				extractErrorMessage(err),
+			)
+		}
+
+		fmt.Printf("Error getting PR ID, retrying... (%d/%d)\n", i+1, MaxRetries-1)


Using fmt.Println and fmt.Printf for logging in a library module is not ideal as it bypasses the caller's control over logging output and format. These print statements will always output to stdout, which may not be desired in all contexts. Consider either removing these debug statements for production code, or if logging is necessary, use a structured logging approach that can be configured by the caller (e.g., passing a logger interface).

Copilot · 2026-02-25T15:01:43Z

gh/main.go

+		timer := time.NewTimer(WaitTimeBetweenRetries)
+		select {
+		case <-timer.C:
+		case <-ctx.Done():
+			timer.Stop()
+			return "", ctx.Err()
+		}


The retry logic uses a fixed 2-second delay between retries. For handling timing issues with the GitHub API, an exponential backoff strategy would be more appropriate and efficient. This would reduce unnecessary load on the GitHub API and likely succeed faster in cases where GitHub needs more time to process the request. Consider implementing exponential backoff (e.g., 2s, 4s, 8s, 16s, 32s) instead of a fixed delay.

Copilot · 2026-02-25T15:01:43Z

gh/main.go

+	for i := range MaxRetries {
+		_, err = ctr.Sync(ctx)
+
+		if err == nil {
+			break
+		}
+
+		if i == MaxRetries-1 {
+			return "", errors.New(
+				extractErrorMessage(err),
+			)
+		}
+
+		fmt.Printf("Error creating PR, retrying... (%d/%d)\n", i+1, MaxRetries-1)
+
+		timer := time.NewTimer(WaitTimeBetweenRetries)
+		select {
+		case <-timer.C:
+		case <-ctx.Done():
+			timer.Stop()
+			return "", ctx.Err()
+		}
 	}

-	prId, err := ctr.
-		WithExec([]string{
-			"gh", "pr", "list",
-			"--head", branch,
-			"--json", "number",
-			"--jq", ".[0].number",
-		}).
-		Stdout(ctx)
+	var prId string
+	fmt.Println("Getting PR ID...")
+	for i := range MaxRetries {
+		prId, err = ctr.
+			WithExec([]string{
+				"gh", "pr", "list",
+				"--head", branch,
+				"--json", "number",
+				"--jq", ".[0].number",
+			}).
+			Stdout(ctx)

-	if err != nil {
-		return "", errors.New(
-			extractErrorMessage(err),
-		)
+		// Check if the prId is a valid integer (which means we got the PR ID successfully). If not, we retry.
+		_, convErr := strconv.Atoi(strings.TrimSpace(prId))
+
+		if err == nil && convErr == nil {
+			break
+		}
+
+		if i == MaxRetries-1 {
+			return "", errors.New(
+				extractErrorMessage(err),
+			)
+		}
+
+		fmt.Printf("Error getting PR ID, retrying... (%d/%d)\n", i+1, MaxRetries-1)
+
+		timer := time.NewTimer(WaitTimeBetweenRetries)
+		select {
+		case <-timer.C:
+		case <-ctx.Done():
+			timer.Stop()
+			return "", ctx.Err()
+		}
 	}


The retry logic is duplicated between the PR creation and PR ID retrieval sections. This code duplication makes maintenance harder and increases the risk of inconsistencies. Consider extracting the retry logic into a reusable helper function that accepts a function to execute and returns its result, similar to: retryWithBackoff(ctx context.Context, operation func() error, maxRetries int, waitTime time.Duration) error. This would also make it easier to improve the retry strategy (e.g., adding exponential backoff) in one place.

Copilot · 2026-02-25T15:01:43Z

gh/main.go

+		timer := time.NewTimer(WaitTimeBetweenRetries)
+		select {
+		case <-timer.C:
+		case <-ctx.Done():
+			timer.Stop()
+			return "", ctx.Err()
+		}


The timer should be stopped after the select statement completes to free resources, regardless of which case is triggered. Currently, the timer is only stopped when the context is cancelled. If the timer fires normally (case <-timer.C), the timer resources are not explicitly cleaned up. While Go's garbage collector will eventually handle this, it's best practice to explicitly stop timers that are no longer needed. Add timer.Stop() after the select statement or use defer timer.Stop() right after creating the timer.

Copilot · 2026-02-25T15:01:44Z

gh/main.go

+		timer := time.NewTimer(WaitTimeBetweenRetries)
+		select {
+		case <-timer.C:
+		case <-ctx.Done():
+			timer.Stop()
+			return "", ctx.Err()
+		}


The timer should be stopped after the select statement completes to free resources, regardless of which case is triggered. Currently, the timer is only stopped when the context is cancelled. If the timer fires normally (case <-timer.C), the timer resources are not explicitly cleaned up. While Go's garbage collector will eventually handle this, it's best practice to explicitly stop timers that are no longer needed. Add timer.Stop() after the select statement or use defer timer.Stop() right after creating the timer.

Copilot · 2026-02-25T15:01:44Z

gh/main.go

+			break
+		}
+
+		if i == MaxRetries-1 {


When extractErrorMessage is called at line 401, if the error that caused the retry loop to fail was actually convErr (the strconv.Atoi error) and not err, then the wrong error message will be extracted. The code should capture and handle both error conditions. If convErr is not nil but err is nil, the error message should reflect the conversion failure rather than calling extractErrorMessage on a potentially nil err.

Suggested change

if i == MaxRetries-1 {

if i == MaxRetries-1 {

if err == nil && convErr != nil {

return "", convErr

}

juanjosevazquezgil and others added 17 commits February 20, 2026 14:27

Merge branch 'main' of https://github.com/prefapp/daggerverse

0b5863f

Merge branch 'main' of https://github.com/prefapp/daggerverse

7b75829

test: Add sleeps to test error

3c1914c

fix: Updated gh dependency

1fa5ac5

fix: Initial version of retry loops

0de040c

Merge branch 'main' into fix/413-fix-error-on-pr-creation

3ef569e

fix: Updated gh dependency

c8b0e91

Merge branch 'fix/413-fix-error-on-pr-creation' of https://github.com…

3a2a99d

…/prefapp/daggerverse into fix/413-fix-error-on-pr-creation

fix: Added sleep when PR creation and checking fails

4d983b8

fix: Logs

56c2832

fix: Updated gh dependency

bdf4fca

test: Fail case

3ca4e9e

fix: Updated gh dependency

bbbad60

test

50154f4

fix: Update gh dependency

7b846b4

fix: Restored functionality

e3b5e87

fix: Updated gh dependency

3712723

juanjosevazquezgil requested a review from Copilot February 25, 2026 13:17

Copilot started reviewing on behalf of juanjosevazquezgil February 25, 2026 13:17 View session

Copilot AI reviewed Feb 25, 2026

View reviewed changes

gh/main.go Show resolved Hide resolved

gh/main.go Show resolved Hide resolved

gh/main.go Outdated Show resolved Hide resolved

fix: Unneeded sleep

8d9a486

juanjosevazquezgil requested a review from Copilot February 25, 2026 13:22

Copilot started reviewing on behalf of juanjosevazquezgil February 25, 2026 13:22 View session

Copilot AI reviewed Feb 25, 2026

View reviewed changes

gh/main.go Outdated Show resolved Hide resolved

fix: Inverted for loops conditions

d1407a8

juanjosevazquezgil requested a review from Copilot February 25, 2026 13:38

Copilot started reviewing on behalf of juanjosevazquezgil February 25, 2026 13:39 View session

Copilot AI reviewed Feb 25, 2026

View reviewed changes

gh/main.go Outdated Show resolved Hide resolved

gh/main.go Outdated Show resolved Hide resolved

gh/main.go Outdated Show resolved Hide resolved

juanjosevazquezgil added 2 commits February 25, 2026 14:48

fix: Removed magical numbers

81a94b4

fix: Improved timer

42c9136

fix: Check PR ID is actually an ID

b13f451

juanjosevazquezgil requested a review from Copilot February 25, 2026 14:56

Copilot started reviewing on behalf of juanjosevazquezgil February 25, 2026 14:57 View session

Copilot AI reviewed Feb 25, 2026

View reviewed changes

Conversation

juanjosevazquezgil commented Feb 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants