From aa7185cdd052906c3a4b4259442aeb7630d1821c Mon Sep 17 00:00:00 2001 From: deveshctl Date: Sun, 12 Jul 2026 10:33:28 +0530 Subject: [PATCH] fix: sharpen first-run error and confirmation messages A user whose container engine is not running hit a dead end: the error named the failure but never mentioned that layerx can read a saved-image archive straight from disk, with no engine required. Both the CLI message and the interactive error screen now point at that fallback, worded engine-agnostically so it applies to Docker and Podman alike. The error screen also wraps long messages to the terminal width instead of letting them overflow on one line. The archive-infra error previously suggested freeing disk space on every failure, including I/O errors on network mounts where that advice is wrong. It now shows the disk-space hint only when the cause is genuinely a full disk (ENOSPC). Finally, the --json write confirmation now reads as a complete sentence with the layerx: prefix used elsewhere on stderr. --- CHANGELOG.md | 11 +++++++++++ cmd/errors.go | 25 ++++++++++++++++++------- cmd/errors_test.go | 23 +++++++++++++++++++++-- cmd/json.go | 2 +- docs/json-export.md | 8 ++++---- tui/model.go | 26 +++++++++++++++++++------- tui/model_test.go | 20 ++++++++++++++++++++ 7 files changed, 94 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a9517e6..d339803 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 lowercase hexadecimal characters after the `sha256:` prefix. This prevents `layerx cache prune` from deleting unrelated directories when `LAYERX_CACHE_DIR` points at a shared parent directory. +- When a container engine (Docker or Podman) is not reachable, the error now + points at archive mode — you can inspect a saved-image archive straight from + disk without any engine running. Applies to both the CLI message and the + interactive error screen. +- The interactive error screen now wraps long messages to the terminal width + instead of letting them overflow on one line. +- The "could not …" archive infrastructure error only suggests freeing disk + space or setting `TMPDIR` when the underlying cause is actually a full disk; + other I/O failures no longer show a misleading disk-space hint. +- The `--json` write confirmation now reads `layerx: wrote analysis to `, + matching the `layerx:` prefix used elsewhere on stderr. ### Fixed - File extraction (`x` key) no longer silently overwrites a dangling symlink diff --git a/cmd/errors.go b/cmd/errors.go index b2610c0..86fb6bf 100644 --- a/cmd/errors.go +++ b/cmd/errors.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "strings" + "syscall" "github.com/deveshctl/layerx/image" "github.com/deveshctl/layerx/image/engine" @@ -39,7 +40,13 @@ func friendlyCLIError(err error) string { return fmt.Sprintf("not a valid image archive %q (expected docker-save or OCI layout tarball)", e.Path) } if e, ok := errors.AsType[*image.ErrArchiveInfra](err); ok { - return fmt.Sprintf("could not %s: %v (free up disk space or set TMPDIR)", e.Op, e.Cause) + // The disk-space hint is only trustworthy when the cause is + // actually a full disk; appending it to an I/O error on a network + // mount misdirects the user. Show it only on ENOSPC. + if errors.Is(e.Cause, syscall.ENOSPC) { + return fmt.Sprintf("could not %s: %v (free up disk space or set TMPDIR)", e.Op, e.Cause) + } + return fmt.Sprintf("could not %s: %v", e.Op, e.Cause) } if e, ok := errors.AsType[*image.ErrPodmanSocketNotSet](err); ok { return fmt.Sprintf("--engine podman on %s: no Podman connection configured "+ @@ -87,22 +94,26 @@ func presentCLIError(w io.Writer, err error) error { // the resolver did not know its engine name, so a test double or a // FromEnv-only resolver still produces a sensible message. func daemonNotRunningLine(e *image.ErrDaemonNotRunning) string { + // Both engines are optional: layerx reads a `docker save` / `podman save` + // / OCI-layout archive straight from disk. Point every daemon-down path + // at that fallback so a user without a running engine is not dead-ended. + const archiveHint = " Or pass a saved-image archive path instead (no engine needed)." engine := e.Engine switch engine { case "docker": if e.Host != "" { - return fmt.Sprintf("Docker daemon at %s is not reachable. Is Docker running there?", e.Host) + return fmt.Sprintf("Docker daemon at %s is not reachable. Is Docker running there?", e.Host) + archiveHint } - return "Docker daemon is not reachable. Is Docker running?" + return "Docker daemon is not reachable. Is Docker running?" + archiveHint case "podman": if e.Host != "" { - return fmt.Sprintf("Podman connection at %s is not reachable. Check the connection with `podman system connection list` / `podman info`.", e.Host) + return fmt.Sprintf("Podman connection at %s is not reachable. Check the connection with `podman system connection list` / `podman info`.", e.Host) + archiveHint } - return "Podman is not reachable. Check the connection with `podman system connection list` / `podman info`." + return "Podman is not reachable. Check the connection with `podman system connection list` / `podman info`." + archiveHint default: if e.Host != "" { - return fmt.Sprintf("Container engine at %s is not reachable.", e.Host) + return fmt.Sprintf("Container engine at %s is not reachable.", e.Host) + archiveHint } - return "Container engine is not reachable." + return "Container engine is not reachable." + archiveHint } } diff --git a/cmd/errors_test.go b/cmd/errors_test.go index ca519e4..212c423 100644 --- a/cmd/errors_test.go +++ b/cmd/errors_test.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "strings" + "syscall" "testing" "github.com/deveshctl/layerx/image" @@ -53,6 +54,9 @@ func TestFriendlyCLIError_DaemonNotRunning_DockerEngineWithHost(t *testing.T) { } msg := friendlyCLIError(err) assert.Contains(t, msg, "Docker daemon at tcp://remote.example:2376") + // Both engines support reading a saved-image archive from disk, so + // every daemon-down line offers that fallback. + assert.Contains(t, msg, "archive") } func TestFriendlyCLIError_DaemonNotRunning_UnknownEngine(t *testing.T) { @@ -104,10 +108,25 @@ func TestFriendlyCLIError_InvalidArchive(t *testing.T) { } func TestFriendlyCLIError_ArchiveInfra(t *testing.T) { - err := &image.ErrArchiveInfra{Op: "spooling image archive", Cause: errors.New("no space left on device")} + // A genuine out-of-space error keeps the disk-space hint. Wrapping + // syscall.ENOSPC mirrors what os file writes surface on a full disk. + err := &image.ErrArchiveInfra{ + Op: "spooling image archive", + Cause: fmt.Errorf("write temp: %w", syscall.ENOSPC), + } + msg := friendlyCLIError(err) + assert.Contains(t, msg, "spooling image archive") + assert.Contains(t, msg, "free up disk space") +} + +func TestFriendlyCLIError_ArchiveInfra_NonDiskError(t *testing.T) { + // A non-ENOSPC infra failure (e.g. an I/O error on a network mount) + // must NOT claim the disk is full — the hint would misdirect the user. + err := &image.ErrArchiveInfra{Op: "spooling image archive", Cause: errors.New("input/output error")} msg := friendlyCLIError(err) assert.Contains(t, msg, "spooling image archive") - assert.Contains(t, msg, "no space left on device") + assert.Contains(t, msg, "input/output error") + assert.NotContains(t, msg, "free up disk space") } func TestFriendlyCLIError_PodmanSocketNotSet_Darwin(t *testing.T) { diff --git a/cmd/json.go b/cmd/json.go index 821e0c4..9099bc6 100644 --- a/cmd/json.go +++ b/cmd/json.go @@ -98,7 +98,7 @@ func runJSONExportFromAnalysis(analysis *image.Analysis, outputPath string) erro return fmt.Errorf("writing %s: %w", outputPath, err) } - fmt.Fprintf(os.Stderr, "Written to %s\n", outputPath) + fmt.Fprintf(os.Stderr, "layerx: wrote analysis to %s\n", outputPath) return nil } diff --git a/docs/json-export.md b/docs/json-export.md index 761b34e..e8805c4 100644 --- a/docs/json-export.md +++ b/docs/json-export.md @@ -49,10 +49,10 @@ A few practical notes: - The JSON is written atomically (temp file in the target directory → `fsync` → rename), so partial writes never reach the destination path. -- The success message `Written to ` goes to **stderr**, not stdout. - This keeps stdout reserved for any pipeline that combines `--json` with - another stdout-producing flow (it doesn't in v1, but the convention - matters). +- The success message `layerx: wrote analysis to ` goes to **stderr**, + not stdout. This keeps stdout reserved for any pipeline that combines + `--json` with another stdout-producing flow (it doesn't in v1, but the + convention matters). - There is no `--json -` (stdout) option. Write to a regular file, then pipe its contents (`cat out.json | jq …`). The export uses an atomic temp-file-and-rename, which is incompatible with character devices like diff --git a/tui/model.go b/tui/model.go index 948f0b7..6a458f4 100644 --- a/tui/model.go +++ b/tui/model.go @@ -1601,7 +1601,16 @@ func (m model) renderRightPanel(width, height int) string { } func (m model) viewError() tea.View { - errStyle := lipgloss.NewStyle().Foreground(removedColor).Bold(true) + // Bound the message width so a long error (e.g. a daemon-down line with + // its archive-mode hint) wraps instead of overflowing a narrow terminal. + wrapWidth := 60 + if m.width > 0 && m.width-4 < wrapWidth { + wrapWidth = m.width - 4 + } + if wrapWidth < 1 { + wrapWidth = 1 + } + errStyle := lipgloss.NewStyle().Foreground(removedColor).Bold(true).Width(wrapWidth) hintStyle := lipgloss.NewStyle().Foreground(statusDimColor) msg := errStyle.Render("Error: "+m.errMsg) + "\n\n" + hintStyle.Render("Press q or Esc to exit.") content := lipgloss.Place(m.width, m.height, lipgloss.Center, lipgloss.Center, msg) @@ -2144,22 +2153,25 @@ func (m *model) viewVisibleHeight() int { func friendlyError(err error) string { if daemonErr, ok := errors.AsType[*image.ErrDaemonNotRunning](err); ok { + // Both engines are optional: layerx reads a saved-image archive + // straight from disk, so a daemon-down user is not dead-ended. + const archiveHint = " Or run layerx on a saved-image archive instead (no engine needed)." switch daemonErr.Engine { case "docker": if daemonErr.Host != "" { - return fmt.Sprintf("Docker daemon at %s is not reachable. Please check the daemon and try again.", daemonErr.Host) + return fmt.Sprintf("Docker daemon at %s is not reachable. Please check the daemon and try again.", daemonErr.Host) + archiveHint } - return "Docker is not running. Please start Docker and try again." + return "Docker is not running. Please start Docker and try again." + archiveHint case "podman": if daemonErr.Host != "" { - return fmt.Sprintf("Podman connection at %s is not reachable. Please check the connection and try again.", daemonErr.Host) + return fmt.Sprintf("Podman connection at %s is not reachable. Please check the connection and try again.", daemonErr.Host) + archiveHint } - return "Podman is not reachable. Please check the connection and try again." + return "Podman is not reachable. Please check the connection and try again." + archiveHint default: if daemonErr.Host != "" { - return fmt.Sprintf("Container engine at %s is not reachable. Please check the endpoint and try again.", daemonErr.Host) + return fmt.Sprintf("Container engine at %s is not reachable. Please check the endpoint and try again.", daemonErr.Host) + archiveHint } - return "Container engine is not reachable. Please check the endpoint and try again." + return "Container engine is not reachable. Please check the endpoint and try again." + archiveHint } } if pullErr, ok := errors.AsType[*image.ErrPullFailed](err); ok { diff --git a/tui/model_test.go b/tui/model_test.go index aa030c9..5a136da 100644 --- a/tui/model_test.go +++ b/tui/model_test.go @@ -571,6 +571,23 @@ func TestViewErrorContainsErrorMessage(t *testing.T) { assert.Contains(t, content, "Docker is not running") } +// TestViewErrorWrapsLongMessage asserts a long daemon-down message (the +// engine-down line plus its archive-mode hint) wraps within the bounded +// width instead of overflowing a standard 80-column terminal on one line. +func TestViewErrorWrapsLongMessage(t *testing.T) { + m := NewModel(Config{ImageRef: "test:latest"}) + m.width = 80 + m.height = 24 + m.state = stateError + m.errMsg = "Docker is not running. Please start Docker and try again. " + + "Or run layerx on a saved-image archive instead (no engine needed)." + v := m.View() + content := viewContent(v) + assert.Contains(t, content, "Docker is not running") + assert.Contains(t, content, "archive") + assert.LessOrEqual(t, maxPanelLineWidth(content), m.width) +} + // --- View: ready state ------------------------------------------------------- func TestViewReadyContainsBrandAndImageRef(t *testing.T) { @@ -690,6 +707,9 @@ func TestFriendlyErrorDaemonNotRunning_DockerEngine(t *testing.T) { } msg := friendlyError(err) assert.Contains(t, msg, "Docker is not running") + // Every daemon-down path points at the archive fallback so a user + // without a running engine is not dead-ended in the TUI. + assert.Contains(t, msg, "archive") } func TestFriendlyErrorDaemonNotRunning_PodmanEngine(t *testing.T) {