[cli] Enhance more template execution error types

.go-version

@@ -1 +1 @@
-1.21.3
+1.21.4

internal/cli/cli_test.go

@@ -48,8 +48,8 @@ const (
 	exitcodeMakesChanges = 91
 	exitcodeNoChanges    = 90
 	exitcodeError        = 92
-
-	testPlanCmdString = "plan --exit-code-no-changes=90 --exit-code-makes-changes=91 --exit-code-error=92"
+	strFailedToFindPack  = "Failed to find pack"
+	testPlanCmdString    = "plan --exit-code-no-changes=90 --exit-code-makes-changes=91 --exit-code-error=92"
 )
 
 func TestCLI_CreateTestRegistry(t *testing.T) {
@@ -102,7 +102,7 @@ func TestCLI_JobRunConflictingDeployment(t *testing.T) {
 		result := runTestPackCmd(t, s, []string{"run", getTestPackPath(t, testPack), "--name=with-name"})
 		must.Eq(t, 1, result.exitCode)
 		must.Eq(t, result.cmdErr.String(), "", must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String()))
-		must.StrContains(t, result.cmdOut.String(), job.ErrExistsInDeployment{JobID: testPack, Deployment: testPack}.Error())
+		must.StrContains(t, result.cmdOut.String(), helper.FirstRuneToUpper(job.ErrExistsInDeployment{JobID: testPack, Deployment: testPack}.Error()))
 
 		// Confirm that it's still possible to update the existing pack
 		expectGoodPackDeploy(t, runTestPackCmd(t, s, []string{"run", getTestPackPath(t, testPack)}))
@@ -121,7 +121,7 @@ func TestCLI_JobRunConflictingNonPackJob(t *testing.T) {
 
 		must.Eq(t, 1, result.exitCode)
 		must.Eq(t, result.cmdErr.String(), "", must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String()))
-		must.StrContains(t, result.cmdOut.String(), job.ErrExistsNonPack{JobID: testPack}.Error())
+		must.StrContains(t, result.cmdOut.String(), helper.FirstRuneToUpper(job.ErrExistsNonPack{JobID: testPack}.Error()))
 	})
 }
 
@@ -136,7 +136,7 @@ func TestCLI_JobRunConflictingJobWithMeta(t *testing.T) {
 		result := runTestPackCmd(t, s, []string{"run", getTestPackPath(t, testPack)})
 		must.Eq(t, 1, result.exitCode)
 		must.Eq(t, result.cmdErr.String(), "", must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String()))
-		must.StrContains(t, result.cmdOut.String(), job.ErrExistsNonPack{JobID: testPack}.Error())
+		must.StrContains(t, result.cmdOut.String(), helper.FirstRuneToUpper(job.ErrExistsNonPack{JobID: testPack}.Error()))
 	})
 }
 
@@ -147,7 +147,7 @@ func TestCLI_JobRunFails(t *testing.T) {
 
 	must.Eq(t, 1, result.exitCode)
 	must.Eq(t, "", result.cmdErr.String(), must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String()))
-	must.StrContains(t, result.cmdOut.String(), "Failed To Find Pack")
+	must.StrContains(t, result.cmdOut.String(), strFailedToFindPack)
 }
 
 func TestCLI_JobPlan(t *testing.T) {
@@ -161,7 +161,7 @@ func TestCLI_JobPlan_BadJob(t *testing.T) {
 		result := runTestPackCmd(t, s, []string{"plan", "fake-job"})
 
 		must.Eq(t, "", result.cmdErr.String(), must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String()))
-		must.StrContains(t, result.cmdOut.String(), "Failed To Find Pack")
+		must.StrContains(t, result.cmdOut.String(), strFailedToFindPack)
 		must.Eq(t, 255, result.exitCode) // Should return 255 indicating an error
 	})
 }
@@ -177,7 +177,7 @@ func TestCLI_JobPlan_ConflictingDeployment(t *testing.T) {
 
 		result := runTestPackCmd(t, s, []string{"run", testPack, testRegFlag, testRefFlag})
 		must.Eq(t, "", result.cmdErr.String(), must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String()))
