Depth parameter is not working properly

[aleksey-hoffman]

Issue 1:

I found a few more issues with depth parameter values:

1. When you set depth > 4 and recursive: true, no events will be emitted
2. When you set depth <= 4 and recursive: true, events emitted properly, though it doesn't make much sense logically, since this setup emits events up to Infinity, even though you specify level 4 as the limit. Perhaps it should throw an error if recursive and depth are both used at the same time?
3. When you set recursive: true without specifying depth, no events will be emitted
4. When you set recursive: true and depth: Infinity, no events will be emitted

I'm not sure what's the intended behavior.

For the following setup:

{
  depth: 10,
  recursive: true
}

I'd expected the following behavior:

limits the amount of nested levels being watched to level 10 on Linux (for performance), while watching up to Infinity on Win and MacOS using the native watchers. Though, I don't really understand the purpose of the recursive option - if its only purpose is to switch to the native watcher, why not call it useNativeWatcher instead?

Issue 2:

Also there's a separate depth problem - when you init the watcher for a drive root path (e.g. "E:/") no events are emitted as well.

[fabiospampinato]

When you set depth > 4 and recursive: true, no events will be emitted

Does it work when depth is set to 3 for example?

Perhaps it should throw an error if recursive and depth are both used at the same time?

Maybe it should 🤔

When you set recursive: true without specifying depth, no events will be emitted

It might be the same issue as 1.

When you set recursive: true and depth: Infinity, no events will be emitted

Still the same issue maybe.

limits the amount of nested levels being watched to level 10 on Linux (for performance), while watching up to Infinity on Win and MacOS using the native watchers.

Maybe it should throw instead because you didn't pass it the "native: false" option so the library can't implement watching consistently across systems, I'm not sure how much sense it makes to have it behave differently on different systems, the whole point is to abstract all the differences away and just work.

Though, I don't really understand the purpose of the recursive option

You don't always want to watch recursively. For example you might be interested in listening for when "the parent folder of this directory gets deleted", but you don't want to watch everything below that too, just the folder.

if its only purpose is to switch to the native watcher, why not call it useNativeWatcher instead?

It doesn't switch on or off to the native recursive watcher, there's the "native" option for that.

Also there's a separate depth problem - when you init the watcher for a drive root path (e.g. "E:/") no events are emitted as well.

At what depth are you watching and under what OS? Are you ignoring initial events? Are you getting the "ready" event?

[aleksey-hoffman]

@fabiospampinato

At what depth are you watching and under what OS? Are you ignoring initial events? Are you getting the "ready" event?

OS: Windows 10; depth: < 5. Tried with ignoreInitial set to true and false, the same result.

And, as I mentioned, you always have to specify depth: < 5 for some reason, otherwise the watcher is not working at all, not even for non-root directories (the ready event will never be emitted either).

const Watcher = require('watcher')

let dirWatcher = new Watcher('E:/', { 
  ignoreInitial: true, 
  depth: 2,
  recursive: true
})
console.log('Watcher: init', dirWatcher)

dirWatcher.on('all', (event, targetPath, targetPathNext) => {
  console.log('Watcher: all', event, targetPath, targetPathNext)
})
dirWatcher.on('ready', (event, targetPath, targetPathNext) => {
  console.log('Watcher: ready', event, targetPath, targetPathNext)

})
dirWatcher.on('error', (error) => {
  console.log('Watcher: error', error)
})

Log:
// Watcher: init {Object}
// Watcher: ready undefined undefined undefined

When I modify any item inside E:/ nothing happens.

[fabiospampinato]

I'm aware of a similar bug existing in the library at the moment, but in your situation if it works by setting depth: 4 and it breaks by setting depth: 5 then most probably you are just instructing the watcher to watch on too many things. You should profile the app to be sure, but my guess is that the initial directory scan which completes in ~reasonable time at depth 4 or below just takes too long at depth 5, the number of directories to scan can grow exponentially.

I'm not sure this is something the library itself can address, unless there's some major performance issue somewhere causing this problem. You should probably adopt a different watching strategy, and potentially turn off the native recursive watcher (native: false).

[aleksey-hoffman]

@fabiospampinato yeah, it looks like it's breaking when you try watching directories containing a lot of paths. That's why it doesn't work for drive root directories (e.g. E:/) and for dirs with depth; 5.

I've just created E:/test directory with only 150 nested paths, and the watcher works fine even on depth 5+.
Where is this limitation of max amount of paths coming from? Node?

The strange thing is. when I init the watcher for a dir that has 100,000 paths (8 GB), the CPU usage stayed at 0 the whole time. It didn't look like it was scanning anything.

[fabiospampinato]

Where is this limitation of max amount of paths coming from? Node?

If you don't set native: false then you are using Node's wrapper over Windows' native recursive filesystem watcher, I've no idea what the actual limitations of that are.

The strange thing is. when I init the watcher for a dir that has 100,000 paths (8 GB), the CPU usage stayed at 0 the whole time. It didn't look like it was scanning anything.

Maybe it was largely just waiting for the filesystem to respond? Scanning isn't particularly expensive on Node's side, it's mostly just waiting for the responses I think. A profile of the app would be necessary to better understand where the time is being spent.

[aleksey-hoffman]

Yeah, I also think the native watcher just waits for the event from OS. So in theory it should emit events no matter how deep the watched directory is. Looks like it just doesn't work on deeply nested dirs at all (including the drive root dirs like E:/).

Just tested with native: false it also gives up on deeply nested dirs. The node.js process closes completely when I try to watch deeply nested dirs

[fabiospampinato]

Just tested with native: false it also gives up on deeply nested dirs. The node.js process closes completely when I try to watch deeply nested dirs

Is it just crashing for you? Do you know the exit code?

[aleksey-hoffman]

Yeah, it just crashes the process, error listener is not emitting any errors
I'm executing a js file from terminal: node test.js

[fabiospampinato]

@aleksey-hoffman if you can make a repro I'll look into this, I'm not sure how to reproduce it.