path/filepath, io/fs: undocumented behavior in WalkDir about the second call reporting the readdir failure

[liuycsd]

With the new WalkDir API, if the readdir fails, the walkDirFn will be called for a second time, with the readdir error.

The doc says

// The error result returned by the function controls how Walk continues.
// If the function returns the special value SkipDir, Walk skips the
// current directory (path if info.IsDir() is true, otherwise path's
// parent directory). Otherwise, if the function returns a non-nil error,
// Walk stops entirely and returns that error.

If the walkDirFn do return a SkipDir for this second call, it would be returned(

go/src/path/filepath/path.go

Lines 394 to 402 in 7b1ba97

	dirs, err := readDir(path)
	if err != nil {
	// Second call, to report ReadDir error.
	err = walkDirFn(path, d, err)
	if err != nil {
	return err
	}
	}

) to the outer loop and break(

go/src/path/filepath/path.go

Lines 403 to 411 in 7b1ba97

	for _, d1 := range dirs {
	path1 := Join(path, d1.Name())
	if err := walkDir(path1, d1, walkDirFn); err != nil {
	if err == SkipDir {
	break
	}
	return err
	}
	}

) it. So it would not cause Walk to stop walking the entire tree and not only skips the current directory, but also skips path's parent directory even though info.IsDir() is true.

And the doc also says

// The err argument reports an error related to path, signaling that Walk
// will not walk into that directory.

But there is a way letting it walk into the possible partial dentry list by returning nil from the walkDirFn.

These behaviors are different from the Walk func. Is it intentional for them to work like this? If it is, it would be better to have them documented out.

[ianlancetaylor]

I think we should change the code so we ignore SkipDir from the second call. The current behavior doesn't make any sense. Want to send a patch?

[liuycsd]

I think we should change the code so we ignore SkipDir from the second call. The current behavior doesn't make any sense. Want to send a patch?

Sorry, I'm not so familiar with the whole project.
I would appreciate it if someone wants to send a patch for this.

[ianlancetaylor]

Thanks, I sent a patch.

[gopherbot]

Change https://go.dev/cl/392154 mentions this issue: io/fs, path/filepath: honor SkipDir on second WalkDirFunc error call