Add new clog.ErrGroup

coder · cmoog · Oct 27, 2020 · Oct 23, 2020 · Oct 26, 2020 · 6c2e59b44eb25e87ac05becd6f646aa61aadaef2
commit 6c2e59b44eb25e87ac05becd6f646aa61aadaef2
diff --git a/internal/clog/doc.go b/internal/clog/doc.go
@@ -0,0 +1,7 @@
+// package clog provides rich error types and logging helpers for coder-cli.
+//
+// clog encourages returning error types rather than
+// logging them and failing with os.Exit as they happen.
+// Error, Fatal, and Warn allow downstream functions to return errors with rich formatting information
+// while preserving the original, single-line error chain.
+package clog
diff --git a/internal/clog/errgroup.go b/internal/clog/errgroup.go
@@ -0,0 +1,58 @@
+package clog
+
+import (
+	"fmt"
+	"sync/atomic"
+
+	"golang.org/x/sync/errgroup"
+	"golang.org/x/xerrors"
+)
+
+// ErrGroup wraps the /x/sync/errgroup.(Group) and adds clog logging and rich error propagation.
+//
+// Take for example, a case in which we are concurrently stopping a slice of environments.
+// In this case, we want to log errors as they happen, not pass them through the callstack as errors.
+// When the operations complete, we want to log how many, if any, failed. The caller is still expected
+// to handle success and info logging.
+type ErrGroup interface {
+	Go(f func() error)
+	Wait() error
+}
+
+type group struct {
+	egroup   errgroup.Group
+	failures int32
+}
+
+// LoggedErrGroup gives an error group with error logging and error propagation handled automatically.
+func LoggedErrGroup() ErrGroup {
+	return &group{
+		egroup:   errgroup.Group{},
+		failures: 0,
+	}
+}
+
+func (g *group) Go(f func() error) {
+	g.egroup.Go(func() error {
+		if err := f(); err != nil {
+			atomic.AddInt32(&g.failures, 1)
+			Log(err)
+
+			// this error does not matter because we discard it in Wait.
+			return xerrors.New("")
+		}
+		return nil
+	})
+}
+
+func (g *group) Wait() error {
+	_ = g.egroup.Wait() // ignore this error because we are already tracking failures manually
+	if g.failures == 0 {
+		return nil
+	}
+	failureWord := "failure"
+	if g.failures > 1 {
+		failureWord += "s"
+	}
+	return Fatal(fmt.Sprintf("%d %s emitted", g.failures, failureWord))
+}
diff --git a/internal/cmd/envs.go b/internal/cmd/envs.go
@@ -4,14 +4,12 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
-	"sync/atomic"
 
 	"cdr.dev/coder-cli/coder-sdk"
 	"cdr.dev/coder-cli/internal/clog"
 	"cdr.dev/coder-cli/internal/x/xtabwriter"
 	"github.com/manifoldco/promptui"
 	"github.com/spf13/cobra"
-	"golang.org/x/sync/errgroup"
 	"golang.org/x/xerrors"
 )
 
@@ -113,36 +111,27 @@ coder envs --user charlie@coder.com ls -o json \
 				return xerrors.Errorf("new client: %w", err)
 			}
 
-			var egroup errgroup.Group
-			var fails int32
+			egroup := clog.LoggedErrGroup()
 			for _, envName := range args {
 				envName := envName
 				egroup.Go(func() error {
 					env, err := findEnv(cmd.Context(), client, envName, *user)
 					if err != nil {
-						atomic.AddInt32(&fails, 1)
-						clog.Log(err)
-						return xerrors.Errorf("find env by name: %w", err)
+						return err
 					}
 
 					if err = client.StopEnvironment(cmd.Context(), env.ID); err != nil {
-						atomic.AddInt32(&fails, 1)
-						err = clog.Fatal(fmt.Sprintf("stop environment %q", env.Name),
+						return clog.Error(fmt.Sprintf("stop environment %q", env.Name),
 							clog.Causef(err.Error()), clog.BlankLine,
 							clog.Hintf("current environment status is %q", env.LatestStat.ContainerStatus),
 						)
-						clog.Log(err)
-						return err
 					}
 					clog.LogSuccess(fmt.Sprintf("successfully stopped environment %q", envName))
 					return nil
 				})
 			}
 
-			if err = egroup.Wait(); err != nil {
-				return clog.Fatal(fmt.Sprintf("%d failure(s) emitted", fails))
-			}
-			return nil
+			return egroup.Wait()
 		},
 	}
 }
@@ -353,35 +342,25 @@ func rmEnvsCommand(user *string) *cobra.Command {
 				}
 			}
 
-			var egroup errgroup.Group
-			var failures int32
+			egroup := clog.LoggedErrGroup()
 			for _, envName := range args {
 				envName := envName
 				egroup.Go(func() error {
 					env, err := findEnv(ctx, client, envName, *user)
 					if err != nil {
-						atomic.AddInt32(&failures, 1)
-						clog.Log(err)
 						return err
 					}
 					if err = client.DeleteEnvironment(cmd.Context(), env.ID); err != nil {
-						atomic.AddInt32(&failures, 1)
-						err = clog.Error(
+						return clog.Error(
 							fmt.Sprintf(`failed to delete environment "%s"`, env.Name),
 							clog.Causef(err.Error()),
 						)
-						clog.Log(err)
-						return err
 					}
 					clog.LogSuccess(fmt.Sprintf("deleted environment %q", env.Name))
 					return nil
 				})
 			}
-
-			if err = egroup.Wait(); err != nil {
-				return xerrors.Errorf("%d failure(s) emitted", failures)
-			}
-			return nil
+			return egroup.Wait()
 		},
 	}
 	cmd.Flags().BoolVarP(&force, "force", "f", false, "force remove the specified environments without prompting first")