-		must.StrContains(t, result.cmdOut.String(), job.ErrExistsInDeployment{JobID: testPack, Deployment: testPack + "@latest"}.Error())
+		must.StrContains(t, result.cmdOut.String(), helper.FirstRuneToUpper(job.ErrExistsInDeployment{JobID: testPack, Deployment: testPack + "@latest"}.Error()))
 		must.Eq(t, 1, result.exitCode)
 	})
 }
@@ -192,7 +192,7 @@ func TestCLI_JobPlan_ConflictingNonPackJob(t *testing.T) {
 		// Now try to register the pack
 		result := runTestPackCmd(t, s, []string{"plan", getTestPackPath(t, testPack)})
 		must.Eq(t, "", result.cmdErr.String(), must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String()))
-		must.StrContains(t, result.cmdOut.String(), job.ErrExistsNonPack{JobID: testPack}.Error())
+		must.StrContains(t, result.cmdOut.String(), helper.FirstRuneToUpper(job.ErrExistsNonPack{JobID: testPack}.Error()))
 		must.Eq(t, 255, result.exitCode) // Should return 255 indicating an error
 	})
 }
@@ -223,7 +223,7 @@ func TestCLI_PackPlan_OverrideExitCodes(t *testing.T) {
 			// Now try to register the pack, should be error
 			result := runTestPackCmd(t, s, testPlanCommand(t))
 			must.Eq(t, "", result.cmdErr.String(), must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String()))
-			must.StrContains(t, result.cmdOut.String(), job.ErrExistsNonPack{JobID: testPack}.Error())
+			must.StrContains(t, result.cmdOut.String(), helper.FirstRuneToUpper(job.ErrExistsNonPack{JobID: testPack}.Error()))
 			must.Eq(t, exitcodeError, result.exitCode) // Should exit-code-error
 		})
 

internal/cli/cli_v1_test.go

@@ -46,7 +46,7 @@ func TestCLI_V1_JobRunConflictingDeployment(t *testing.T) {
 		result := runTestPackV1Cmd(t, s, []string{"run", getTestPackV1Path(t, testPack), "--name=with-name"})
 		must.Eq(t, 1, result.exitCode)
 		expectNoStdErrOutput(t, result)
-		must.StrContains(t, result.cmdOut.String(), job.ErrExistsInDeployment{JobID: testPack, Deployment: testPack}.Error())
+		must.StrContains(t, result.cmdOut.String(), helper.FirstRuneToUpper(job.ErrExistsInDeployment{JobID: testPack, Deployment: testPack}.Error()))
 
 		// Confirm that it's still possible to update the existing pack
 		expectGoodPackDeploy(t, runTestPackV1Cmd(t, s, []string{"run", getTestPackV1Path(t, testPack)}))
@@ -65,7 +65,7 @@ func TestCLI_V1_JobRunConflictingNonPackJob(t *testing.T) {
 
 		must.Eq(t, 1, result.exitCode)
 		expectNoStdErrOutput(t, result)
-		must.StrContains(t, result.cmdOut.String(), job.ErrExistsNonPack{JobID: testPack}.Error())
+		must.StrContains(t, result.cmdOut.String(), helper.FirstRuneToUpper(job.ErrExistsNonPack{JobID: testPack}.Error()))
 	})
 }
 
@@ -80,7 +80,7 @@ func TestCLI_V1_JobRunConflictingJobWithMeta(t *testing.T) {
 		result := runTestPackV1Cmd(t, s, []string{"run", getTestPackV1Path(t, testPack)})
 		must.Eq(t, 1, result.exitCode)
 		expectNoStdErrOutput(t, result)
-		must.StrContains(t, result.cmdOut.String(), job.ErrExistsNonPack{JobID: testPack}.Error())
+		must.StrContains(t, result.cmdOut.String(), helper.FirstRuneToUpper(job.ErrExistsNonPack{JobID: testPack}.Error()))
 	})
 }
 
