fix: download with retry, dont hide error of loaded plugin

Author: 23doors

cmd/executor.go

@@ -277,8 +277,13 @@ func (e *Executor) addPluginsCommands() error {
 
 	for _, plug := range e.cfg.Plugins {
 		plug := plug
+		if plug == nil || plug.Loaded() == nil {
+			continue
+		}
+
+		loaded := plug.Loaded()
 
-		for cmdName, cmdt := range plug.Loaded().Commands {
+		for cmdName, cmdt := range loaded.Commands {
 			cmdName := cmdName
 			cmdt := cmdt
 
@@ -287,7 +292,7 @@ func (e *Executor) addPluginsCommands() error {
 			cmd, _, err := e.rootCmd.Find([]string{cmdName})
 			if err != nil {
 				cmd = &cobra.Command{
-					Use:          fmt.Sprintf("%s-%s", plug.Loaded().ShortName(), cmdName),
+					Use:          fmt.Sprintf("%s-%s", loaded.ShortName(), cmdName),
 					Short:        cmdt.Short,
 					Long:         cmdt.Long,
 					SilenceUsage: true,
@@ -301,7 +306,7 @@ func (e *Executor) addPluginsCommands() error {
 						return actions.NewCommand(e.log, e.cfg, &actions.CommandOptions{
 							Name:       cmdName,
 							InputTypes: cmdt.InputTypes(),
-							Plugin:     plug.Loaded(),
+							Plugin:     loaded,
 							Args:       cmdt.Proto(args),
 						}).Run(cmd.Context())
 					},

cmd/executor_test.go

@@ -0,0 +1,18 @@
+package cmd
+
+import (
+	"testing"
+
+	"github.com/outblocks/outblocks-cli/pkg/config"
+)
+
+func TestAddPluginsCommandsSkipsUnloadedPlugins(t *testing.T) {
+	e := NewExecutor()
+	e.cfg = &config.Project{
+		Plugins: []*config.Plugin{{}},
+	}
+
+	if err := e.addPluginsCommands(); err != nil {
+		t.Fatalf("addPluginsCommands returned error: %v", err)
+	}
+}

pkg/config/project.go

@@ -6,6 +6,7 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"time"
 
 	"github.com/23doors/go-yaml"
 	"github.com/23doors/go-yaml/ast"
@@ -52,6 +53,11 @@ const (
 	LoadModeSkip
 )
 
+const (
+	pluginDownloadMaxAttempts = 3
+	pluginDownloadRetryDelay  = time.Second
+)
+
 type LoadAppsOptions struct {
 	Targets *util.TargetMatcher
 	Skips   *util.TargetMatcher
@@ -511,17 +517,20 @@ func (p *Project) LoadPlugins(ctx context.Context, log logger.Logger, loader *pl
 
 			prog.UpdateTitle(title)
 
-			plugin, err := loader.DownloadPlugin(ctx, plug.Name, plug.VerConstr(), plug.Source, p.PluginLock(plug))
-			plugs[i] = plugin
-
-			plug.SetLoaded(plugin)
+			plugin, err := downloadPluginWithRetry(ctx, pluginDownloadMaxAttempts, pluginDownloadRetryDelay, func() (*plugins.Plugin, error) {
+				return loader.DownloadPlugin(ctx, plug.Name, plug.VerConstr(), plug.Source, p.PluginLock(plug))
+			})
 
 			if err != nil {
 				prog.Stop()
 
 				return merry.Errorf("unable to load '%s' plugin: %w", plug.Name, err)
 			}
 
+			plugs[i] = plugin
+
+			plug.SetLoaded(plugin)
+
 			prog.Increment()
 			pterm.Success.Printf("Downloaded plugin '%s' at version: %s\n", plug.Name, plugin.Version)
 		}
@@ -549,6 +558,45 @@ func (p *Project) LoadPlugins(ctx context.Context, log logger.Logger, loader *pl
 	return nil
 }
 
+func downloadPluginWithRetry(ctx context.Context, maxAttempts int, retryDelay time.Duration, download func() (*plugins.Plugin, error)) (*plugins.Plugin, error) {
+	if maxAttempts < 1 {
+		maxAttempts = 1
+	}
+
+	var lastErr error
+
+	for attempt := 1; attempt <= maxAttempts; attempt++ {
+		plugin, err := download()
+		if err == nil {
+			return plugin, nil
+		}
+
+		lastErr = err
+
+		if attempt == maxAttempts {
+			break
+		}
+
+		if retryDelay <= 0 {
+			continue
+		}
+
+		timer := time.NewTimer(time.Duration(attempt) * retryDelay)
+
+		select {
+		case <-ctx.Done():
+			if !timer.Stop() {
+				<-timer.C
+			}
+
+			return nil, ctx.Err()
+		case <-timer.C:
+		}
+	}
+
+	return nil, lastErr
+}
+
 func (p *Project) DomainInfoProto() []*apiv1.DomainInfo {
 	var ret []*apiv1.DomainInfo
 

pkg/config/project_retry_test.go

@@ -0,0 +1,57 @@
+package config
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/outblocks/outblocks-cli/pkg/plugins"
+)
+
+func TestDownloadPluginWithRetryRetriesAndSucceeds(t *testing.T) {
+	t.Parallel()
+
+	attempts := 0
+	want := &plugins.Plugin{}
+
+	got, err := downloadPluginWithRetry(context.Background(), 3, 0, func() (*plugins.Plugin, error) {
+		attempts++
+
+		if attempts < 3 {
+			return nil, errors.New("temporary")
+		}
+
+		return want, nil
+	})
+	if err != nil {
+		t.Fatalf("downloadPluginWithRetry returned error: %v", err)
+	}
+
+	if got != want {
+		t.Fatalf("unexpected plugin pointer: got %p want %p", got, want)
+	}
+
+	if attempts != 3 {
+		t.Fatalf("unexpected attempts count: got %d want %d", attempts, 3)
+	}
+}
+
+func TestDownloadPluginWithRetryReturnsLastError(t *testing.T) {
+	t.Parallel()
+
+	wantErr := errors.New("still failing")
+	attempts := 0
+
+	_, err := downloadPluginWithRetry(context.Background(), 3, 0, func() (*plugins.Plugin, error) {
+		attempts++
+
+		return nil, wantErr
+	})
+	if !errors.Is(err, wantErr) {
+		t.Fatalf("unexpected error: got %v want %v", err, wantErr)
+	}
+
+	if attempts != 3 {
+		t.Fatalf("unexpected attempts count: got %d want %d", attempts, 3)
+	}
+}