coder · Emyrk · Jan 11, 2024 · Jan 11, 2024 · Jan 11, 2024 · Jan 11, 2024
diff --git a/cli/errors.go b/cli/errors.go
@@ -1,6 +1,7 @@
 package cli
 
 import (
+	"errors"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -43,6 +44,10 @@ func (RootCmd) errorExample() *clibase.Cmd {
 	//nolint:errorlint,forcetypeassert
 	apiError.(*codersdk.Error).Helper = "Have you tried turning it off and on again?"
 
+	//nolint:errorlint,forcetypeassert
+	apiErrorNoHelper := apiError.(*codersdk.Error)
+	apiErrorNoHelper.Helper = ""
+
 	// Some flags
 	var magicWord clibase.String
 
@@ -65,14 +70,28 @@ func (RootCmd) errorExample() *clibase.Cmd {
 			// A multi-error
 			{
 				Use: "multi-error",
+				Handler: func(inv *clibase.Invocation) error {
+					return xerrors.Errorf("wrapped: %w", errors.Join(
+						xerrors.Errorf("first error: %w", errorWithStackTrace()),
+						xerrors.Errorf("second error: %w", errorWithStackTrace()),
+						xerrors.Errorf("wrapped api error: %w", apiErrorNoHelper),
+					))
+				},
+			},
+			{
+				Use:   "multi-multi-error",
+				Short: "This is a multi error inside a multi error",
 				Handler: func(inv *clibase.Invocation) error {
 					// Closing the stdin file descriptor will cause the next close
 					// to fail. This is joined to the returned Command error.
 					if f, ok := inv.Stdin.(*os.File); ok {
 						_ = f.Close()
 					}
 
-					return xerrors.Errorf("some error: %w", errorWithStackTrace())
+					return errors.Join(
+						xerrors.Errorf("first error: %w", errorWithStackTrace()),
+						xerrors.Errorf("second error: %w", errorWithStackTrace()),
+					)
 				},
 			},
 

diff --git a/cli/root.go b/cli/root.go
@@ -1016,7 +1016,7 @@ type prettyErrorFormatter struct {
 // format formats the error to the console. This error should be human
 // readable.
 func (p *prettyErrorFormatter) format(err error) {
-	output := cliHumanFormatError(err, &formatOpts{
+	output, _ := cliHumanFormatError("", err, &formatOpts{
 		Verbose: p.verbose,
 	})
 	// always trail with a newline
@@ -1030,41 +1030,66 @@ type formatOpts struct {
 const indent = "    "
 
 // cliHumanFormatError formats an error for the CLI. Newlines and styling are
-// included.
-func cliHumanFormatError(err error, opts *formatOpts) string {
+// included. The second return value is true if the error is special and the error
+// chain has custom formatting applied.
+//
+// If you change this code, you can use the cli "example-errors" tool to
+// verify all errors still look ok.
+//
+//	go run main.go exp example-error <type>
+//	       go run main.go exp example-error api
+//	       go run main.go exp example-error cmd
+//	       go run main.go exp example-error multi-error
+//	       go run main.go exp example-error validation
+//
+//nolint:errorlint
+func cliHumanFormatError(from string, err error, opts *formatOpts) (string, bool) {
 	if opts == nil {
 		opts = &formatOpts{}
 	}
+	if err == nil {
+		return "<nil>", true
+	}
 
-	//nolint:errorlint
 	if multi, ok := err.(interface{ Unwrap() []error }); ok {
 		multiErrors := multi.Unwrap()
 		if len(multiErrors) == 1 {
 			// Format as a single error
-			return cliHumanFormatError(multiErrors[0], opts)
+			return cliHumanFormatError(from, multiErrors[0], opts)
 		}
-		return formatMultiError(multiErrors, opts)
+		return formatMultiError(from, multiErrors, opts), true
 	}
 
 	// First check for sentinel errors that we want to handle specially.
 	// Order does matter! We want to check for the most specific errors first.
-	var sdkError *codersdk.Error
-	if errors.As(err, &sdkError) {
-		return formatCoderSDKError(sdkError, opts)
+	if sdkError, ok := err.(*codersdk.Error); ok {
+		return formatCoderSDKError(from, sdkError, opts), true
 	}
 
-	var cmdErr *clibase.RunCommandError
-	if errors.As(err, &cmdErr) {
-		return formatRunCommandError(cmdErr, opts)
+	if cmdErr, ok := err.(*clibase.RunCommandError); ok {
+		// no need to pass the "from" context to this since it is always
+		// top level. We care about what is below this.
+		return formatRunCommandError(cmdErr, opts), true
 	}
 
+	uw, ok := err.(interface{ Unwrap() error })
+	if ok {
+		msg, special := cliHumanFormatError(from+traceError(err), uw.Unwrap(), opts)
+		if special {
+			return msg, special
+		}
+	}
+	// If we got here, that means that the wrapped error chain does not have
+	// any special formatting below it. So we want to return the topmost non-special
+	// error (which is 'err')
+
 	// Default just printing the error. Use +v for verbose to handle stack
 	// traces of xerrors.
 	if opts.Verbose {
-		return pretty.Sprint(headLineStyle(), fmt.Sprintf("%+v", err))
+		return pretty.Sprint(headLineStyle(), fmt.Sprintf("%+v", err)), false
 	}
 
-	return pretty.Sprint(headLineStyle(), fmt.Sprintf("%v", err))
+	return pretty.Sprint(headLineStyle(), fmt.Sprintf("%v", err)), false
 }
 
 // formatMultiError formats a multi-error. It formats it as a list of errors.
@@ -1075,15 +1100,20 @@ func cliHumanFormatError(err error, opts *formatOpts) string {
 //		   <verbose error message>
 //		2. <heading error message>
 //		   <verbose error message>
-func formatMultiError(multi []error, opts *formatOpts) string {
+func formatMultiError(from string, multi []error, opts *formatOpts) string {
 	var errorStrings []string
 	for _, err := range multi {
-		errorStrings = append(errorStrings, cliHumanFormatError(err, opts))
+		msg, _ := cliHumanFormatError("", err, opts)
+		errorStrings = append(errorStrings, msg)
 	}
 
 	// Write errors out
 	var str strings.Builder
-	_, _ = str.WriteString(pretty.Sprint(headLineStyle(), fmt.Sprintf("%d errors encountered:", len(multi))))
+	var traceMsg string
+	if from != "" {
+		traceMsg = fmt.Sprintf("Trace=[%s])", from)
+	}
+	_, _ = str.WriteString(pretty.Sprint(headLineStyle(), fmt.Sprintf("%d errors encountered: %s", len(multi), traceMsg)))
 	for i, errStr := range errorStrings {
 		// Indent each error
 		errStr = strings.ReplaceAll(errStr, "\n", "\n"+indent)
@@ -1112,24 +1142,30 @@ func formatRunCommandError(err *clibase.RunCommandError, opts *formatOpts) strin
 	var str strings.Builder
 	_, _ = str.WriteString(pretty.Sprint(headLineStyle(), fmt.Sprintf("Encountered an error running %q", err.Cmd.FullName())))
 
-	msgString := fmt.Sprintf("%v", err.Err)
-	if opts.Verbose {
-		// '%+v' includes stack traces
-		msgString = fmt.Sprintf("%+v", err.Err)
-	}
+	msgString, special := cliHumanFormatError("", err.Err, opts)
 	_, _ = str.WriteString("\n")
-	_, _ = str.WriteString(pretty.Sprint(tailLineStyle(), msgString))
+	if special {
+		_, _ = str.WriteString(msgString)
+	} else {
+		_, _ = str.WriteString(pretty.Sprint(tailLineStyle(), msgString))
+	}
+
 	return str.String()
 }
 
 // formatCoderSDKError come from API requests. In verbose mode, add the
 // request debug information.
-func formatCoderSDKError(err *codersdk.Error, opts *formatOpts) string {
+func formatCoderSDKError(from string, err *codersdk.Error, opts *formatOpts) string {
 	var str strings.Builder
 	if opts.Verbose {
 		_, _ = str.WriteString(pretty.Sprint(headLineStyle(), fmt.Sprintf("API request error to \"%s:%s\". Status code %d", err.Method(), err.URL(), err.StatusCode())))
 		_, _ = str.WriteString("\n")
 	}
+	// Always include this trace. Users can ignore this.
+	if from != "" {
+		_, _ = str.WriteString(pretty.Sprint(headLineStyle(), fmt.Sprintf("Trace=[%s]", from)))
+		_, _ = str.WriteString("\n")
+	}
 
 	_, _ = str.WriteString(pretty.Sprint(headLineStyle(), err.Message))
 	if err.Helper != "" {
@@ -1144,6 +1180,21 @@ func formatCoderSDKError(err *codersdk.Error, opts *formatOpts) string {
 	return str.String()
 }
 
+// traceError is a helper function that aides developers debugging failed cli
+// commands. When we pretty print errors, we lose the context in which they came.
+// This function adds the context back. Unfortunately there is no easy way to get
+// the prefix to: "error string: %w", so we do a bit of string manipulation.
+//
+//nolint:errorlint
+func traceError(err error) string {
+	if uw, ok := err.(interface{ Unwrap() error }); ok {
+		a, b := err.Error(), uw.Unwrap().Error()
+		c := strings.TrimSuffix(a, b)
+		return c
+	}
+	return err.Error()
+}
+
 // These styles are arbitrary.
 func headLineStyle() pretty.Style {
 	return cliui.DefaultStyles.Error

diff --git a/cli/templatecreate.go b/cli/templatecreate.go
@@ -95,6 +95,18 @@ func (r *RootCmd) templateCreate() *clibase.Cmd {
 
 			message := uploadFlags.templateMessage(inv)
 
+			var varsFiles []string
+			if !uploadFlags.stdin() {
+				varsFiles, err = DiscoverVarsFiles(uploadFlags.directory)
+				if err != nil {
+					return err
+				}
+
+				if len(varsFiles) > 0 {
+					_, _ = fmt.Fprintln(inv.Stdout, "Auto-discovered Terraform tfvars files. Make sure to review and clean up any unused files.")
+				}
+			}
+
 			// Confirm upload of the directory.
 			resp, err := uploadFlags.upload(inv, client)
 			if err != nil {
@@ -107,6 +119,7 @@ func (r *RootCmd) templateCreate() *clibase.Cmd {
 			}
 
 			userVariableValues, err := ParseUserVariableValues(
+				varsFiles,
 				variablesFile,
 				commandLineVariables)
 			if err != nil {

diff --git a/cli/templateedit.go b/cli/templateedit.go
@@ -87,48 +87,86 @@ func (r *RootCmd) templateEdit() *clibase.Cmd {
 				return xerrors.Errorf("get workspace template: %w", err)
 			}
 
-			// Copy the default value if the list is empty, or if the user
-			// specified the "none" value clear the list.
-			if len(autostopRequirementDaysOfWeek) == 0 {
-				autostopRequirementDaysOfWeek = template.AutostopRequirement.DaysOfWeek
+			// Default values
+			if !userSetOption(inv, "description") {
+				description = template.Description
 			}
-			if len(autostartRequirementDaysOfWeek) == 1 && autostartRequirementDaysOfWeek[0] == "all" {
-				// Set it to every day of the week
-				autostartRequirementDaysOfWeek = []string{"monday", "tuesday", "wednesday", "thursday", "friday", "saturday", "sunday"}
-			} else if len(autostartRequirementDaysOfWeek) == 0 {
-				autostartRequirementDaysOfWeek = template.AutostartRequirement.DaysOfWeek
+
+			if !userSetOption(inv, "icon") {
+				icon = template.Icon
 			}
-			if unsetAutostopRequirementDaysOfWeek {
-				autostopRequirementDaysOfWeek = []string{}
+
+			if !userSetOption(inv, "display-name") {
+				displayName = template.DisplayName
 			}
-			if failureTTL == 0 {
+
+			if !userSetOption(inv, "max-ttl") {
+				maxTTL = time.Duration(template.MaxTTLMillis) * time.Millisecond
+			}
+
+			if !userSetOption(inv, "default-ttl") {
+				defaultTTL = time.Duration(template.DefaultTTLMillis) * time.Millisecond
+			}
+
+			if !userSetOption(inv, "allow-user-autostop") {
+				allowUserAutostop = template.AllowUserAutostop
+			}
+
+			if !userSetOption(inv, "allow-user-autostart") {
+				allowUserAutostart = template.AllowUserAutostart
+			}
+
+			if !userSetOption(inv, "allow-user-cancel-workspace-jobs") {
+				allowUserCancelWorkspaceJobs = template.AllowUserCancelWorkspaceJobs
+			}
+
+			if !userSetOption(inv, "failure-ttl") {
 				failureTTL = time.Duration(template.FailureTTLMillis) * time.Millisecond
 			}
-			if dormancyThreshold == 0 {
+
+			if !userSetOption(inv, "dormancy-threshold") {
 				dormancyThreshold = time.Duration(template.TimeTilDormantMillis) * time.Millisecond
 			}
-			if dormancyAutoDeletion == 0 {
+
+			if !userSetOption(inv, "dormancy-auto-deletion") {
 				dormancyAutoDeletion = time.Duration(template.TimeTilDormantAutoDeleteMillis) * time.Millisecond
 			}
 
-			// Default values
-			if !userSetOption(inv, "description") {
-				description = template.Description
+			if !userSetOption(inv, "require-active-version") {
+				requireActiveVersion = template.RequireActiveVersion
 			}
 
-			if !userSetOption(inv, "icon") {
-				icon = template.Icon
+			if !userSetOption(inv, "autostop-requirement-weekdays") {
+				autostopRequirementDaysOfWeek = template.AutostopRequirement.DaysOfWeek
 			}
 
-			if !userSetOption(inv, "display-name") {
-				displayName = template.DisplayName
+			if unsetAutostopRequirementDaysOfWeek {
+				autostopRequirementDaysOfWeek = []string{}
+			}
+
+			if !userSetOption(inv, "autostop-requirement-weeks") {
+				autostopRequirementWeeks = template.AutostopRequirement.Weeks
+			}
+
+			if len(autostartRequirementDaysOfWeek) == 1 && autostartRequirementDaysOfWeek[0] == "all" {
+				// Set it to every day of the week
+				autostartRequirementDaysOfWeek = []string{"monday", "tuesday", "wednesday", "thursday", "friday", "saturday", "sunday"}
+			} else if !userSetOption(inv, "autostart-requirement-weekdays") {
+				autostartRequirementDaysOfWeek = template.AutostartRequirement.DaysOfWeek
+			} else if len(autostartRequirementDaysOfWeek) == 0 {
+				autostartRequirementDaysOfWeek = []string{}
 			}
 
 			var deprecated *string
-			if !userSetOption(inv, "deprecated") {
+			if userSetOption(inv, "deprecated") {
 				deprecated = &deprecationMessage
 			}
 
+			var disableEveryoneGroup bool
+			if userSetOption(inv, "private") {
+				disableEveryoneGroup = disableEveryone
+			}
+
 			req := codersdk.UpdateTemplateMeta{
 				Name:             name,
 				DisplayName:      displayName,
@@ -151,7 +189,7 @@ func (r *RootCmd) templateEdit() *clibase.Cmd {
 				AllowUserAutostop:              allowUserAutostop,
 				RequireActiveVersion:           requireActiveVersion,
 				DeprecationMessage:             deprecated,
-				DisableEveryoneGroupAccess:     disableEveryone,
+				DisableEveryoneGroupAccess:     disableEveryoneGroup,
 			}
 
 			_, err = client.UpdateTemplateMeta(inv.Context(), template.ID, req)

diff --git a/cli/templatepush.go b/cli/templatepush.go
@@ -78,6 +78,18 @@ func (r *RootCmd) templatePush() *clibase.Cmd {
 
 			message := uploadFlags.templateMessage(inv)
 
+			var varsFiles []string
+			if !uploadFlags.stdin() {
+				varsFiles, err = DiscoverVarsFiles(uploadFlags.directory)
+				if err != nil {
+					return err
+				}
+
+				if len(varsFiles) > 0 {
+					_, _ = fmt.Fprintln(inv.Stdout, "Auto-discovered Terraform tfvars files. Make sure to review and clean up any unused files.")
+				}
+			}
+
 			resp, err := uploadFlags.upload(inv, client)
 			if err != nil {
 				return err
@@ -89,6 +101,7 @@ func (r *RootCmd) templatePush() *clibase.Cmd {
 			}
 
 			userVariableValues, err := ParseUserVariableValues(
+				varsFiles,
 				variablesFile,
 				commandLineVariables)
 			if err != nil {