@@ -91,7 +91,7 @@ func TestCLI_V1_JobRunFails(t *testing.T) {
 
 	must.Eq(t, 1, result.exitCode)
 	expectNoStdErrOutput(t, result)
-	must.StrContains(t, result.cmdOut.String(), "Failed To Find Pack")
+	must.StrContains(t, result.cmdOut.String(), strFailedToFindPack)
 }
 
 func TestCLI_V1_JobPlan(t *testing.T) {
@@ -106,7 +106,7 @@ func TestCLI_V1_JobPlan_BadJob(t *testing.T) {
 
 		must.Eq(t, 255, result.exitCode) // Should return 255 indicating an error
 		must.Eq(t, "", result.cmdErr.String(), must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String()))
-		must.StrContains(t, result.cmdOut.String(), "Failed To Find Pack")
+		must.StrContains(t, result.cmdOut.String(), strFailedToFindPack)
 	})
 }
 
@@ -122,7 +122,7 @@ func TestCLI_V1_JobPlan_ConflictingDeployment(t *testing.T) {
 		result := runTestPackV1Cmd(t, s, []string{"run", testPack, testRegFlag, testRefFlag})
 		must.Eq(t, 1, result.exitCode)
 		expectNoStdErrOutput(t, result)
-		must.StrContains(t, result.cmdOut.String(), job.ErrExistsInDeployment{JobID: testPack, Deployment: testPack + "@latest"}.Error())
+		must.StrContains(t, result.cmdOut.String(), helper.FirstRuneToUpper(job.ErrExistsInDeployment{JobID: testPack, Deployment: testPack + "@latest"}.Error()))
 	})
 }
 
@@ -137,7 +137,7 @@ func TestCLI_V1_JobPlan_ConflictingNonPackJob(t *testing.T) {
 		result := runTestPackV1Cmd(t, s, []string{"plan", getTestPackV1Path(t, testPack)})
 		must.Eq(t, 255, result.exitCode) // Should return 255 indicating an error
 		expectNoStdErrOutput(t, result)
-		must.StrContains(t, result.cmdOut.String(), job.ErrExistsNonPack{JobID: testPack}.Error())
+		must.StrContains(t, result.cmdOut.String(), helper.FirstRuneToUpper(job.ErrExistsNonPack{JobID: testPack}.Error()))
 	})
 }
 
@@ -166,7 +166,7 @@ func TestCLI_V1_PackPlan_OverrideExitCodes(t *testing.T) {
 			// Now try to register the pack, should make error
 			result := runTestPackV1Cmd(t, s, testPlanCommand(t))
 			must.Eq(t, "", result.cmdErr.String(), must.Sprintf("cmdErr should be empty, but was %q", result.cmdErr.String()))
-			must.StrContains(t, result.cmdOut.String(), job.ErrExistsNonPack{JobID: testPack}.Error())
+			must.StrContains(t, result.cmdOut.String(), helper.FirstRuneToUpper(job.ErrExistsNonPack{JobID: testPack}.Error()))
 			must.Eq(t, exitcodeError, result.exitCode) // Should exit-code-error
 		})
 

internal/pkg/errors/template_errors.go

