Repository: fsnotify/fsnotify
Pull request #460 (2022-07-21 02:43:31)

Use common error when removing an unwatched file
Applies https://github.com/shogo82148/fsnotify/pull/8 to replace #455.

---

The errors returned by the various implementations of the watcher are all different
which makes handling them difficult. This PR follows the suggestion in:
https://github.com/fsnotify/fsnotify/pull/455#issuecomment-1146037157 by @mattn
to create a common error which is wrapped by the implementations.

Replaces: https://github.com/fsnotify/fsnotify/pull/455
Signed-off-by: Andrew Thornton <art27@cantab.net>


Patch:
diff --git a/fsnotify.go b/fsnotify.go
index 0f4ee52..b474b52 100644
--- a/fsnotify.go
+++ b/fsnotify.go
@@ -65,5 +65,6 @@ func (e Event) String() string {
 
 // Common errors that can be reported by a watcher
 var (
-	ErrEventOverflow = errors.New("fsnotify queue overflow")
+	ErrNonExistentWatch = errors.New("can't remove non-existent watcher")
+	ErrEventOverflow    = errors.New("fsnotify queue overflow")
 )
diff --git a/inotify.go b/inotify.go
index a6d0e0e..e6abddf 100644
--- a/inotify.go
+++ b/inotify.go
@@ -134,7 +134,7 @@ func (w *Watcher) Remove(name string) error {
 
 	// Remove it from inotify.
 	if !ok {
-		return fmt.Errorf("can't remove non-existent inotify watch for: %s", name)
+		return fmt.Errorf("%w: %s", ErrNonExistentWatch, name)
 	}
 
 	// We successfully removed the watch if InotifyRmWatch doesn't return an
diff --git a/kqueue.go b/kqueue.go
index 6fb8d85..1b46acb 100644
--- a/kqueue.go
+++ b/kqueue.go
@@ -74,7 +74,7 @@ func (w *Watcher) Close() error {
 	w.isClosed = true
 
 	// copy paths to remove while locked
-	var pathsToRemove = make([]string, 0, len(w.watches))
+	pathsToRemove := make([]string, 0, len(w.watches))
 	for name := range w.watches {
 		pathsToRemove = append(pathsToRemove, name)
 	}
@@ -107,7 +107,7 @@ func (w *Watcher) Remove(name string) error {
 	watchfd, ok := w.watches[name]
 	w.mu.Unlock()
 	if !ok {
-		return fmt.Errorf("can't remove non-existent kevent watch for: %s", name)
+		return fmt.Errorf("%w: %s", ErrNonExistentWatch, name)
 	}
 
 	const registerRemove = unix.EV_DELETE
@@ -442,7 +442,6 @@ func (w *Watcher) sendDirectoryChangeEvents(dirPath string) {
 	for _, fileInfo := range files {
 		filePath := filepath.Join(dirPath, fileInfo.Name())
 		err := w.sendFileCreatedEventIfNew(filePath, fileInfo)
-
 		if err != nil {
 			return
 		}
diff --git a/windows.go b/windows.go
index 02ce7de..1c1fe41 100644
--- a/windows.go
+++ b/windows.go
@@ -21,14 +21,16 @@ import (
 
 // Watcher watches a set of files, delivering events to a channel.
 type Watcher struct {
-	Events   chan Event
-	Errors   chan error
-	isClosed bool           // Set to true when Close() is first called
-	mu       sync.Mutex     // Map access
-	port     syscall.Handle // Handle to completion port
-	watches  watchMap       // Map of watches (key: i-number)
-	input    chan *input    // Inputs to the reader are sent on this channel
-	quit     chan chan<- error
+	Events chan Event
+	Errors chan error
+
+	port  syscall.Handle // Handle to completion port
+	input chan *input    // Inputs to the reader are sent on this channel
+	quit  chan chan<- error
+
+	mu       sync.Mutex // Protects access to watches, isClosed
+	watches  watchMap   // Map of watches (key: i-number)
+	isClosed bool       // Set to true when Close() is first called
 }
 
 // NewWatcher establishes a new watcher with the underlying OS and begins waiting for events.
@@ -51,6 +53,9 @@ func NewWatcher() (*Watcher, error) {
 
 // Close removes all watches and closes the events channel.
 func (w *Watcher) Close() error {
+	w.mu.Lock()
+	defer w.mu.Unlock()
+
 	if w.isClosed {
 		return nil
 	}
@@ -67,9 +72,12 @@ func (w *Watcher) Close() error {
 
 // Add starts watching the named file or directory (non-recursively).
 func (w *Watcher) Add(name string) error {
+	w.mu.Lock()
 	if w.isClosed {
 		return errors.New("watcher already closed")
 	}
+	w.mu.Unlock()
+
 	in := &input{
 		op:    opAddWatch,
 		path:  filepath.Clean(name),
@@ -188,8 +196,10 @@ type watch struct {
 	buf    [4096]byte
 }
 
-type indexMap map[uint64]*watch
-type watchMap map[uint32]indexMap
+type (
+	indexMap map[uint64]*watch
+	watchMap map[uint32]indexMap
+)
 
 func (w *Watcher) wakeupReader() error {
 	e := syscall.PostQueuedCompletionStatus(w.port, 0, 0, nil)
@@ -316,7 +326,7 @@ func (w *Watcher) remWatch(pathname string) error {
 	watch := w.watches.get(ino)
 	w.mu.Unlock()
 	if watch == nil {
-		return fmt.Errorf("can't remove non-existent watch for: %s", pathname)
+		return fmt.Errorf("%w: %s", ErrNonExistentWatch, pathname)
 	}
 	if pathname == dir {
 		w.sendEvent(watch.path, watch.mask&sysFSIGNORED)


Test patch:
diff --git a/inotify_test.go b/inotify_test.go
index bb6db09..728f5c2 100644
--- a/inotify_test.go
+++ b/inotify_test.go
@@ -8,6 +8,7 @@
 package fsnotify
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -303,6 +304,8 @@ func TestInotifyRemoveTwice(t *testing.T) {
 	err = w.Remove(testFile)
 	if err == nil {
 		t.Fatalf("no error on removing invalid file")
+	} else if !errors.Is(err, ErrNonExistentWatch) {
+		t.Fatalf("unexpected error %v on removing invalid file", err)
 	}
 
 	w.mu.Lock()
@@ -384,7 +387,7 @@ func TestInotifyOverflow(t *testing.T) {
 	for dn := 0; dn < numDirs; dn++ {
 		testSubdir := fmt.Sprintf("%s/%d", testDir, dn)
 
-		err := os.Mkdir(testSubdir, 0777)
+		err := os.Mkdir(testSubdir, 0o777)
 		if err != nil {
 			t.Fatalf("Cannot create subdir: %v", err)
 		}