Skip to content

Commit 22f980e

Browse files
ostermanaknyshautofix-ci[bot]
authored
Fix: Preserve exit codes from shell commands and workflows (#1660)
* Changes auto-committed by Conductor * Changes auto-committed by Conductor * Changes auto-committed by Conductor * updates * updates * Changes auto-committed by Conductor * updates * updates * updates * Fix duplicate exit code error messages using error wrapping pattern… * [autofix.ci] apply automated fixes * Fix duplicate exit code error messages by moving OsExit to errors package - Move OsExit variable from pkg/utils to errors package to avoid import cycle - Update all references to use errUtils.OsExit instead of utils.OsExit - This fixes the issue where tests were failing because Exit() called os.Exit() directly - The mockable OsExit is now in the errors package where Exit() function resides - Resolves duplicate error message output in test failures * Fix duplicate error message in shell runner exit code handling - Remove errors.Join() that was combining ExitCodeError with original error - Return only ExitCodeError to avoid duplicate message like 'subcommand exited with code 1 exit status 1' - Update golden snapshot to reflect correct error message format - Error message is now just 'subcommand exited with code 1' without duplication * Remove temporary development files - Remove check_fix_all_worktrees.sh (temporary worktree debugging script) - Remove fix_worktree_indexes.sh (temporary worktree repair script) - Remove test-fixes.patch (temporary patch file for testing) These were development artifacts that should not be in the repository. * Fix markdown linting issues in auth list blog post - Add 'text' language identifier to code block (fixes MD040) - Replace bold emphasis with proper heading syntax (fixes MD036) - Convert **Quick Overview**, **Detailed Tree View**, and **Automation Integration** to #### headings - Maintain consistent heading hierarchy within the document * Fix duplicate error printing in workflow execution - Make IsKnownWorkflowError recognize ExitCodeError as a known/handled error - This prevents error from being printed twice (once by workflow, once by main.go) - Return original error from workflow step failure to preserve exit code - Workflow context error is already printed before returning - Maintains error chain for proper exit code handling without duplication * Remove auth-list blog post (covered by separate PR) This file was accidentally included in this branch and is covered by the osterman/auth-list-cmd PR. Removing to keep this PR focused on exit code fixes. * Fix workspace creation to detect ExitCodeError instead of ExitError - Updated terraform workspace handling to check for ExitCodeError with code 1 - Previously checked for os/exec.ExitError but ExecuteShellCommand now returns ExitCodeError - This fixes tests failing with 'Workspace doesn't exist' errors - Workspace creation fallback now works correctly when workspace select fails - Resolves test failures: terraform_output_function, env_function, terraform_apply_env, etc. * Update circuit-breaker test snapshot to remove trailing whitespace - Regenerated snapshot for TestCLICommands/atmos_circuit-breaker test - Removed excessive trailing whitespace from error message lines - Snapshot now matches actual output without trailing spaces - Error message 'subcommand exited with code 1' is now clean * updates * updates --------- Co-authored-by: aknysh <[email protected]> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Andriy Knysh <[email protected]>
1 parent 8ef47df commit 22f980e

28 files changed

+1119
-163
lines changed

.goreleaser.yml

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,29 @@
11
# Visit https://goreleaser.com for documentation on how to customize this behavior.
22

3-
#before:
4-
# hooks:
5-
# - 'go mod tidy'
3+
before:
4+
hooks:
5+
# Free disk space on GitHub Actions runners before building to prevent "no space left on device" errors.
6+
# Building 14 platform binaries (linux, darwin, windows, freebsd × multiple architectures) requires
7+
# significant temporary disk space for the Go linker.
8+
#
9+
# The default ubuntu-latest runner (currently ubuntu-24.04 or ubuntu-22.04) does come preloaded with many development tools that consume disk space:
10+
#
11+
# Category Approx. Size Notes
12+
# System files ~20 GB Base Ubuntu OS + required dependencies
13+
# Android SDK / NDK ~12 GB Installed under /usr/local/lib/android — large but unused for most workflows
14+
# .NET SDK ~2–3 GB Installed under /usr/share/dotnet
15+
# Haskell (GHC + Cabal) ~2 GB Installed under /opt/ghc
16+
# CodeQL ~6 GB Installed under /opt/hostedtoolcache/CodeQL — used for GitHub's code scanning
17+
# Other languages/tools ~15–20 GB Includes Node.js, Python, Ruby, Go, Java, etc.
18+
# Free space at job start ~14 GB You typically get 13–15 GB of free disk on /
19+
- |
20+
sh -c 'if [ "$GITHUB_ACTIONS" = "true" ]; then
21+
echo "Freeing disk space..."
22+
df -h /
23+
sudo rm -rf /usr/local/lib/android /usr/share/dotnet /opt/ghc 2>/dev/null || true
24+
df -h /
25+
fi'
26+
627
builds:
728
- env:
829
# goreleaser does not work with CGO, it could also complicate
@@ -23,7 +44,14 @@ builds:
2344
binary: atmos
2445
ldflags:
2546
# Set `atmos` version to the GitHub release tag using Go `ldflags`
47+
# -s: strip symbol table
48+
# -w: strip DWARF debug info
49+
# Both reduce binary size and disk usage during linking.
2650
- '-s -w -X "github.com/cloudposse/atmos/pkg/version.Version={{.Version}}"'
51+
flags:
52+
# Remove file system paths from compiled executables for reproducible builds and slightly smaller binaries.
53+
# This is standard practice for open-source and CI/CD builds (used by Go core team, HashiCorp, Docker, etc.).
54+
- -trimpath
2755

2856
archives:
2957
- format: binary

cmd/root.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ var RootCmd = &cobra.Command{
114114
if versionErr != nil {
115115
errUtils.CheckErrorPrintAndExit(versionErr, "", "")
116116
}
117-
utils.OsExit(0)
117+
errUtils.OsExit(0)
118118
return
119119
}
120120
}
@@ -699,7 +699,7 @@ func initCobraConfig() {
699699
pager := pager.NewWithAtmosConfig(pagerEnabled)
700700
if err := pager.Run("Atmos CLI Help", buf.String()); err != nil {
701701
log.Error("Failed to run pager", "error", err)
702-
utils.OsExit(1)
702+
errUtils.OsExit(1)
703703
}
704704
} else {
705705
fmt.Println()

cmd/root_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ import (
1010
"github.com/spf13/cobra"
1111
"github.com/stretchr/testify/assert"
1212

13+
errUtils "github.com/cloudposse/atmos/errors"
1314
log "github.com/cloudposse/atmos/pkg/logger"
1415
"github.com/cloudposse/atmos/pkg/schema"
15-
"github.com/cloudposse/atmos/pkg/utils"
1616
)
1717

1818
func TestNoColorLog(t *testing.T) {
@@ -392,9 +392,9 @@ func TestVersionFlagExecutionPath(t *testing.T) {
392392
for _, tt := range tests {
393393
t.Run(tt.name, func(t *testing.T) {
394394
// Save original OsExit and restore after test.
395-
originalOsExit := utils.OsExit
395+
originalOsExit := errUtils.OsExit
396396
t.Cleanup(func() {
397-
utils.OsExit = originalOsExit
397+
errUtils.OsExit = originalOsExit
398398
tt.cleanup()
399399
})
400400

@@ -403,7 +403,7 @@ func TestVersionFlagExecutionPath(t *testing.T) {
403403
type exitPanic struct {
404404
code int
405405
}
406-
utils.OsExit = func(code int) {
406+
errUtils.OsExit = func(code int) {
407407
panic(exitPanic{code: code})
408408
}
409409

@@ -414,7 +414,7 @@ func TestVersionFlagExecutionPath(t *testing.T) {
414414
// Execute should call version command and then exit with expected code.
415415
// We expect it to panic with our exitPanic struct containing the exit code.
416416
// This verifies that the --version flag handler is being executed and
417-
// calls os.Exit via utils.OsExit.
417+
// calls os.Exit via errUtils.OsExit.
418418
assert.PanicsWithValue(t, exitPanic{code: tt.expectExit}, func() {
419419
_ = Execute()
420420
}, "Execute should exit with code %d", tt.expectExit)

cmd/validate_schema.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@ package cmd
33
import (
44
"errors"
55

6-
log "github.com/cloudposse/atmos/pkg/logger"
76
"github.com/spf13/cobra"
87

8+
errUtils "github.com/cloudposse/atmos/errors"
99
"github.com/cloudposse/atmos/internal/exec"
10-
u "github.com/cloudposse/atmos/pkg/utils"
10+
log "github.com/cloudposse/atmos/pkg/logger"
1111
)
1212

1313
// ValidateSchemaCmd represents the 'atmos validate schema' command.
@@ -59,12 +59,12 @@ and are compliant with expected formats, reducing configuration drift and runtim
5959

6060
if key == "" && schema != "" {
6161
log.Error("key not provided for the schema to be used")
62-
u.OsExit(1)
62+
errUtils.OsExit(1)
6363
}
6464

6565
if err := exec.NewAtmosValidatorExecutor(&atmosConfig).ExecuteAtmosValidateSchemaCmd(key, schema); err != nil {
6666
if errors.Is(err, exec.ErrInvalidYAML) {
67-
u.OsExit(1)
67+
errUtils.OsExit(1)
6868
}
6969
return err
7070
}

cmd/workflow.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package cmd
22

33
import (
4+
"errors"
5+
46
"github.com/spf13/cobra"
57

68
errUtils "github.com/cloudposse/atmos/errors"
@@ -41,6 +43,11 @@ var workflowCmd = &cobra.Command{
4143
// Check if it's a known error that's already printed in ExecuteWorkflowCmd.
4244
// If it is, we don't need to print it again, but we do need to exit with a non-zero exit code.
4345
if e.IsKnownWorkflowError(err) {
46+
// Check if the error wraps an ExitCodeError to preserve the actual exit code.
47+
var exitCodeErr errUtils.ExitCodeError
48+
if errors.As(err, &exitCodeErr) {
49+
errUtils.Exit(exitCodeErr.Code)
50+
}
4451
errUtils.Exit(1)
4552
}
4653
return err

errors/error_funcs.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,17 @@ import (
55
"os"
66
"os/exec"
77

8-
log "github.com/cloudposse/atmos/pkg/logger"
98
"golang.org/x/text/cases"
109
"golang.org/x/text/language"
1110

11+
log "github.com/cloudposse/atmos/pkg/logger"
1212
"github.com/cloudposse/atmos/pkg/schema"
1313
"github.com/cloudposse/atmos/pkg/ui/markdown"
1414
)
1515

16+
// OsExit is a variable for testing, so we can mock os.Exit.
17+
var OsExit = os.Exit
18+
1619
// render is the global Markdown renderer instance initialized via InitializeMarkdown.
1720
var render *markdown.Renderer
1821

@@ -59,6 +62,12 @@ func CheckErrorPrintAndExit(err error, title string, suggestion string) {
5962

6063
CheckErrorAndPrint(err, title, suggestion)
6164

65+
// Check for ExitCodeError (from ShellRunner preserving interp.ExitStatus)
66+
var exitCodeErr ExitCodeError
67+
if errors.As(err, &exitCodeErr) {
68+
Exit(exitCodeErr.Code)
69+
}
70+
6271
// Find the executed command's exit code from the error
6372
var exitError *exec.ExitError
6473
if errors.As(err, &exitError) {
@@ -73,5 +82,5 @@ func CheckErrorPrintAndExit(err error, title string, suggestion string) {
7382

7483
// Exit exits the program with the specified exit code.
7584
func Exit(exitCode int) {
76-
os.Exit(exitCode)
85+
OsExit(exitCode)
7786
}

errors/error_funcs_test.go

Lines changed: 136 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,102 @@ func Test_CheckErrorPrintAndExit(t *testing.T) {
4242
}
4343
}
4444

45+
func TestPrintErrorMarkdownAndExit(t *testing.T) {
46+
if os.Getenv("TEST_EXIT") == "1" {
47+
er := errors.New("critical failure")
48+
CheckErrorPrintAndExit(er, "Fatal Error", "Check logs.")
49+
return
50+
}
51+
execPath, err := exec.LookPath(os.Args[0])
52+
assert.Nil(t, err)
53+
cmd := exec.Command(execPath, "-test.run=TestPrintErrorMarkdownAndExit")
54+
cmd.Env = append(os.Environ(), "TEST_EXIT=1")
55+
err = cmd.Run()
56+
var exitError *exec.ExitError
57+
if errors.As(err, &exitError) {
58+
assert.Equal(t, 1, exitError.ExitCode())
59+
} else {
60+
assert.Fail(t, "Expected an exit error with code 1")
61+
}
62+
}
63+
64+
func TestPrintInvalidUsageErrorAndExit(t *testing.T) {
65+
if os.Getenv("TEST_EXIT") == "1" {
66+
er := errors.New("invalid command")
67+
CheckErrorPrintAndExit(er, "", "Use --help for usage information.")
68+
return
69+
}
70+
execPath, err := exec.LookPath(os.Args[0])
71+
assert.Nil(t, err)
72+
cmd := exec.Command(execPath, "-test.run=TestPrintInvalidUsageErrorAndExit")
73+
cmd.Env = append(os.Environ(), "TEST_EXIT=1")
74+
err = cmd.Run()
75+
var exitError *exec.ExitError
76+
if errors.As(err, &exitError) {
77+
assert.Equal(t, 1, exitError.ExitCode())
78+
} else {
79+
assert.Fail(t, "Expected an exit error with code 1")
80+
}
81+
}
82+
83+
func TestCheckErrorPrintAndExit_ExitCodeError(t *testing.T) {
84+
if os.Getenv("TEST_EXIT_CODE_2") == "1" {
85+
err := ExitCodeError{Code: 2}
86+
CheckErrorPrintAndExit(err, "Exit Code Error", "")
87+
return
88+
}
89+
if os.Getenv("TEST_EXIT_CODE_42") == "1" {
90+
err := ExitCodeError{Code: 42}
91+
CheckErrorPrintAndExit(err, "Exit Code Error", "")
92+
return
93+
}
94+
95+
// Test exit code 2
96+
execPath, err := exec.LookPath(os.Args[0])
97+
assert.Nil(t, err)
98+
cmd := exec.Command(execPath, "-test.run=TestCheckErrorPrintAndExit_ExitCodeError")
99+
cmd.Env = append(os.Environ(), "TEST_EXIT_CODE_2=1")
100+
err = cmd.Run()
101+
var exitError *exec.ExitError
102+
if errors.As(err, &exitError) {
103+
assert.Equal(t, 2, exitError.ExitCode(), "Should exit with code 2")
104+
} else {
105+
assert.Fail(t, "Expected an exit error with code 2")
106+
}
107+
108+
// Test exit code 42
109+
cmd = exec.Command(execPath, "-test.run=TestCheckErrorPrintAndExit_ExitCodeError")
110+
cmd.Env = append(os.Environ(), "TEST_EXIT_CODE_42=1")
111+
err = cmd.Run()
112+
if errors.As(err, &exitError) {
113+
assert.Equal(t, 42, exitError.ExitCode(), "Should exit with code 42")
114+
} else {
115+
assert.Fail(t, "Expected an exit error with code 42")
116+
}
117+
}
118+
119+
func TestCheckErrorPrintAndExit_ExecExitError(t *testing.T) {
120+
if os.Getenv("TEST_EXEC_EXIT") == "1" {
121+
// Create an exec.ExitError
122+
cmd := exec.Command("sh", "-c", "exit 3")
123+
err := cmd.Run()
124+
CheckErrorPrintAndExit(err, "Exec Error", "")
125+
return
126+
}
127+
128+
execPath, err := exec.LookPath(os.Args[0])
129+
assert.Nil(t, err)
130+
cmd := exec.Command(execPath, "-test.run=TestCheckErrorPrintAndExit_ExecExitError")
131+
cmd.Env = append(os.Environ(), "TEST_EXEC_EXIT=1")
132+
err = cmd.Run()
133+
var exitError *exec.ExitError
134+
if errors.As(err, &exitError) {
135+
assert.Equal(t, 3, exitError.ExitCode(), "Should exit with code 3 from exec.ExitError")
136+
} else {
137+
assert.Fail(t, "Expected an exit error with code 3")
138+
}
139+
}
140+
45141
func TestCheckErrorAndPrint(t *testing.T) {
46142
// Save original logger
47143
originalLogger := log.Default()
@@ -96,42 +192,47 @@ func TestCheckErrorAndPrint(t *testing.T) {
96192
// Should log error directly
97193
assert.Contains(t, logBuf.String(), "error with nil render")
98194
})
99-
}
100195

101-
func TestPrintErrorMarkdownAndExit(t *testing.T) {
102-
if os.Getenv("TEST_EXIT") == "1" {
103-
er := errors.New("critical failure")
104-
CheckErrorPrintAndExit(er, "Fatal Error", "Check logs.")
105-
return
106-
}
107-
execPath, err := exec.LookPath(os.Args[0])
108-
assert.Nil(t, err)
109-
cmd := exec.Command(execPath, "-test.run=TestPrintErrorMarkdownAndExit")
110-
cmd.Env = append(os.Environ(), "TEST_EXIT=1")
111-
err = cmd.Run()
112-
var exitError *exec.ExitError
113-
if errors.As(err, &exitError) {
114-
assert.Equal(t, 1, exitError.ExitCode())
115-
} else {
116-
assert.Fail(t, "Expected an exit error with code 1")
117-
}
196+
// Test with empty title (defaults to "Error")
197+
t.Run("empty title", func(t *testing.T) {
198+
render, _ = markdown.NewTerminalMarkdownRenderer(schema.AtmosConfiguration{})
199+
200+
r, w, _ := os.Pipe()
201+
os.Stderr = w
202+
203+
testErr := errors.New("test error with empty title")
204+
CheckErrorAndPrint(testErr, "", "")
205+
206+
w.Close()
207+
os.Stderr = oldStderr
208+
209+
var output bytes.Buffer
210+
io.Copy(&output, r)
211+
212+
// Should default to "Error" as title
213+
assert.Contains(t, output.String(), "Error")
214+
})
118215
}
119216

120-
func TestPrintInvalidUsageErrorAndExit(t *testing.T) {
121-
if os.Getenv("TEST_EXIT") == "1" {
122-
er := errors.New("invalid command")
123-
CheckErrorPrintAndExit(er, "", "Use --help for usage information.")
124-
return
125-
}
126-
execPath, err := exec.LookPath(os.Args[0])
127-
assert.Nil(t, err)
128-
cmd := exec.Command(execPath, "-test.run=TestPrintErrorMarkdownAndExit")
129-
cmd.Env = append(os.Environ(), "TEST_EXIT=1")
130-
err = cmd.Run()
131-
var exitError *exec.ExitError
132-
if errors.As(err, &exitError) {
133-
assert.Equal(t, 1, exitError.ExitCode())
134-
} else {
135-
assert.Fail(t, "Expected an exit error with code 1")
136-
}
217+
func TestInitializeMarkdown(t *testing.T) {
218+
// Save original logger
219+
originalLogger := log.Default()
220+
defer log.SetDefault(originalLogger)
221+
222+
// Create test logger to capture log output
223+
var logBuf bytes.Buffer
224+
testLogger := log.New()
225+
testLogger.SetOutput(&logBuf)
226+
testLogger.SetLevel(log.TraceLevel)
227+
log.SetDefault(testLogger)
228+
229+
// Test with valid configuration
230+
t.Run("valid configuration", func(t *testing.T) {
231+
logBuf.Reset()
232+
atmosConfig := schema.AtmosConfiguration{}
233+
InitializeMarkdown(atmosConfig)
234+
235+
// Should initialize without error
236+
assert.NotContains(t, logBuf.String(), "failed to initialize Markdown renderer")
237+
})
137238
}

0 commit comments

Comments
 (0)