@@ -76,10 +76,12 @@ func (p *PackTemplateError) pos() string {
 	if p.Line == 0 {
 		return ""
 	}
-	out = fmt.Sprint(p.Line)
-	if p.StartChar != 0 {
-		out += fmt.Sprintf(",%d", p.StartChar)
+	out = fmt.Sprintf("Ln %v", p.Line)
+	start := p.StartChar
+	if p.StartChar == 0 {
+		p.StartChar = 1
 	}
+	out += fmt.Sprintf(", Col %d", start)
 	return out
 }
 
@@ -116,11 +118,13 @@ func (p *PackTemplateError) parseExecError(execErr template.ExecError) {
 }
 
 func (p *PackTemplateError) extractSource() {
-	hasElement := true
-	in := p.Err.Error()
+	var a, b string
+	found := true
 
-	for hasElement {
-		if b, a, found := strings.Cut(in, ": "); found {
+	in := p.Err.Error() // in is mutated to over time
+
+	for found {
+		if b, a, found = strings.Cut(in, ": "); found {
 			// the first element is "template"
 			if b == "template" {
 				in = a
@@ -141,8 +145,9 @@ func (p *PackTemplateError) extractSource() {
 				p.Filename = parts[0]
 				in = a
 				continue
+			} else {
+				found = false
 			}
-			hasElement = false
 		}
 	}
 }
@@ -154,6 +159,27 @@ func (p *PackTemplateError) enhance() {
 	if p.isV2Error() {
 		p.enhanceV2Error()
 	}
+	if ok, ce := p.hasCallingError(); ok {
+		// In this case, the calling error makes more sense as the given error
+		// and the template runtime error is more of a detail--switch them around.
+		p.Details = p.Error()
+		p.Err = fmt.Errorf("%v", ce)
+	}
+}
+
+func (p *PackTemplateError) hasCallingError() (bool, string) {
+	const errorCallingPrefix = "error calling "
+	for _, e := range p.Extra {
+		if strings.HasPrefix(e, errorCallingPrefix) {
+			return true,
+				fmt.Sprintf(
+					"%s`%s`",
+					errorCallingPrefix,
+					strings.TrimPrefix(e, errorCallingPrefix),
+				)
+		}
+	}
+	return false, ""
 }
 
 func (p *PackTemplateError) isNPE() bool {

internal/pkg/helper/ucformat.go

@@ -0,0 +1,25 @@
+package helper
+
+import (
+	"strings"
+	"unicode"
+	"unicode/utf8"
+)
+
+// FirstRuneToUpper converts first rune to upper case if necessary.
+func FirstRuneToUpper(str string) string {
+	if str == "" {
+		return str
+	}
+
+	r, size := utf8.DecodeRuneInString(str)
+
+	if !unicode.IsLower(r) {
+		return str
+	}
+
+	buf := strings.Builder{}
+	buf.WriteRune(unicode.ToUpper(r))
+	buf.WriteString(str[size:])
+	return buf.String()
+}

internal/testui/main.go

@@ -206,8 +206,8 @@ func (ui *nonInteractiveTestUI) Error(msg string) {
 // ErrorWithContext satisfies the ErrorWithContext function on the UI
 // interface.
 func (ui *nonInteractiveTestUI) ErrorWithContext(err error, sub string, ctx ...string) {
-	ui.Error(helper.Title(sub))
-	ui.Error("  Error: " + err.Error())
+	ui.Error(helper.FirstRuneToUpper(sub))
+	ui.Error("  Error: " + helper.FirstRuneToUpper(err.Error()))
 
 	// Selectively promote Details and Suggestion from the context.
 	var extractItem = func(ctx []string, key string) ([]string, string, bool) {
@@ -231,9 +231,9 @@ func (ui *nonInteractiveTestUI) ErrorWithContext(err error, sub string, ctx ...s
 			case 1:
 				// There is something odd going on if we don't get a 2 split
 				// if we get 1, print the whole thing out.
-				ui.Error("  " + splits[0])
+				ui.Error("  " + helper.FirstRuneToUpper(splits[0]))
 			default:
-				ui.Error("  " + splits[0] + ": " + strings.Join(splits[1:], ": "))
+				ui.Error("  " + splits[0] + ": " + helper.FirstRuneToUpper(strings.Join(splits[1:], ": ")))
 			}
 		}
 	}

terminal/glint.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"regexp"
 	"slices"
 	"strings"
 	"text/tabwriter"
@@ -24,6 +25,8 @@ import (
 	"github.com/hashicorp/nomad-pack/internal/pkg/helper"
 )
 
+var rePlaceholder = regexp.MustCompile("^<<.+>>$")
+
 type glintUI struct {
 	ctx context.Context
 	d   *glint.Document
@@ -362,15 +365,15 @@ func (ui *glintUI) ErrorWithContext(err error, sub string, ctx ...string) {
 	// Title the error output in red with the subject.
 	d.Append(glint.Layout(
 		glint.Style(
-			glint.Text(fmt.Sprintf("! %s\n", helper.Title(sub))),
-			glint.Color("red"),
+			glint.Text(fmt.Sprintf("! %s\n", helper.FirstRuneToUpper(sub))),
+			glint.Bold(), glint.Color("red"),
 		),
 	).Row())
 
 	// Add the error string as well as the error type to the output.
 	d.Append(glint.Layout(
 		glint.Style(glint.Text("    Error:   "), glint.Bold()),
-		glint.Text(err.Error()),
+		glint.Text(helper.FirstRuneToUpper(err.Error())),
 	).Row())
 
 	// Selectively promote Details and Suggestion from the context.
@@ -396,12 +399,12 @@ func (ui *glintUI) ErrorWithContext(err error, sub string, ctx ...string) {
 				// There is something odd going on if we don't get a 2 split
 				// if we get 1, print the whole thing out.
 				d.Append(glint.Layout(
-					glint.Style(glint.Text("    " + splits[0])),
+					glint.Style(glint.Text("    " + helper.FirstRuneToUpper(splits[0]))),
 				).Row())
 			default:
 				d.Append(glint.Layout(
 					glint.Style(glint.Text("    "+splits[0]+":   "), glint.Bold()),
-					glint.Text(strings.Join(splits[1:], ": "))).Row())
+					glint.Text(helper.FirstRuneToUpper(strings.Join(splits[1:], ": ")))).Row())
 			}
 		}
 	}
@@ -419,14 +422,28 @@ func (ui *glintUI) ErrorWithContext(err error, sub string, ctx ...string) {
 
 	// Iterate the addition context items and append these to the output.
 	for _, additionCTX := range ctx {
+		k, v, _ := strings.Cut(additionCTX, ": ")
 		d.Append(glint.Layout(
-			glint.Style(glint.Text(fmt.Sprintf("        - %s", additionCTX))),
+			glint.Text("        • "),
+			glint.Style(
+				glint.Text(fmt.Sprintf("%s: ", k)),
+				glint.Color("lightYellow"),
+			),
+			formatPlaceholder(v),
 		).Row())
 	}
 	// Add a new line
 	d.Append(glint.Layout(glint.Text("")).Row())
 }
 
+func formatPlaceholder(v string) glint.Component {
+	if rePlaceholder.MatchString(v) {
+		v = strings.ReplaceAll(strings.ReplaceAll(v, "<<", "«"), ">>", "»")
+		return glint.Style(glint.Text(v), glint.Italic())
+	}
+	return glint.Text(v)
+}
+
 // Header implements UI
 func (ui *glintUI) Header(msg string) {
 	ui.Output(msg, WithHeaderStyle())

terminal/noninteractive.go

@@ -213,8 +213,8 @@ func (ui *nonInteractiveUI) Error(msg string) {
 // ErrorWithContext satisfies the ErrorWithContext function on the UI
 // interface.
 func (ui *nonInteractiveUI) ErrorWithContext(err error, sub string, ctx ...string) {
-	ui.Error(helper.Title(sub))
-	ui.Error("  Error: " + err.Error())
+	ui.Error(helper.FirstRuneToUpper(sub))
+	ui.Error("  Error: " + helper.FirstRuneToUpper(err.Error()))
 
 	// Selectively promote Details and Suggestion from the context.
 	var extractItem = func(ctx []string, key string) ([]string, string, bool) {
@@ -240,15 +240,15 @@ func (ui *nonInteractiveUI) ErrorWithContext(err error, sub string, ctx ...strin
 				wrapped := wordwrap.WrapString(key, 78)
 				lines := strings.Split(wrapped, "\n")
 				for _, l := range lines {
-					ui.Error("  " + l)
+					ui.Error("  " + helper.FirstRuneToUpper(l))
 				}
 				return
 			}
 			wrapped := wordwrap.WrapString(rest, uint(78-len(key)))
 			lines := strings.Split(wrapped, "\n")
 			for i, l := range lines {
 				if i == 0 {
-					ui.Error(fmt.Sprintf("  %s: %s", key, l))
+					ui.Error(fmt.Sprintf("  %s: %s", key, helper.FirstRuneToUpper(l)))
 					continue
 				}