Fix update check context handling

Author: kindermax

.golangci.yaml

@@ -5,6 +5,7 @@ run:
 linters:
   default: all
   disable:
+    - goconst
     - containedctx
     - copyloopvar
     - cyclop

internal/cli/cli.go

@@ -104,9 +104,9 @@ func Main(version string, buildDate string) int {
 	}
 
 	if rootFlags.upgrade {
-		upgrader, err := upgrade.NewBinaryUpgrader(registry.NewGithubRegistry(ctx), version)
+		upgrader, err := upgrade.NewBinaryUpgrader(registry.NewGithubRegistry(), version)
 		if err == nil {
-			err = upgrader.Upgrade()
+			err = upgrader.Upgrade(ctx)
 		}
 
 		if err != nil {
@@ -138,6 +138,7 @@ func Main(version string, buildDate string) int {
 		}
 
 		log.Errorf("lets: %s", err.Error())
+
 		return getExitCode(err, 1)
 	}
 
@@ -212,7 +213,7 @@ func maybeStartUpdateCheck(
 
 	log.Debugf("lets: start update check")
 
-	notifier, err := upgrade.NewUpdateNotifier(registry.NewGithubRegistry(ctx))
+	notifier, err := upgrade.NewUpdateNotifier(registry.NewGithubRegistry())
 	if err != nil {
 		return nil, func() {}
 	}
@@ -225,6 +226,7 @@ func maybeStartUpdateCheck(
 		if err != nil {
 			upgrade.LogUpdateCheckError(err)
 		}
+
 		log.Debugf("lets: update check done")
 
 		ch <- updateCheckResult{

internal/upgrade/notifier.go

@@ -152,6 +152,7 @@ func (n *UpdateNotifier) noticeFromState(
 	}
 
 	command := "lets self upgrade"
+
 	if isHomebrewInstall(n.executablePath) {
 		if !state.LatestPublishedAt.IsZero() && now.Sub(state.LatestPublishedAt) < homebrewNoticeDelay {
 			return nil

internal/upgrade/notifier_test.go

@@ -19,7 +19,7 @@ func (m *mockNotifierRegistry) GetLatestReleaseInfo(ctx context.Context) (*regis
 	return m.release, nil
 }
 
-func (m *mockNotifierRegistry) GetLatestRelease() (string, error) {
+func (m *mockNotifierRegistry) GetLatestRelease(ctx context.Context) (string, error) {
 	m.calls++
 	if m.release == nil {
 		return "", nil
@@ -28,7 +28,7 @@ func (m *mockNotifierRegistry) GetLatestRelease() (string, error) {
 	return m.release.TagName, nil
 }
 
-func (m *mockNotifierRegistry) DownloadReleaseBinary(packageName string, version string, dstPath string) error {
+func (m *mockNotifierRegistry) DownloadReleaseBinary(ctx context.Context, packageName string, version string, dstPath string) error {
 	return nil
 }
 

internal/upgrade/registry/registry.go

@@ -26,30 +26,28 @@ var osMap = map[string]string{
 
 type RepoRegistry interface {
 	GetLatestReleaseInfo(ctx context.Context) (*ReleaseInfo, error)
-	GetLatestRelease() (string, error)
-	DownloadReleaseBinary(packageName string, version string, dstPath string) error
+	GetLatestRelease(ctx context.Context) (string, error)
+	DownloadReleaseBinary(ctx context.Context, packageName string, version string, dstPath string) error
 	GetPackageName(os string, arch string) (string, error)
 	GetDownloadURL(repoURI string, packageName string, version string) string
 }
 
 type GithubRegistry struct {
 	client                 *http.Client
-	ctx                    context.Context
 	repoURI                string
 	apiURI                 string
 	downloadURL            string
 	downloadPackageTimeout time.Duration
 	latestReleaseTimeout   time.Duration
 }
 
-func NewGithubRegistry(ctx context.Context) *GithubRegistry {
+func NewGithubRegistry() *GithubRegistry {
 	client := &http.Client{
 		Timeout: 15 * 60 * time.Second, // global timeout
 	}
 
 	reg := &GithubRegistry{
 		client:                 client,
-		ctx:                    ctx,
 		repoURI:                "https://github.com/lets-cli/lets",
 		apiURI:                 "https://api.github.com/repos/lets-cli/lets",
 		downloadURL:            "",
@@ -76,13 +74,14 @@ func (reg *GithubRegistry) GetPackageName(os string, arch string) (string, error
 }
 
 func (reg *GithubRegistry) DownloadReleaseBinary(
+	ctx context.Context,
 	packageName string,
 	version string,
 	dstPath string,
 ) error {
 	downloadURL := reg.GetDownloadURL(reg.repoURI, packageName+".tar.gz", version)
 
-	ctx, cancel := context.WithTimeout(reg.ctx, reg.downloadPackageTimeout)
+	ctx, cancel := context.WithTimeout(ctx, reg.downloadPackageTimeout)
 	defer cancel()
 
 	req, err := http.NewRequestWithContext(
@@ -128,7 +127,7 @@ func (reg *GithubRegistry) DownloadReleaseBinary(
 
 	// TODO add download progress bar
 	// TODO drop extract dependency, replace with own code
-	err = extract.Gz(reg.ctx, resp.Body, dstDir, nil)
+	err = extract.Gz(ctx, resp.Body, dstDir, nil)
 	if err != nil {
 		return fmt.Errorf("failed to extract package: %w", err)
 	}
@@ -147,8 +146,8 @@ type ReleaseInfo struct {
 	PublishedAt time.Time `json:"published_at"`
 }
 
-func (reg *GithubRegistry) GetLatestRelease() (string, error) {
-	release, err := reg.GetLatestReleaseInfo(reg.ctx)
+func (reg *GithubRegistry) GetLatestRelease(ctx context.Context) (string, error) {
+	release, err := reg.GetLatestReleaseInfo(ctx)
 	if err != nil {
 		return "", err
 	}
@@ -157,12 +156,7 @@ func (reg *GithubRegistry) GetLatestRelease() (string, error) {
 }
 
 func (reg *GithubRegistry) GetLatestReleaseInfo(ctx context.Context) (*ReleaseInfo, error) {
-	requestCtx := reg.ctx
-	if ctx != nil {
-		requestCtx = ctx
-	}
-
-	requestCtx, cancel := context.WithTimeout(requestCtx, reg.latestReleaseTimeout)
+	requestCtx, cancel := context.WithTimeout(ctx, reg.latestReleaseTimeout)
 	defer cancel()
 
 	url := reg.apiURI + "/releases/latest"

internal/upgrade/registry/registry_test.go

@@ -21,7 +21,7 @@ func TestGithubRegistryGetLatestReleaseInfo(t *testing.T) {
 	}))
 	defer server.Close()
 
-	reg := NewGithubRegistry(context.Background())
+	reg := NewGithubRegistry()
 	reg.apiURI = server.URL
 
 	release, err := reg.GetLatestReleaseInfo(context.Background())
@@ -43,10 +43,10 @@ func TestGithubRegistryGetLatestRelease(t *testing.T) {
 	}))
 	defer server.Close()
 
-	reg := NewGithubRegistry(context.Background())
+	reg := NewGithubRegistry()
 	reg.apiURI = server.URL
 
-	version, err := reg.GetLatestRelease()
+	version, err := reg.GetLatestRelease(context.Background())
 	if err != nil {
 		t.Fatalf("GetLatestRelease() error = %v", err)
 	}

internal/upgrade/upgrade.go

@@ -1,6 +1,7 @@
 package upgrade
 
 import (
+	"context"
 	"fmt"
 	"io"
 	"os"
@@ -12,7 +13,7 @@ import (
 )
 
 type Upgrader interface {
-	Upgrade() error
+	Upgrade(ctx context.Context) error
 }
 
 type BinaryUpgrader struct {
@@ -39,8 +40,8 @@ func NewBinaryUpgrader(reg registry.RepoRegistry, currentVersion string) (*Binar
 	}, nil
 }
 
-func (up *BinaryUpgrader) Upgrade() error {
-	latestVersion, err := up.registry.GetLatestRelease()
+func (up *BinaryUpgrader) Upgrade(ctx context.Context) error {
+	latestVersion, err := up.registry.GetLatestRelease(ctx)
 	if err != nil {
 		return fmt.Errorf("failed to get latest release version: %w", err)
 	}
@@ -59,6 +60,7 @@ func (up *BinaryUpgrader) Upgrade() error {
 	log.Printf("Downloading latest release %s...", latestVersion)
 
 	err = up.registry.DownloadReleaseBinary(
+		ctx,
 		packageName,
 		latestVersion,
 		up.downloadPath,

internal/upgrade/upgrade_test.go

@@ -15,21 +15,21 @@ type MockRegistry struct {
 	latestVersion string
 }
 
-func (m MockRegistry) GetLatestRelease() (string, error) {
+func (m MockRegistry) GetLatestRelease(ctx context.Context) (string, error) {
 	return m.latestVersion, nil
 }
 
 func (m MockRegistry) GetLatestReleaseInfo(ctx context.Context) (*registry.ReleaseInfo, error) {
 	return &registry.ReleaseInfo{TagName: m.latestVersion}, nil
 }
 
-func (m MockRegistry) DownloadReleaseBinary(packageName string, version string, dstPath string) error {
+func (m MockRegistry) DownloadReleaseBinary(ctx context.Context, packageName string, version string, dstPath string) error {
 	file, err := os.Create(dstPath)
 	if err != nil {
 		return err
 	}
 
-	latest, _ := m.GetLatestRelease()
+	latest, _ := m.GetLatestRelease(ctx)
 
 	_, err = fmt.Fprint(file, latest)
 	if err != nil {
@@ -101,7 +101,7 @@ func TestSelfUpgrade(t *testing.T) {
 			t.Errorf("expected version %s", currentVersion)
 		}
 
-		err = upgrader.Upgrade()
+		err = upgrader.Upgrade(context.Background())
 		if err != nil {
 			t.Errorf("failed to upgrade: %s", err)
 		}
@@ -129,7 +129,7 @@ func TestSelfUpgrade(t *testing.T) {
 		// sleep to be sure files not created at the same time
 		time.Sleep(10 * time.Millisecond)
 
-		err = upgrader.Upgrade()
+		err = upgrader.Upgrade(context.Background())
 		if err != nil {
 			t.Errorf("failed to upgrade: %s", err)
 		}