Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ title: Changelog
* `[Added]` Expose `LETS_OS` and `LETS_ARCH` environment variables at command runtime.
* `[Removed]` Drop deprecated `eval_env` directive. Use `env` with `sh` execution mode instead.
* `[Added]` When a command or its `depends` chain fails, print an indented tree to stderr showing the full chain with the failing command highlighted
* `[Changed]` Format command failure output as a `lets:`-prefixed tree plus a separate final status line such as `lets: exit status 1`.
* `[Added]` Support `env_file` in global config and commands. File names are expanded after `env` is resolved, and values loaded from env files override values from `env`.
* `[Changed]` Migrate the LSP YAML parser from the CGO-based tree-sitter bindings to pure-Go [`gotreesitter`](https://github.com/odvcencio/gotreesitter), removing the C toolchain requirement from normal builds and release packaging.
* `[Refactoring]` Move CLI startup flow from `cmd/lets/main.go` into `internal/cli/cli.go`, keeping `main.go` as a thin launcher.
Expand Down
2 changes: 2 additions & 0 deletions internal/cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ func Main(version string, buildDate string) int {
var depErr *executor.DependencyError
if errors.As(err, &depErr) {
executor.PrintDependencyTree(depErr, os.Stderr)
log.Errorf("%s", depErr.FailureMessage())
return getExitCode(err, 1)
}

log.Errorf("%s", err.Error())
Expand Down
18 changes: 17 additions & 1 deletion internal/executor/dependency_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ type DependencyError struct {
Err error
}

const treePrefix = "lets: "

func (e *DependencyError) Error() string { return e.Err.Error() }
func (e *DependencyError) Unwrap() error { return e.Err }

Expand All @@ -29,6 +31,15 @@ func (e *DependencyError) ExitCode() int {
return 1
}

func (e *DependencyError) FailureMessage() string {
var executeErr *ExecuteError
if errors.As(e.Err, &executeErr) {
return executeErr.Cause().Error()
}

return e.Err.Error()
}

// prependToChain prepends name to the chain in err if err is already a *DependencyError,
// otherwise wraps err in a new single-element DependencyError.
func prependToChain(name string, err error) error {
Expand All @@ -45,9 +56,14 @@ func prependToChain(name string, err error) error {
// Respects NO_COLOR automatically via fatih/color.
func PrintDependencyTree(e *DependencyError, w io.Writer) {
red := color.New(color.FgRed).SprintFunc()
treeIndent := strings.Repeat(" ", len(treePrefix))

for i, name := range e.Chain {
indent := strings.Repeat(" ", i+1)
indent := treeIndent + strings.Repeat(" ", i+1)
if i == 0 {
indent = treePrefix
}

if i == len(e.Chain)-1 {
fmt.Fprintf(w, "%s%s %s\n", indent, name, red("<-- failed here"))
} else {
Expand Down
52 changes: 47 additions & 5 deletions internal/executor/dependency_error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ package executor
import (
"bytes"
"fmt"
"os"
"os/exec"
"strings"
"syscall"
"testing"
)

Expand Down Expand Up @@ -101,6 +103,46 @@ func TestDependencyErrorError(t *testing.T) {
}
}

func TestDependencyErrorFailureMessage(t *testing.T) {
t.Run("returns root cause for execute errors", func(t *testing.T) {
depErr := &DependencyError{
Chain: []string{"lint"},
Err: &ExecuteError{err: fmt.Errorf("failed to run command 'lint': %w", fmt.Errorf("exit status 1"))},
}

if got := depErr.FailureMessage(); got != "exit status 1" {
t.Fatalf("expected root cause message, got %q", got)
}
})

t.Run("keeps non execute errors intact", func(t *testing.T) {
depErr := &DependencyError{
Chain: []string{"lint"},
Err: fmt.Errorf("failed to calculate checksum for command 'lint': missing file"),
}

if got := depErr.FailureMessage(); got != "failed to calculate checksum for command 'lint': missing file" {
t.Fatalf("expected original message, got %q", got)
}
})

t.Run("keeps path context for execute errors", func(t *testing.T) {
depErr := &DependencyError{
Chain: []string{"lint"},
Err: &ExecuteError{
err: fmt.Errorf(
"failed to run command 'lint': %w",
&os.PathError{Op: "chdir", Path: "/tmp/missing", Err: syscall.ENOENT},
),
},
}

if got := depErr.FailureMessage(); got != "chdir /tmp/missing: no such file or directory" {
t.Fatalf("expected path-aware message, got %q", got)
}
})
}

func TestPrintDependencyTree(t *testing.T) {
t.Run("single node", func(t *testing.T) {
depErr := &DependencyError{Chain: []string{"lint"}, Err: fmt.Errorf("fail")}
Expand All @@ -111,8 +153,8 @@ func TestPrintDependencyTree(t *testing.T) {
if len(lines) != 1 {
t.Fatalf("expected 1 line, got %d: %v", len(lines), lines)
}
if !strings.HasPrefix(lines[0], " lint") {
t.Errorf("expected line to start with ' lint', got: %q", lines[0])
if !strings.HasPrefix(lines[0], "lets: lint") {
t.Errorf("expected line to start with 'lets: lint', got: %q", lines[0])
}
if !strings.Contains(out, "failed here") {
t.Errorf("expected 'failed here' annotation on lint line, got: %q", out)
Expand All @@ -137,9 +179,9 @@ func TestPrintDependencyTree(t *testing.T) {
name string
hasFailed bool
}{
{" ", "deploy", false},
{" ", "build", false},
{" ", "lint", true},
{"lets: ", "deploy", false},
{" ", "build", false},
{" ", "lint", true},
}
for i, c := range checks {
if !strings.HasPrefix(lines[i], c.prefix+c.name) {
Expand Down
12 changes: 12 additions & 0 deletions internal/executor/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,18 @@ func (e *ExecuteError) Error() string {
return e.err.Error()
}

func (e *ExecuteError) Unwrap() error {
return e.err
}

func (e *ExecuteError) Cause() error {
if err := errors.Unwrap(e.err); err != nil {
return err
}

return e.err
}

// ExitCode will return exit code from underlying ExitError or returns default error code.
func (e *ExecuteError) ExitCode() int {
var exitErr *exec.ExitError
Expand Down
5 changes: 3 additions & 2 deletions tests/command_cmd.bats
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ setup() {
# as there is no guarantee in which order cmds runs
# we can not guarantee that all commands will run and complete.
# But error message must be in the output.
assert_output --partial "failed to run command 'cmd-as-map-error': exit status 2"
assert_output --partial "lets: cmd-as-map-error"
assert_output --partial "lets: exit status 2"
}
Comment on lines 43 to 48
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (testing): Assert ordering and avoid accidentally allowing the old verbose error format

The new expectations only verify that the lets: lines appear somewhere in the output. To better capture the intended behavior and prevent regressions, please also (1) assert that the command line appears before the exit status line, and (2) assert that the old verbose message ("failed to run command 'cmd-as-map-error': exit status 2") does not appear.

Suggested change
# as there is no guarantee in which order cmds runs
# we can not guarantee that all commands will run and complete.
# But error message must be in the output.
assert_output --partial "failed to run command 'cmd-as-map-error': exit status 2"
assert_output --partial "lets: cmd-as-map-error"
assert_output --partial "lets: exit status 2"
}
# as there is no guarantee in which order cmds runs
# we can not guarantee that all commands will run and complete.
# But error message must be in the output.
assert_output --partial "lets: cmd-as-map-error"
assert_output --partial "lets: exit status 2"
# Ensure the command line appears before the exit status line.
local cmd_prefix exit_prefix
cmd_prefix=${output%%lets: cmd-as-map-error*}
exit_prefix=${output%%lets: exit status 2*}
# Sanity check: both fragments must be present
[ "$cmd_prefix" != "$output" ]
[ "$exit_prefix" != "$output" ]
# cmd-as-map-error must appear before the exit status
(( ${#cmd_prefix} < ${#exit_prefix} ))
# Ensure the old verbose error format is not present.
refute_output --partial "failed to run command 'cmd-as-map-error': exit status 2"
}


@test "command_cmd: cmd-as-map must propagate env" {
Expand Down Expand Up @@ -83,4 +84,4 @@ setup() {

assert_success
assert_line --index 0 "Hello from short"
}
}
4 changes: 3 additions & 1 deletion tests/default_env.bats
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,7 @@ setup() {
LETS_CONFIG_DIR=./a run lets print-workdir

assert_failure
assert_line "lets: failed to run command 'print-workdir': chdir ${TEST_DIR}/b: no such file or directory"
assert_line --index 0 --partial "lets: print-workdir"
assert_line --index 0 --partial "failed here"
assert_line --index 1 "lets: chdir ${TEST_DIR}/b: no such file or directory"
}
10 changes: 6 additions & 4 deletions tests/dependency_failure_tree.bats
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,17 @@ setup() {
@test "dependency_failure_tree: shows full 3-level chain on failure" {
run env NO_COLOR=1 lets deploy
assert_failure
assert_line --index 0 " deploy"
assert_line --index 1 " build"
assert_line --index 2 --partial " lint"
assert_line --index 0 "lets: deploy"
assert_line --index 1 " build"
assert_line --index 2 --partial " lint"
assert_line --index 2 --partial "failed here"
assert_line --index 3 "lets: exit status 1"
}

@test "dependency_failure_tree: single node when no depends" {
run env NO_COLOR=1 lets lint
assert_failure
assert_line --index 0 --partial " lint"
assert_line --index 0 --partial "lets: lint"
assert_line --index 0 --partial "failed here"
assert_line --index 1 "lets: exit status 1"
}
Loading