Application that interacts with a remote GitHub repository with retries

Question

I came up with the logic for an app that talks to a remote GitHub repository using below. I tried to stick to Go best practices and approaches, but would like to identify bottlenecks and adopt better Go practices wherever applicable

package main
import (
 "context"
 "fmt"
 "log"
 "net/http"
 "strings"
 "time"
 "github.com/google/go-github/v61/github"
 "github.com/sethvargo/go-retry"
)
type APIResult struct {
 Data any
 Response *github.Response
 Error error
}
type GitHubAPIFunc func(ctx context.Context) APIResult
func retryWithExpBackoff(ctx context.Context, githubAPIRequest GitHubAPIFunc) APIResult {
 var result APIResult
 b := retry.NewExponential(1 * time.Second)
 b = retry.WithMaxRetries(5, b)
 b = retry.WithMaxDuration(5*time.Second, b)
 err := retry.Do(ctx, b, func(ctx context.Context) error {
 result = githubAPIRequest(ctx)
 if retryErr := retryDecision(result.Response); retryErr != nil {
 return retryErr
 }
 return result.Error
 })
 result.Error = err
 return result
}
func GetRepository(ctx context.Context, client *github.Client, owner, repo string) (*github.Repository, *github.Response, error) {
 githubAPIRequest := func(ctx context.Context) APIResult {
 repo, resp, err := client.Repositories.Get(ctx, owner, repo)
 return APIResult{Data: repo, Response: resp, Error: err}
 }
 result := retryWithExpBackoff(ctx, githubAPIRequest)
 if result.Error != nil {
 return nil, nil, result.Error
 }
 return result.Data.(*github.Repository), result.Response, nil
}
func retryDecision(resp *github.Response) error {
 // Check the response code. We retry on 500-range responses to allow
 // the server time to recover, as 500's are typically not permanent
 // errors and may relate to outages on the server side. This will catch
 // invalid response codes as well, like 0 and 999.
 if resp.StatusCode == 0 || (resp.StatusCode >= 500 && resp.StatusCode != http.StatusNotImplemented) {
 return retry.RetryableError(fmt.Errorf("unexpected HTTP status %s", resp.Status))
 }
 return nil
}
func CheckGitTag(ctx context.Context, client *github.Client, repo, tag string) error {
 r := strings.Split(repo, "/")
 request := func(ctx context.Context) APIResult {
 ref, resp, err := client.Git.GetRef(ctx, r[0], r[1], fmt.Sprintf("refs/tags/%s", tag))
 return APIResult{Data: ref, Response: resp, Error: err}
 }
 result := retryWithExpBackoff(ctx, request)
 if result.Error != nil {
 return result.Error
 }
 fmt.Printf("result=%v\n", result)
 return nil
}
// NewGitHubClient returns an instance of a GitHub client to interact with a
// remote repository with an OAuth token as an argument.
func NewGitHubClient(ctx context.Context, token, repo string) (*github.Client, error) {
 r := strings.Split(repo, "/")
 client := github.NewClient(nil).WithAuthToken(token)
 request := func(ctx context.Context) APIResult {
 repo, resp, err := client.Repositories.Get(ctx, r[0], r[1])
 return APIResult{Data: repo, Response: resp, Error: err}
 }
 result := retryWithExpBackoff(ctx, request)
 if result.Error != nil {
 return nil, result.Error
 }
 fmt.Printf("result=%v\n", result)
 return client, nil
}
func CommitGitTag(ctx context.Context, client *github.Client, repo, baseSHA, tag string) error {
 r := strings.Split(repo, "/")
 ghTag := &github.Tag{
 Tag: github.String(tag),
 Message: github.String("initial version"),
 Object: &github.GitObject{
 Type: github.String("commit"),
 SHA: github.String(baseSHA),
 },
 }
 request := func(ctx context.Context) APIResult {
 tagInfo, resp, err := client.Git.CreateTag(ctx, r[0], r[1], ghTag)
 return APIResult{Data: tagInfo, Response: resp, Error: err}
 }
 result := retryWithExpBackoff(ctx, request)
 if result.Error != nil {
 return result.Error
 }
 tagCreated, ok := result.Data.(*github.Tag)
 if !ok {
 return fmt.Errorf("unable to get github.Tag from API response")
 }
 // Create a reference to the tag
 ref := &github.Reference{
 Ref: github.String(fmt.Sprintf("refs/tags/%s", tag)),
 Object: &github.GitObject{
 SHA: tagCreated.SHA,
 },
 }
 request = func(ctx context.Context) APIResult {
 _, resp, err := client.Git.CreateRef(ctx, r[0], r[1], ref)
 return APIResult{Response: resp, Error: err}
 }
 result = retryWithExpBackoff(ctx, request)
 if result.Error != nil {
 return result.Error
 }
 return nil
}
func main() {
 ctx := context.Background()
 repo := "randomorg/randomrepo"
 sha := "somrandombaseshaofsomevaryinglength"
 tag := "v0.0.1"
 client, err := NewGitHubClient(ctx, "somerandomauthtokenthatisofnousehere", repo)
 if err != nil {
 log.Fatal(err)
 }
 if err := CheckGitTag(ctx, client, repo, tag); err == nil {
 log.Fatal(fmt.Errorf("tag %s already references an exiting commit in %s", tag, repo))
 }
 if err := CommitGitTag(ctx, client, repo, sha, tag); err != nil {
 log.Fatal(err)
 }
}

Answer 1

Retrying

Instead of implementing your own retries, you could implement retries at the client level.

Github.NewClient takes in a optional http.Client argument. You could either implement your own http.Client that does retry or you could use something like go-retryablehttp to do it for you.

retryableClient := retryablehttp.NewClient()
ghclient := github.NewClient(retryableClient.StandardClient())

The benefit of this is that you're pushing the retry logic down the stack, and your application code itself does not need to care or implement how retries are being done.

This can also help remove the APIResult struct, which I presume is created for the sole purpose of implement retries.

General comments

• Be consistent with naming. Some of your functions start with lowercase alphabets, while others start with uppercase. Since you're not exporting any of these functions, you should just use lowercase for all of them.

• Be careful of bad input.

  r := strings.Split(repo, "/")
  r[0], r[1]
  

  will panic if someone passes in a repo which does not contain /.