Fix Docker Desktop Socket Location for WSL2 #1386
Conversation
|
/review |
There was a problem hiding this comment.
Summary
Good addition for WSL2 support! The change correctly adds detection for the WSL2 Docker Desktop socket path between LinuxKit and native Linux checks. The refactored control flow is clearer and handles the priority order well.
Reviewed Areas
- ✅ Correctness: Logic flow is correct with proper priority ordering
- ✅ Error Handling: Proper use of
errors.Isfor non-ErrNotExist errors ⚠️ Testing: No tests exist for this function- ✅ Idiomatic Go: Follows Go conventions
Minor Suggestions
See inline comments for potential improvements around error handling consistency and code maintainability.
|
|
||
| // On Linux | ||
| if _, err = os.Stat("/mnt/wsl/docker-desktop/shared-sockets/host-services/backend.sock"); err == nil { | ||
| // Inside WSL2 |
There was a problem hiding this comment.
DRY Violation: This error check pattern if !errors.Is(err, os.ErrNotExist) is repeated twice (lines 18-20 and 29-31). Consider extracting it:
if err := checkNotExist(err); err != nil {
return DockerDesktopPaths{}, err
}
func checkNotExist(err error) error {
if err != nil && !errors.Is(err, os.ErrNotExist) {
return err
}
return nil
}Or simply reuse the err variable:
_, err = os.Stat("/mnt/wsl/docker-desktop/shared-sockets/host-services/backend.sock")
if err == nil {
return DockerDesktopPaths{...}, nil
}
if !errors.Is(err, os.ErrNotExist) {
return DockerDesktopPaths{}, err
}| } | ||
|
|
||
| // On Linux | ||
| if _, err = os.Stat("/mnt/wsl/docker-desktop/shared-sockets/host-services/backend.sock"); err == nil { |
There was a problem hiding this comment.
Variable Shadowing: The WSL2 check uses if _, err = os.Stat(...) which shadows the outer err variable. While this works correctly, it might be clearer to use a new variable name or be explicit:
_, wslErr := os.Stat("/mnt/wsl/docker-desktop/shared-sockets/host-services/backend.sock")
if wslErr == nil {
// Inside WSL2
return DockerDesktopPaths{...}, nil
}
if !errors.Is(wslErr, os.ErrNotExist) {
return DockerDesktopPaths{}, wslErr
}This makes it clearer that we're checking a different filesystem operation.
Socket path detection now checks WSL2 path (
/mnt/wsl/docker-desktop/shared-sockets/host-services/backend.sock) between LinuxKit and native Linux checks inpkg/desktop/sockets_linux.go.