diff --git a/CHANGELOG.md b/CHANGELOG.md index fb12da4..e0a0cd5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 identical structs on every redraw are eliminated. Reduces GC pressure noticeably on low-power machines and slow terminals. +### Fixed +- Running `layerx ` with no reachable container engine (or with a + Podman connection that is unconfigured, missing, or malformed) now prints the + same friendly, actionable message the other commands already showed — + previously the interactive command printed the raw internal error string with + no recovery hint. +- The archive "could not …" error in the interactive viewer no longer suggests + freeing disk space or setting `TMPDIR` for failures that are not disk-full + (for example a seek or I/O error on a network mount), matching the + command-line behaviour. The disk-space hint now appears only when the + underlying cause is a full disk. +- JSON export now reports a clean "no space left to write <path>" message on a + full disk instead of leaking the internal temporary spool file path into the + error shown to the user. + ## [v1.6.0] - 2026-07-28 Eight built-in colour themes, transparent-background mode, and TUI visual diff --git a/cmd/json.go b/cmd/json.go index 9099bc6..8461226 100644 --- a/cmd/json.go +++ b/cmd/json.go @@ -3,9 +3,11 @@ package cmd import ( "context" "encoding/json" + "errors" "fmt" "os" "path/filepath" + "syscall" "github.com/deveshctl/layerx/image" ) @@ -95,7 +97,10 @@ func runJSONExportFromAnalysis(analysis *image.Analysis, outputPath string) erro } if err := writeJSONAtomic(outputPath, data); err != nil { - return fmt.Errorf("writing %s: %w", outputPath, err) + // writeJSONAtomic already names outputPath in its disk-full message; + // wrap the generic path with %w so callers can still errors.Is/As it + // without prefixing a second, redundant "writing :". + return fmt.Errorf("could not write JSON: %w", err) } fmt.Fprintf(os.Stderr, "layerx: wrote analysis to %s\n", outputPath) @@ -174,26 +179,37 @@ func writeJSONAtomic(targetPath string, data []byte) error { dir := filepath.Dir(targetPath) f, err := os.CreateTemp(dir, ".layerx-json-*.tmp") if err != nil { - return err + return cleanDiskWriteErr(targetPath, err) } tmp := f.Name() if _, err := f.Write(data); err != nil { f.Close() os.Remove(tmp) - return err + return cleanDiskWriteErr(targetPath, err) } if err := f.Sync(); err != nil { f.Close() os.Remove(tmp) - return err + return cleanDiskWriteErr(targetPath, err) } if err := f.Close(); err != nil { os.Remove(tmp) - return err + return cleanDiskWriteErr(targetPath, err) } if err := os.Rename(tmp, targetPath); err != nil { os.Remove(tmp) - return err + return cleanDiskWriteErr(targetPath, err) } return nil } + +// cleanDiskWriteErr rewrites a disk-full failure so the user-facing message +// names the output path they supplied instead of the internal .tmp spool file +// (which the raw syscall error carries). Non-ENOSPC errors pass through +// unchanged — their paths are already the target or a directory the user chose. +func cleanDiskWriteErr(targetPath string, err error) error { + if errors.Is(err, syscall.ENOSPC) { + return fmt.Errorf("no space left to write %s (free up disk space and try again)", targetPath) + } + return err +} diff --git a/cmd/json_test.go b/cmd/json_test.go index 1279c5c..23d0719 100644 --- a/cmd/json_test.go +++ b/cmd/json_test.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "strings" + "syscall" "testing" "github.com/deveshctl/layerx/image" @@ -308,3 +309,22 @@ func TestRunJSONExport_ContextCancelled(t *testing.T) { _, statErr := os.Stat(outPath) assert.True(t, os.IsNotExist(statErr), "no output file must exist when resolve was cancelled (statErr = %v)", statErr) } + +func TestCleanDiskWriteErr_ENOSPCNamesTargetNotTmp(t *testing.T) { + // The raw write error carries the internal .tmp spool path; on ENOSPC the + // user-facing message must name the output path they supplied instead. + target := "/home/user/out.json" + raw := &os.PathError{Op: "write", Path: "/home/user/.layerx-json-123.tmp", Err: syscall.ENOSPC} + got := cleanDiskWriteErr(target, raw) + assert.Contains(t, got.Error(), target) + assert.NotContains(t, got.Error(), ".tmp") + assert.Contains(t, got.Error(), "no space left") +} + +func TestCleanDiskWriteErr_NonENOSPCPassesThrough(t *testing.T) { + // Non-disk-full errors are returned unchanged — their paths are the target + // or a directory the user chose, so there is nothing internal to hide. + raw := errors.New("permission denied") + got := cleanDiskWriteErr("/home/user/out.json", raw) + assert.Same(t, raw, got) +} diff --git a/cmd/root.go b/cmd/root.go index d3a0c62..6d28f95 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -256,24 +256,35 @@ func runInspect(cmd *cobra.Command, args []string) error { return runJSONExport(cmd.Context(), imageRef, flagJSON, noCache) } + // selectResolver produces the typed engine/resolver errors (no engine + // found, Podman connection not configured, malformed connection config) + // synchronously, before the TUI starts. Route them through presentCLIError + // so the user sees the friendly, actionable message rather than the raw + // Error() string cobra would otherwise print — and silence cobra's own + // printer so it does not double-print. Mirrors the --json branch above. + cmd.SilenceErrors = true + resolver, err := selectResolver(imageRef) if err != nil { - return err + return presentCLIError(os.Stderr, err) } theme, err := resolveTheme(cfg.Theme, flagTheme) if err != nil { - return err + return presentCLIError(os.Stderr, err) } - return tui.Run(tui.Config{ + if err := tui.Run(tui.Config{ ImageRef: imageRef, Resolver: resolver, NoCache: noCache, Platform: activePlatformDisplay(), Theme: theme, TransparentBg: cfg.TransparentBackground, - }) + }); err != nil { + return presentCLIError(os.Stderr, err) + } + return nil } // warnCIThresholdFlagsIgnored prints a warning to w when CI=true is active diff --git a/main.go b/main.go index 4ab68ee..fa3db94 100644 --- a/main.go +++ b/main.go @@ -47,6 +47,10 @@ func main() { // `layerx build` exactly like `docker build` / `podman build`. os.Exit(e.ExitCode) } + // Exit 2 covers ErrCIUsage, ErrCompareUsage, and any unrecognised error. + // Usage errors are deliberately distinct from rule failures (ErrCIFailed → + // 1) and build failures (ErrBuildFailed → engine code): do NOT remap + // ErrCIUsage/ErrCompareUsage to exit 1. os.Exit(2) } diff --git a/tui/model.go b/tui/model.go index 5ad0b2f..c2650f5 100644 --- a/tui/model.go +++ b/tui/model.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "strings" + "syscall" "time" tea "charm.land/bubbletea/v2" @@ -2438,7 +2439,14 @@ func friendlyError(err error) string { return fmt.Sprintf("Not a valid image archive: %q. Expected a docker-save or OCI layout tarball.", invalidErr.Path) } if infraErr, ok := errors.AsType[*image.ErrArchiveInfra](err); ok { - return fmt.Sprintf("Could not %s: %v. Free up disk space or set TMPDIR to a writable location and try again.", infraErr.Op, infraErr.Cause) + // The disk-space hint is only trustworthy when the cause is actually a + // full disk. ErrArchiveInfra also covers seek/temp-file/I/O failures + // (e.g. on a network mount), where telling the user to free disk space + // misdirects them. Show the hint only on ENOSPC, matching the CLI. + if errors.Is(infraErr.Cause, syscall.ENOSPC) { + return fmt.Sprintf("Could not %s: %v. Free up disk space or set TMPDIR to a writable location and try again.", infraErr.Op, infraErr.Cause) + } + return fmt.Sprintf("Could not %s: %v.", infraErr.Op, infraErr.Cause) } // Platform errors are already user-readable; pass them through verbatim // so the multi-line "Available platforms:" list keeps its formatting. diff --git a/tui/model_test.go b/tui/model_test.go index 40e2a24..e29bafd 100644 --- a/tui/model_test.go +++ b/tui/model_test.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "strings" + "syscall" "testing" tea "charm.land/bubbletea/v2" @@ -758,6 +759,25 @@ func TestFriendlyErrorGenericReturnsMessage(t *testing.T) { assert.Equal(t, "unexpected failure", friendlyError(err)) } +func TestFriendlyErrorArchiveInfra_ENOSPCKeepsHint(t *testing.T) { + // A genuine disk-full cause should keep the free-up-space hint. + err := &image.ErrArchiveInfra{Op: "spool layer to temp file", Cause: syscall.ENOSPC} + msg := friendlyError(err) + assert.Contains(t, msg, "spool layer to temp file") + assert.Contains(t, msg, "Free up disk space") +} + +func TestFriendlyErrorArchiveInfra_NonENOSPCDropsHint(t *testing.T) { + // A seek/I/O failure that is not ENOSPC must NOT tell the user to free + // disk space — that misdirects on network-mount and permission errors. + err := &image.ErrArchiveInfra{Op: "seek in spooled archive", Cause: errors.New("input/output error")} + msg := friendlyError(err) + assert.Contains(t, msg, "seek in spooled archive") + assert.Contains(t, msg, "input/output error") + assert.NotContains(t, msg, "Free up disk space") + assert.NotContains(t, msg, "TMPDIR") +} + // --- renderLayers ------------------------------------------------------------ func TestRenderLayersDoesNotPanic(t *testing.T) {