Merge pull request #71 from twpayne/fix-error-handling

Ensure walkFn is called for all errors
twpayne · Feb 24, 2024 · 158f456 · 158f456
2 parents 8a30e71 + abfa11b
commit 158f456
Show file tree

Hide file tree

Showing 2 changed files with 104 additions and 3 deletions.
diff --git a/vfst/fs_test.go b/vfst/fs_test.go
@@ -1,8 +1,10 @@
 package vfst_test
 
 import (
+	"errors"
 	"io/fs"
 	"path/filepath"
+	"runtime"
 	"testing"
 
 	"github.com/alecthomas/assert/v2"
@@ -38,3 +40,91 @@ func TestWalk(t *testing.T) {
 	}
 	assert.Equal(t, expectedPathTypeMap, pathTypeMap)
 }
+
+func TestWalkErrors(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("test uses UNIX file permissions")
+	}
+	for _, tc := range []struct {
+		name          string
+		root          any
+		postFunc      func(vfs.FS) error
+		expectedPaths []string
+	}{
+		{
+			name: "empty",
+			expectedPaths: []string{
+				"/",
+			},
+		},
+		{
+			name: "simple",
+			root: map[string]any{
+				"/dir/subdir/subsubdir/file": "",
+			},
+			expectedPaths: []string{
+				"/",
+				"/dir",
+				"/dir/subdir",
+				"/dir/subdir/subsubdir",
+				"/dir/subdir/subsubdir/file",
+			},
+		},
+		{
+			name: "private_subdir",
+			root: map[string]any{
+				"/dir/subdir/subsubdir/file": "",
+			},
+			postFunc: func(fileSystem vfs.FS) error {
+				return fileSystem.Chmod("/dir/subdir", 0)
+			},
+			expectedPaths: []string{
+				"/",
+				"/dir",
+				"/dir/subdir",
+			},
+		},
+		{
+			name: "private_subdir_keep_going",
+			root: map[string]any{
+				"/dir/subdir/subsubdir/file": "",
+				"/dir/subdir2/file":          "",
+			},
+			postFunc: func(fileSystem vfs.FS) error {
+				return fileSystem.Chmod("/dir/subdir", 0)
+			},
+			expectedPaths: []string{
+				"/",
+				"/dir",
+				"/dir/subdir",
+				"/dir/subdir2",
+				"/dir/subdir2/file",
+			},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			fileSystem, cleanup, err := vfst.NewTestFS(tc.root)
+			assert.NoError(t, err)
+			if tc.postFunc != nil {
+				assert.NoError(t, tc.postFunc(fileSystem))
+			}
+			defer cleanup()
+			var actualPaths []string
+			assert.NoError(t, vfs.Walk(fileSystem, "/", func(path string, info fs.FileInfo, err error) error {
+				switch {
+				case errors.Is(err, fs.ErrPermission):
+					if info.IsDir() {
+						return vfs.SkipDir
+					}
+					return nil
+				case err != nil:
+					return err
+				default:
+					actualPaths = append(actualPaths, path)
+					return nil
+				}
+			}))
+			assert.Equal(t, tc.expectedPaths, actualPaths)
+		})
+	}
+}
diff --git a/walk.go b/walk.go
@@ -39,19 +39,30 @@ func walk(fileSystem LstatReadDirer, path string, walkFn filepath.WalkFunc, info
 	}
 	dirEntries, err := fileSystem.ReadDir(path)
 	if err != nil {
-		return err
+		switch err := walkFn(path, info, err); {
+		case errors.Is(err, fs.SkipDir):
+			return nil
+		case err != nil:
+			return err
+		}
 	}
 	sort.Sort(dirEntriesByName(dirEntries))
 	for _, dirEntry := range dirEntries {
 		name := dirEntry.Name()
 		if name == "." || name == ".." {
 			continue
 		}
+		path := filepath.Join(path, dirEntry.Name())
 		info, err := dirEntry.Info()
 		if err != nil {
-			return err
+			switch err := walkFn(path, info, err); {
+			case errors.Is(err, fs.SkipDir) && info.IsDir():
+				// Do nothing.
+			case err != nil:
+				return err
+			}
 		}
-		if err := walk(fileSystem, filepath.Join(path, dirEntry.Name()), walkFn, info, nil); err != nil {
+		if err := walk(fileSystem, path, walkFn, info, nil); err != nil {
 			return err
 		}
 	}