panic in modd on OSX watching file in SMB volume

[fredcy]

As reported in https://github.com/cortesi/modd/issues/22 I get a panic from the notify code in modd when I watch a file in OSX on a SMB volume (mounted from an NFS server). The same modd never panics when watching files on the local disk of the OSX machine.

OSX 10.10.5. modd 0.3 built with Go 1.6.

09:06:03: prep: make
make: `process.ps' is up to date.
>> done (4.173ms)
panic: unhandled error: notify.FSEventsMustScanSubDirs|notify.FSEventsIsDir|notify.FSEventsInodeMetaMod

goroutine 18 [running]:
panic(0x422e4e0, 0xc82000ab00)
    /usr/local/go/src/runtime/panic.go:464 +0x3e6
github.com/cortesi/modd/vendor/github.com/rjeczalik/notify.(*watch).Dispatch(0xc820096580, 0xc820148980, 0x2, 0x2)
    /Users/fcy/go/src/github.com/cortesi/modd/vendor/github.com/rjeczalik/notify/watcher_fsevents.go:136 +0x54f
github.com/cortesi/modd/vendor/github.com/rjeczalik/notify.(*watch).Dispatch-fm(0xc820148980, 0x2, 0x2)
    /Users/fcy/go/src/github.com/cortesi/modd/vendor/github.com/rjeczalik/notify/watcher_fsevents.go:205 +0x3e
github.com/cortesi/modd/vendor/github.com/rjeczalik/notify.gostream(0x4b00ba0, 0x1, 0x2, 0x4b00d90, 0x47ff000, 0x47fe000)
    /Users/fcy/go/src/github.com/cortesi/modd/vendor/github.com/rjeczalik/notify/watcher_fsevents_cgo.go:104 +0x11c
github.com/cortesi/modd/vendor/github.com/rjeczalik/notify._cgoexpwrap_b138320e5461_gostream(0x4b00ba0, 0x1, 0x2, 0x4b00d90, 0x47ff000, 0x47fe000)
    ??:0 +0x53
github.com/cortesi/modd/vendor/github.com/rjeczalik/notify._Cfunc_CFRunLoopRun()
    ??:0 +0x36
github.com/cortesi/modd/vendor/github.com/rjeczalik/notify.init.1.func1()
    /Users/fcy/go/src/github.com/cortesi/modd/vendor/github.com/rjeczalik/notify/watcher_fsevents_cgo.go:68 +0x112
created by github.com/cortesi/modd/vendor/github.com/rjeczalik/notify.init.1
    /Users/fcy/go/src/github.com/cortesi/modd/vendor/github.com/rjeczalik/notify/watcher_fsevents_cgo.go:70 +0x48

[rjeczalik]

notify.FSEventsMustScanSubDirs

@fredcy It basically means the event was dropped according to the FSEvents documentation, I believe it should also send kFSEventStreamEventFlagKernelDropped however it's missing here. I'm not sure FSEvents works on anything other than HFS+ volumes, which means e.g. USB drives mounted under /Volumes/ won't work as well.

Out of curiosity, does .fseventd directory exist on your SMB volume?

[fredcy]

I don't see any .fseventd directory either in the directory I was working in or in the SMB volume root or in any directory in between those two.

[fjl]

A similar crash has been reported with our application. There is no way for us to know in advance whether the directory chosen by a user is located on a compatible file system. Why is this a panic? Panicking in a background goroutine because of a harmless condition is wrong.

There should be a better way to handle this. One way I could think of would be to introduce an Error event type. I can send you a PR to send errors from the RunLoop as events if you're interested in this.

[rjeczalik]

@fjl The panic there was mostly to catch edge-cases like this, sorry you guys were the ones to be affected. The panic should be gone, there's no sense to keep it.

I think the proper solution would be to make the Watch fail if path is on incompatible filesystem - check filesystem type and fail if != HFS+ - I don't know off-hand what syscall to use.

I know that error event would be easy to implement, but I don't think it is suitable API, to have 2 ways of reporting Watch failures.

[fjl]

I still think an Error event is superior. Let me illustrate how we use notify so you can understand why:

We use notify to watch a single directory for changes. Changes in the directory invalidate a cache. If notify.Watch fails, we fall back to polling (but poll only if the user actually needs information about the files). This strategy works if Watch can detect that notifications are unavailable in all possible cases. However, if an error occurs inside package notify that prevents further notifications, our app will present outdated results to the user because it cannot learn about the error. We could solve this by polling always but that kind of kills the benefit of using notify.

It would be nice to fail Watch immediately for incompatible file systems, but there are other errors that could occur and there is no outlet for them. If you don't like having two outlets you could also remove the error from Wait and return all errors as events. Looking at the source, there are many errors which package notify can effectively not handle because there is nowhere to send them to.