Refactor socket error handling to unify Docker and Podman socket detection; enhance error messaging for better user guidance

Signed-off-by: DeiAsPie <93835541+DeiAsPie@users.noreply.github.com>

pkg/commands/socket_detection_common.go

@@ -16,7 +16,11 @@ import (
 )
 
 var (
-	ErrNoDockerSocket = fmt.Errorf("no working Docker/Podman socket found")
+	// ErrNoContainerSocket is returned when no working Docker or Podman socket is found.
+	ErrNoContainerSocket = fmt.Errorf("no working Docker/Podman socket found")
+	// ErrNoDockerSocket is an alias for ErrNoContainerSocket for backwards compatibility.
+	// Deprecated: Use ErrNoContainerSocket instead.
+	ErrNoDockerSocket = ErrNoContainerSocket
 )
 
 // Timeout for validating socket connectivity
@@ -94,6 +98,11 @@ func inferRuntimeFromHostHeuristic(host string) ContainerRuntime {
 	return RuntimeDocker
 }
 
+// isSSHHost returns true if the host uses the SSH protocol.
+func isSSHHost(host string) bool {
+	return strings.HasPrefix(host, "ssh://")
+}
+
 func detectDockerHostInternal(log *logrus.Entry) (string, ContainerRuntime, error) {
 	// Priority 1: Explicit DOCKER_HOST environment variable
 	if dockerHost := os.Getenv("DOCKER_HOST"); dockerHost != "" {
@@ -107,11 +116,12 @@ func detectDockerHostInternal(log *logrus.Entry) (string, ContainerRuntime, erro
 			}
 		}
 
-		if !strings.HasPrefix(dockerHost, "ssh://") {
+		if !isSSHHost(dockerHost) {
 			ctx, cancel := context.WithTimeout(context.Background(), socketValidationTimeout)
 			defer cancel()
 			if err := validateSocketFunc(ctx, dockerHost, true); err != nil {
-				return "", RuntimeUnknown, fmt.Errorf("DOCKER_HOST=%s is set but not accessible: %w", dockerHost, err)
+				errMsg := formatConnectionError(ctx, err)
+				return "", RuntimeUnknown, fmt.Errorf("DOCKER_HOST=%s is set but not accessible: %s", dockerHost, errMsg)
 			}
 
 			runtime, err := inferRuntimeFromHostFunc(ctx, dockerHost, true)
@@ -136,14 +146,15 @@ func detectDockerHostInternal(log *logrus.Entry) (string, ContainerRuntime, erro
 		log.Debugf("Failed to get host from default context: %v", err)
 	} else if contextHost != "" {
 		log.Debugf("Using host from Docker context: %s", contextHost)
-		if !strings.HasPrefix(contextHost, "ssh://") {
+		if !isSSHHost(contextHost) {
 			ctx, cancel := context.WithTimeout(context.Background(), socketValidationTimeout)
 			defer cancel()
 			if err := validateSocketFunc(ctx, contextHost, false); err != nil {
+				errMsg := formatConnectionError(ctx, err)
 				if os.Getenv("DOCKER_CONTEXT") != "" {
-					return "", RuntimeUnknown, fmt.Errorf("DOCKER_CONTEXT host %s is not accessible: %w", contextHost, err)
+					return "", RuntimeUnknown, fmt.Errorf("DOCKER_CONTEXT host %s is not accessible: %s", contextHost, errMsg)
 				}
-				log.Warnf("Context host %s is not accessible: %v", contextHost, err)
+				log.Warnf("Context host %s is not accessible: %s", contextHost, errMsg)
 			} else {
 				runtime, err := inferRuntimeFromHostFunc(ctx, contextHost, false)
 				if err != nil {
@@ -162,6 +173,18 @@ func detectDockerHostInternal(log *logrus.Entry) (string, ContainerRuntime, erro
 	return detectPlatformCandidatesFunc(log)
 }
 
+// formatConnectionError returns a user-friendly error message, distinguishing timeout from other errors.
+func formatConnectionError(ctx context.Context, err error) string {
+	if ctx.Err() == context.DeadlineExceeded {
+		return "connection timed out (is the container runtime running?)"
+	}
+	errStr := strings.ToLower(err.Error())
+	if strings.Contains(errStr, "permission denied") || strings.Contains(errStr, "eacces") {
+		return "permission denied (check your user permissions for this socket)"
+	}
+	return err.Error()
+}
+
 // getHostFromContext retrieves the host from the current Docker context
 func getHostFromContext() (string, error) {
 	currentContext := os.Getenv("DOCKER_CONTEXT")

pkg/commands/socket_detection_unix.go

@@ -4,7 +4,6 @@ package commands
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -155,7 +154,7 @@ func detectPlatformCandidates(log *logrus.Entry) (string, ContainerRuntime, erro
 
 	// All candidates failed - provide actionable error
 	if lastErr != nil {
-		return "", RuntimeUnknown, fmt.Errorf("%w: last error: %v", ErrNoDockerSocket, lastErr)
+		return "", RuntimeUnknown, fmt.Errorf("%w: %v", ErrNoContainerSocket, lastErr)
 	}
 
 	var checkedSockets []string
@@ -163,9 +162,8 @@ func detectPlatformCandidates(log *logrus.Entry) (string, ContainerRuntime, erro
 		checkedSockets = append(checkedSockets, strings.TrimPrefix(candidate.Path, DockerSocketSchema))
 	}
 
-	msg := fmt.Sprintf("%v: no Docker or Podman socket found; checked %s. "+
+	return "", RuntimeUnknown, fmt.Errorf("%w: no Docker or Podman socket found; checked %s. "+
 		"If you are using Podman with systemd socket activation, try running "+
 		"'systemctl --user enable --now podman.socket' and then re-run this command.",
-		ErrNoDockerSocket, strings.Join(checkedSockets, ", "))
+		ErrNoContainerSocket, strings.Join(checkedSockets, ", "))
-	return "", RuntimeUnknown, errors.New(msg)
 }

pkg/commands/socket_detection_windows.go

@@ -73,5 +73,5 @@ func detectPlatformCandidates(log *logrus.Entry) (string, ContainerRuntime, erro
 		}
 	}
 
-	return "", RuntimeUnknown, ErrNoDockerSocket
+	return "", RuntimeUnknown, ErrNoContainerSocket
 }

pkg/commands/socket_detection_windows_test.go

@@ -60,7 +60,7 @@ func TestDetectPlatformCandidates_Windows_Failure(t *testing.T) {
 	log := logrus.NewEntry(logrus.New())
 	_, _, err := detectPlatformCandidates(log)
 	assert.Error(t, err)
-	assert.Equal(t, ErrNoDockerSocket, err)
+	assert.ErrorIs(t, err, ErrNoContainerSocket)
 }
 
 func TestGetPodmanPipes(t *testing.T) {
@@ -72,12 +72,16 @@ func TestGetPodmanPipes(t *testing.T) {
 
 	tmpDir, err := os.MkdirTemp("", "podman-test")
 	assert.NoError(t, err)
-	defer os.RemoveAll(tmpDir)
+	t.Cleanup(func() {
+		if err := os.RemoveAll(tmpDir); err != nil {
+			t.Errorf("failed to remove temp dir %s: %v", tmpDir, err)
+		}
+	})
 
 	// Set USERPROFILE to our temp dir
 	oldProfile := os.Getenv("USERPROFILE")
+	t.Cleanup(func() { os.Setenv("USERPROFILE", oldProfile) })
 	os.Setenv("USERPROFILE", tmpDir)
-	defer os.Setenv("USERPROFILE", oldProfile)
 
 	// 1. Test fallback when directory doesn't exist
 	pipes := getPodmanPipes(log)
@@ -99,4 +103,27 @@ func TestGetPodmanPipes(t *testing.T) {
 	assert.Len(t, pipes, 2)
 	assert.Contains(t, pipes, "npipe:////./pipe/machine1")
 	assert.Contains(t, pipes, "npipe:////./pipe/machine2")
+
+	// 3. Test with empty config directory (no JSON files)
+	emptyDir := filepath.Join(tmpDir, ".config", "containers", "podman", "machine", "empty-wsl")
+	err = os.MkdirAll(emptyDir, 0755)
+	assert.NoError(t, err)
+
+	// Create a new temp dir for this specific test
+	tmpDir2, err := os.MkdirTemp("", "podman-test-empty")
+	assert.NoError(t, err)
+	t.Cleanup(func() {
+		if err := os.RemoveAll(tmpDir2); err != nil {
+			t.Errorf("failed to remove temp dir %s: %v", tmpDir2, err)
+		}
+	})
+	os.Setenv("USERPROFILE", tmpDir2)
+
+	// Create the wsl directory but leave it empty
+	emptyConfigDir := filepath.Join(tmpDir2, ".config", "containers", "podman", "machine", "wsl")
+	err = os.MkdirAll(emptyConfigDir, 0755)
+	assert.NoError(t, err)
+
+	pipes = getPodmanPipes(log)
+	assert.Equal(t, []string{"npipe:////./pipe/podman-machine-default"}, pipes)
 }