Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deleting an empty directory triggers error instead of unlinkDir #566

Closed
npetruzzelli opened this issue Dec 29, 2016 · 32 comments
Closed

Deleting an empty directory triggers error instead of unlinkDir #566

npetruzzelli opened this issue Dec 29, 2016 · 32 comments
Labels

Comments

@npetruzzelli
Copy link

When deleting an empty directory, I get an unexpected error event. If the same directory has any kind of content (other than empty directories), the expected unlinkDir event is triggered without an error.

  • OS: Windows 10 Pro
  • node: 4.3.1
  • npm: 3.7.3
  • I'm using glob-watcher (3.0.0), which gives me a chokidar (1.6.1) instance.

The console output:

events.js:141
      throw er; // Unhandled 'error' event
      ^

Error: watch null EPERM
    at exports._errnoException (util.js:870:11)
    at FSEvent.FSWatcher._handle.onchange (fs.js:1217:21)

The directory is deleted by selecting it in Windows Explorer and pressing delete.

Is this a bug? or the expected behavior I need to account for with the error event?

@es128
Copy link
Contributor

es128 commented Dec 29, 2016

What gets emitted on the raw event in this scenario?

@es128
Copy link
Contributor

es128 commented Dec 29, 2016

Also, is the watcher directly on this directory or a parent? Are you using glob patterns?

@npetruzzelli
Copy link
Author

var watcher = globWatcher('local_packages/**/*');
watcher.on('raw', function(event, path, details){
    console.log('Raw event info:', event, path, details);
});
// => Raw event info: rename New folder { watchedPath: 'local_packages' }

@npetruzzelli
Copy link
Author

New folder is a child of local_packages

@es128
Copy link
Contributor

es128 commented Dec 29, 2016

And if you add a noop error event listener, you still do not get an unlinkDir event? Or you just aren't seeing it because the lack of an error listener causes node to throw?

@es128
Copy link
Contributor

es128 commented Dec 29, 2016

The code at

watcher.on('error', function(error) {
// Workaround for https://github.com/joyent/node/issues/4337
if (process.platform === 'win32' && error.code === 'EPERM') {
fs.open(path, 'r', function(err, fd) {
if (fd) fs.close(fd);
if (!err) broadcastErr(error);
});
} else {
broadcastErr(error);
}
});
is intended to suppress the sort of error you're describing, so we have to narrow down where it's actually coming from.

@npetruzzelli
Copy link
Author

The noop prevents the error from throwing, but the error still happens (confirmed by logging it). the unlinkDir event still does not happen.

@es128
Copy link
Contributor

es128 commented Dec 29, 2016

How does it behave with the ignorePermissionErrors: true option?

@npetruzzelli
Copy link
Author

No error event, but also no unlinkDir event. Raw output hasn't changed.

@es128
Copy link
Contributor

es128 commented Dec 29, 2016

It does seem like a bug, for which a PR is welcome.

Unfortunately I do not have a Windows environment presently available with which to try to reproduce and debug this.

@npetruzzelli
Copy link
Author

I don't know if I have the time/skill to solve it, but I will at least give it a try starting with the code you highlighted. What I am working on is a personal project, which unfortunately is lower on the priority list.

Alternatively, there are some free virtual machines available, but I don't know if they have enough capabilities for node as they are meant for browser testing: https://developer.microsoft.com/en-us/microsoft-edge/tools/vms/ . It might be worth a shot, depending on your host OS.

@es128
Copy link
Contributor

es128 commented Dec 29, 2016

Yes, I've used stuff like that before, but working with them is kind of a hassle, and I'm at a point where I'd rather encourage more community participation.

@npetruzzelli
Copy link
Author

if (!err) broadcastErr(error);

// => fd === 3;
// => err === null

Additional References:

I've run into something that is even more confusing. I've added a couple log statements.

    watcher.on('error', function(error) {
// console.log('   ERROR:', error);
      // Workaround for https://github.com/joyent/node/issues/4337
      if (process.platform === 'win32' && error.code === 'EPERM') {
        fs.open(path, 'r', function(err, fd) {
console.log('     ERR:', err);
          if (fd) fs.close(fd);
          if (!err) broadcastErr(error);
        });
      } else {
        broadcastErr(error);
      }
    });

As is we get the following:

     ERR: null

but simply un-commenting the first console.log statement causes a different result:

   ERROR: { [Error: watch null EPERM]
  code: 'EPERM',
  errno: 'EPERM',
  syscall: 'watch null',
  filename: null }
Raw event info: rename New folder { watchedPath: 'local_packages' }
     ERR: { [Error: ENOENT: no such file or directory, open 'P:\projects\project-name\local_packages\New folder']
  errno: -4058,
  code: 'ENOENT',
  syscall: 'open',
  path: 'P:\\projects\\project-name\\local_packages\\New folder' }

Since err is not null, the error event isn't triggered. That still leaves unlinkDir not being triggered. This behavior is just bizarre. Simply logging error shouldn't change err unless the difference in milliseconds it takes to output the log allowed something to finish on the file system.

console.log( fs.statSync(path).isDirectory() );
        fs.open(path, 'r', function(err, fd) {

// => true

Using the async version (fs.stat(path)) also returns true. For whatever reason, it looks like it isn't done being deleted, despite the filename given to the error (in fs.js) being null. Could this be used to our advantage in this case? if err == null && stat.isDirectory() === true then trigger the unlinkDir event? Or could this be too unreliable?

This is about as far as I can get for now. If I can free up more time, I'll try doing more digging, but it may well be above my current skill level.

@ScorpioCoding
Copy link

ScorpioCoding commented Mar 7, 2017

Got The Same issue

What versions of npm and node are you using?

node v7.7.1
npm 4.1.2

What OS are you using?

Microsoft Windows 10 Pro x64
Version	10.0.14393 Build 14393

My gulpfile.js code

var gulp = require('gulp');
var chokidar = require('chokidar');

gulp.task('event', function(){
  var watcher = chokidar.watch('src/**/*',{
    ignored: /(^|[\/\\])\../,
    persistent: true
  });
  var log = console.log.bind(console);
  watcher
    .on('add', path => log(`File ${path} has been added`))
    .on('change', path => log(`File ${path} has been changed`))
    .on('unlink', path => log(`File ${path} has been removed`))
    .on('addDir', path => log(`Directory ${path} has been added`))
    .on('unlinkDir', path => log(`Directory ${path} has been removed`))
    .on('error', error => log(`Watcher error: ${error}`))
    .on('ready', () => log('Initial scan complete. Ready for changes'))
    .on('raw', (event, path, details) => {
      log('Raw event info:', event, path, details);
    });
});

gulp.task('default', ['event']);

Testing

Testing in different Editors and using windows explore

.Atom -v1.14.4 x64
.VScode -v1.10.1

Testing sequence of events
So in my root directory I have the directory "src" upon the watcher is watching for any changes
Then I add the folder 'jump' and in my powershell I get the following response

Initial scan complete. Ready for changes
Raw event info: rename jump { watchedPath: 'src' }
Directory src\jump has been added

So when I delete the folder 'jump' the response is

Initial scan complete. Ready for changes
Raw event info: rename jump { watchedPath: 'src' }
Directory src\jump has been added
Watcher error: Error: Error watching file for changes: EPERM
Raw event info: rename jump { watchedPath: 'src' }

Testing without the .on('raw') section gives the following error

Error: Error watching file for changes: EPERM
at exports._errnoException (util.js:1029:11)
at FSEvent.FSWatcher._handle.onchange (fs.js:1375:11)

Questions

  1. What is wrong ?
  2. What am I doing wrong ?
  3. What can I do about it?
  4. Can someone fix my code sample so I can see what I 'm doing wrong and learn from it?

@AndyOGo
Copy link

AndyOGo commented Apr 2, 2017

I'm using chokidar and it emits the following EPERM error each time I delete an empty directory on windows 10.

Error watching file for changes: EPERM

I use chokidar 1.6.1

@es128
Copy link
Contributor

es128 commented Apr 3, 2017

PR welcome for trapping and handling the EPERM error as it comes out of the fs.watch() handle.

When detected, something like running NodeFsHandler.prototype._handleDir() on the parent might work to recalibrate and emit the appropriate events.

@AndyOGo
Copy link

AndyOGo commented Apr 11, 2017

@es128
Thanks for your quick reply.
I'm sorry for my late answer, I'm very busy atm and I wanted to supply a full failing test.

I'm the author of node-sync-glob which uses awesome chokidar under the hood.
Yep, I have usePolling: true enabled.

So I prepared my lib and created a dedicated reproducible test case regarding of deleting empty sub directories.

It first creates an empty folder tmp/mock/bar/empty then it copies all contents of tmp/mock to tmp/sync. After this mirror is done tmp/mock/bar/empty gets deleted (at least it is supposed to).

TLDR:

Build Matrices

  • OS: MacOS, Linux, Windows (x86, x64)
  • Node Version: Stable, 6, 5, 4
OS Result CI Build Notice
MacOS ✔️ Travis EXISTS: tmp/mock/bar -> true
Linux Travis EXISTS: tmp/mock/bar -> true 
Windows AppVeyor EXISTS: tmp/mock/bar -> false

MacOS Logs

Start at https://travis-ci.org/AndyOGo/node-sync-glob/jobs/220915576#L952

$ node --version
v7.8.0

$ npm --version
4.2.0

$ npm run test

> sync-glob@1.3.7 test /Users/travis/build/AndyOGo/node-sync-glob
> jest --runInBand --verbose --no-cache --config ./.jestrc.json

 PASS  test/lib/is-glob.spec.js
  lib/is-glob
    truthy
      ✓ should match asterisk wildcard (4ms)
      ✓ should match globstar wildcard (1ms)
      ✓ should match question mark wildcard (2ms)
      ✓ should match braced sections
      ✓ should match range wildcards (2ms)
      ✓ should match extended globs (1ms)

    falsy
      ✓ should not match escaped asterisks
      ✓ should not match escaped globstar (1ms)
      ✓ should not match escaped question mark
      ✓ should not match escaped braced sections (1ms)
      ✓ should not match escaped range wildcards
      ✓ should not match escaped extended globs (1ms)
      ✓ should not match non-globs

 PASS  test/copy.spec.js
  node-sync-glob copy
    ✓ should copy a file (49ms)
    ✓ should copy an array of files (22ms)
    ✓ should copy a directory (without contents) (85ms)
    ✓ should copy globs (53ms)
    ✓ should copy globstar (165ms)
    ✓ should copy empty sub directories (83ms)
    ○ skipped 1 test

console.log test/sync.spec.js:99
    EXISTS: tmp/mock/bar -> true

console.log src/index.js:102
    sources: tmp/mock/**/* -> tmp/mock/**/*

console.log src/index.js:103
    target: tmp/sync -> tmp/sync

console.log src/index.js:104
    globed files: 
    	tmp/mock/@org
    	tmp/mock/@org/a.txt
    	tmp/mock/@org/b.txt
    	tmp/mock/@org/d.txt
    	tmp/mock/a.txt
    	tmp/mock/b.txt
    	tmp/mock/bar
    	tmp/mock/bar/c.txt
    	tmp/mock/bar/empty
    	tmp/mock/foo
    	tmp/mock/foo/b.txt
    	tmp/mock/foo/d.txt
    	tmp/mock/transform.js

 PASS  test/sync.spec.js (10.163s)
  node-sync-glob watch
    ✓ should sync a file (2358ms)
    ✓ should sync an array of files (2510ms)
    ✓ should sync a directory (2439ms)
    ✓ should sync globstar (2420ms)
    ✓ should sync empty sub directory deletion (207ms)

console.log src/index.js:242
    RAW: deleted -> /Users/travis/build/AndyOGo/node-sync-glob/tmp/mock/bar/empty 	
    { path: '/Users/travis/build/AndyOGo/node-sync-glob/tmp/mock/bar/empty',
      event: 'deleted',
      type: 'directory',
      changes: { inode: false, finder: false, access: false, xattrs: false },
      flags: 131840 }

console.log src/index.js:190
    ALL: unlinkDir -> tmp/mock/bar/empty 

 PASS  test/lib/sources-bases.spec.js
  lib/sources-bases
    ✓ should resolve the base path of globs (8ms)
    ✓ should resolve the base path of files (1ms)
    ✓ should leave directories unchanged (1ms)
    ✓ should ignore exclude patterns
    ✓ should list multiple distinct base paths
    ✓ should list common base baths no more than once (1ms)

 PASS  test/lib/resolve-target.spec.js
  lib/resolve-target
    ✓ should resolve targets (6ms)

 PASS  test/transform.spec.js
  node-sync-glob transform
    ✓ should transform a file (65ms)

console.log test/mock/transform.js:3
    transform tmp/trans/b.txt

 PASS  test/lib/trim-quotes.spec.js
  lib/trim-quotes
    ✓ should trim single quotes (2ms)
    ✓ should trim double quotes
    ✓ should not remove quotes within text (1ms)

Test Suites: 7 passed, 7 total
Tests:       1 skipped, 35 passed, 36 total
Snapshots:   0 total
Time:        14.348s

Ran all test suites.

Linux Logs

Start at https://travis-ci.org/AndyOGo/node-sync-glob/jobs/220915572#L918

$ node --version
v7.8.0

$ npm --version
4.2.0

$ npm run test

> sync-glob@1.3.7 test /home/travis/build/AndyOGo/node-sync-glob
> jest --runInBand --verbose --no-cache --config ./.jestrc.json

 PASS  test/lib/is-glob.spec.js
  lib/is-glob
    truthy
      ✓ should match asterisk wildcard (4ms)
      ✓ should match globstar wildcard (1ms)
      ✓ should match question mark wildcard (2ms)
      ✓ should match braced sections
      ✓ should match range wildcards (2ms)
      ✓ should match extended globs (1ms)

    falsy
      ✓ should not match escaped asterisks (1ms)
      ✓ should not match escaped globstar
      ✓ should not match escaped question mark (1ms)
      ✓ should not match escaped braced sections
      ✓ should not match escaped range wildcards (1ms)
      ✓ should not match escaped extended globs
      ✓ should not match non-globs (1ms)

 PASS  test/copy.spec.js
  node-sync-glob copy
    ✓ should copy a file (32ms)
    ✓ should copy an array of files (9ms)
    ✓ should copy a directory (without contents) (26ms)
    ✓ should copy globs (16ms)
    ✓ should copy globstar (19ms)
    ✓ should copy empty sub directories (20ms)
    ○ skipped 1 test

console.log test/sync.spec.js:99
    EXISTS: tmp/mock/bar -> true

console.log src/index.js:102
    sources: tmp/mock/**/* -> tmp/mock/**/*

console.log src/index.js:103
    target: tmp/sync -> tmp/sync

console.log src/index.js:104
    globed files: 
    	tmp/mock/@org
    	tmp/mock/@org/a.txt
    	tmp/mock/@org/b.txt
    	tmp/mock/@org/d.txt
    	tmp/mock/a.txt
    	tmp/mock/b.txt
    	tmp/mock/bar
    	tmp/mock/bar/c.txt
    	tmp/mock/bar/empty
    	tmp/mock/foo
    	tmp/mock/foo/b.txt
    	tmp/mock/foo/d.txt
    	tmp/mock/transform.js

console.log src/index.js:242
    RAW: rename -> empty 	
    { watchedPath: 'tmp/mock/bar/empty' }

console.log src/index.js:242
    RAW: rename -> empty 	
    { watchedPath: 'tmp/mock/bar/empty' }

console.log src/index.js:242
    RAW: rename -> empty 	
    { watchedPath: 'tmp/mock/bar' }

 FAIL  test/sync.spec.js (13.696s)
  ● node-sync-glob watch › should sync empty sub directory deletion
    Timeout - Async callback was not invoked within timeout specified by jasmine.DEFAULT_TIMEOUT_INTERVAL.
      at ontimeout (timers.js:386:14)
      at tryOnTimeout (timers.js:250:5)
      at Timer.listOnTimeout (timers.js:214:5)

  node-sync-glob watch
    ✓ should sync a file (2135ms)
    ✓ should sync an array of files (2125ms)
    ✓ should sync a directory (2153ms)
    ✓ should sync globstar (2149ms)
    ✕ should sync empty sub directory deletion (5008ms)

console.log src/index.js:242
    RAW: change -> a.txt 	
    { watchedPath: 'tmp/mock/@org/a.txt' }

console.log src/index.js:242
    RAW: rename -> a.txt 	
    { watchedPath: 'tmp/mock/@org/a.txt' }

console.log src/index.js:242
    RAW: rename -> a.txt 	
    { watchedPath: 'tmp/mock/@org/a.txt' }

console.log src/index.js:242
    RAW: rename -> a.txt 	
    { watchedPath: 'tmp/mock/@org' }

console.log src/index.js:242
    RAW: change -> b.txt 	
    { watchedPath: 'tmp/mock/@org/b.txt' }

console.log src/index.js:242
    RAW: rename -> b.txt 	
    { watchedPath: 'tmp/mock/@org/b.txt' }

console.log src/index.js:242
    RAW: rename -> b.txt 	
    { watchedPath: 'tmp/mock/@org/b.txt' }

console.log src/index.js:242
    RAW: rename -> b.txt 	
    { watchedPath: 'tmp/mock/@org' }

console.log src/index.js:242
    RAW: change -> d.txt 	
    { watchedPath: 'tmp/mock/@org/d.txt' }

console.log src/index.js:242
    RAW: rename -> d.txt 	
    { watchedPath: 'tmp/mock/@org/d.txt' }

console.log src/index.js:242
    RAW: rename -> d.txt 	
    { watchedPath: 'tmp/mock/@org/d.txt' }

console.log src/index.js:242
    RAW: rename -> d.txt 	
    { watchedPath: 'tmp/mock/@org' }

console.log src/index.js:242
    RAW: rename -> @org 	
    { watchedPath: 'tmp/mock/@org' }

console.log src/index.js:242
    RAW: rename -> @org 	
    { watchedPath: 'tmp/mock/@org' }

console.log src/index.js:242
    RAW: rename -> @org 	
    { watchedPath: 'tmp/mock' }

console.log src/index.js:242
    RAW: change -> a.txt 	
    { watchedPath: 'tmp/mock/a.txt' }

console.log src/index.js:242
    RAW: rename -> a.txt 	
    { watchedPath: 'tmp/mock/a.txt' }

console.log src/index.js:242
    RAW: rename -> a.txt 	
    { watchedPath: 'tmp/mock/a.txt' }

console.log src/index.js:242
    RAW: rename -> a.txt 	
    { watchedPath: 'tmp/mock' }

console.log src/index.js:242
    RAW: change -> b.txt 	
    { watchedPath: 'tmp/mock/b.txt' }

console.log src/index.js:242
    RAW: rename -> b.txt 	
    { watchedPath: 'tmp/mock/b.txt' }

console.log src/index.js:242
    RAW: rename -> b.txt 	
    { watchedPath: 'tmp/mock/b.txt' }

console.log src/index.js:242
    RAW: rename -> b.txt 	
    { watchedPath: 'tmp/mock' }

console.log src/index.js:242
    RAW: change -> c.txt 	
    { watchedPath: 'tmp/mock/bar/c.txt' }

console.log src/index.js:242
    RAW: rename -> c.txt 	
    { watchedPath: 'tmp/mock/bar/c.txt' }

console.log src/index.js:242
    RAW: rename -> c.txt 	
    { watchedPath: 'tmp/mock/bar/c.txt' }

console.log src/index.js:242
    RAW: rename -> c.txt 	
    { watchedPath: 'tmp/mock/bar' }

console.log src/index.js:242
    RAW: rename -> bar 	
    { watchedPath: 'tmp/mock/bar' }

console.log src/index.js:242
    RAW: rename -> bar 	
    { watchedPath: 'tmp/mock/bar' }

console.log src/index.js:242
    RAW: rename -> bar 	
    { watchedPath: 'tmp/mock' }

console.log src/index.js:242
    RAW: change -> b.txt 	
    { watchedPath: 'tmp/mock/foo/b.txt' }

console.log src/index.js:242
    RAW: rename -> b.txt 	
    { watchedPath: 'tmp/mock/foo/b.txt' }

console.log src/index.js:242
    RAW: rename -> b.txt 	
    { watchedPath: 'tmp/mock/foo/b.txt' }

console.log src/index.js:242
    RAW: rename -> b.txt 	
    { watchedPath: 'tmp/mock/foo' }

console.log src/index.js:242
    RAW: change -> d.txt 	
    { watchedPath: 'tmp/mock/foo/d.txt' }

console.log src/index.js:242
    RAW: rename -> d.txt 	
    { watchedPath: 'tmp/mock/foo/d.txt' }

console.log src/index.js:242
    RAW: rename -> d.txt 	
    { watchedPath: 'tmp/mock/foo/d.txt' }

console.log src/index.js:242
    RAW: rename -> d.txt 	
    { watchedPath: 'tmp/mock/foo' }

console.log src/index.js:242
    RAW: rename -> foo 	
    { watchedPath: 'tmp/mock/foo' }

console.log src/index.js:242
    RAW: rename -> foo 	
    { watchedPath: 'tmp/mock/foo' }

console.log src/index.js:242
    RAW: rename -> foo 	
    { watchedPath: 'tmp/mock' }

console.log src/index.js:242
    RAW: change -> transform.js 	
    { watchedPath: 'tmp/mock/transform.js' }

console.log src/index.js:242
    RAW: rename -> transform.js 	
    { watchedPath: 'tmp/mock/transform.js' }

console.log src/index.js:242
    RAW: rename -> transform.js 	
    { watchedPath: 'tmp/mock/transform.js' }

console.log src/index.js:242
    RAW: rename -> transform.js 	
    { watchedPath: 'tmp/mock' }

console.log src/index.js:242
    RAW: rename -> mock 	
    { watchedPath: 'tmp/mock' }

console.log src/index.js:242
    RAW: rename -> mock 	
    { watchedPath: 'tmp/mock' }

 PASS  test/lib/sources-bases.spec.js
  lib/sources-bases
    ✓ should resolve the base path of globs (8ms)
    ✓ should resolve the base path of files (1ms)
    ✓ should leave directories unchanged (1ms)
    ✓ should ignore exclude patterns
    ✓ should list multiple distinct base paths (1ms)
    ✓ should list common base baths no more than once (1ms)

 PASS  test/lib/resolve-target.spec.js
  lib/resolve-target
    ✓ should resolve targets (4ms)

console.log src/index.js:190
    ALL: unlink -> tmp/mock/@org/a.txt 

console.log src/index.js:190
    ALL: unlink -> tmp/mock/@org/b.txt 

console.log src/index.js:190
    ALL: unlink -> tmp/mock/@org/d.txt 

console.log src/index.js:190
    ALL: unlink -> tmp/mock/a.txt 

console.log src/index.js:190
    ALL: unlink -> tmp/mock/b.txt 

console.log src/index.js:190
    ALL: unlink -> tmp/mock/bar/c.txt 

console.log src/index.js:190
    ALL: unlink -> tmp/mock/foo/b.txt 

console.log src/index.js:190
    ALL: unlink -> tmp/mock/foo/d.txt 

console.log src/index.js:190
    ALL: unlink -> tmp/mock/transform.js 

console.log src/index.js:190
    ALL: unlinkDir -> /home/travis/build/AndyOGo/node-sync-glob/tmp/mock/@org 

console.log src/index.js:190
    ALL: unlinkDir -> /home/travis/build/AndyOGo/node-sync-glob/tmp/mock/foo 

console.error test/helpers.js:77
    error -> Error: expect(received).toBe(expected)

    Expected value to be (using ===):
      true
    Received:
      false

console.log test/helpers.js:80
    Error: expect(received).toBe(expected)

    Expected value to be (using ===):
      true
    Received:
      false
        at /home/travis/build/AndyOGo/node-sync-glob/test/sync.spec.js:112:89
        at /home/travis/build/AndyOGo/node-sync-glob/test/helpers.js:100:7
        at /home/travis/build/AndyOGo/node-sync-glob/src/index.js:222:9
        at tryCatcher (/home/travis/build/AndyOGo/node-sync-glob/node_modules/bluebird/js/release/util.js:16:23)
        at Promise._settlePromiseFromHandler (/home/travis/build/AndyOGo/node-sync-glob/node_modules/bluebird/js/release/promise.js:512:31)
        at Promise._settlePromise (/home/travis/build/AndyOGo/node-sync-glob/node_modules/bluebird/js/release/promise.js:569:18)
        at Promise._settlePromise0 (/home/travis/build/AndyOGo/node-sync-glob/node_modules/bluebird/js/release/promise.js:614:10)
        at Promise._settlePromises (/home/travis/build/AndyOGo/node-sync-glob/node_modules/bluebird/js/release/promise.js:693:18)
        at Promise._fulfill (/home/travis/build/AndyOGo/node-sync-glob/node_modules/bluebird/js/release/promise.js:638:18)
        at /home/travis/build/AndyOGo/node-sync-glob/node_modules/bluebird/js/release/nodeback.js:42:21
        at CB (/home/travis/build/AndyOGo/node-sync-glob/node_modules/fs-extra/lib/remove/rimraf.js:57:5)
        at options.lstat (/home/travis/build/AndyOGo/node-sync-glob/node_modules/fs-extra/lib/remove/rimraf.js:81:14)
        at /home/travis/build/AndyOGo/node-sync-glob/node_modules/graceful-fs/polyfills.js:284:29
        at /home/travis/build/AndyOGo/node-sync-glob/node_modules/graceful-fs/polyfills.js:284:29
        at FSReqWrap.oncomplete (fs.js:114:15)
console.warn node_modules/bluebird/js/release/debuggability.js:870
    Unhandled rejection TypeError: Cannot read property 'addExpectationResult' of undefined
        at Env.fail (/home/travis/build/AndyOGo/node-sync-glob/node_modules/jest-jasmine2/vendor/jasmine-2.5.2.js:1021:24)
        at fail (/home/travis/build/AndyOGo/node-sync-glob/node_modules/jest-jasmine2/vendor/jasmine-2.5.2.js:3636:23)
        at /home/travis/build/AndyOGo/node-sync-glob/test/sync.spec.js:104:9
        at /home/travis/build/AndyOGo/node-sync-glob/test/helpers.js:84:9
        at notifyError (/home/travis/build/AndyOGo/node-sync-glob/src/index.js:64:48)
        at tryCatcher (/home/travis/build/AndyOGo/node-sync-glob/node_modules/bluebird/js/release/util.js:16:23)
        at Promise._settlePromiseFromHandler (/home/travis/build/AndyOGo/node-sync-glob/node_modules/bluebird/js/release/promise.js:512:31)
        at Promise._settlePromise (/home/travis/build/AndyOGo/node-sync-glob/node_modules/bluebird/js/release/promise.js:569:18)
        at Promise._settlePromise0 (/home/travis/build/AndyOGo/node-sync-glob/node_modules/bluebird/js/release/promise.js:614:10)
        at Promise._settlePromises (/home/travis/build/AndyOGo/node-sync-glob/node_modules/bluebird/js/release/promise.js:689:18)
        at Async.Object.<anonymous>.Async._drainQueue (/home/travis/build/AndyOGo/node-sync-glob/node_modules/bluebird/js/release/async.js:133:16)
        at Async.Object.<anonymous>.Async._drainQueues (/home/travis/build/AndyOGo/node-sync-glob/node_modules/bluebird/js/release/async.js:143:10)
        at Immediate.Async.drainQueues (/home/travis/build/AndyOGo/node-sync-glob/node_modules/bluebird/js/release/async.js:17:14)
        at runCallback (timers.js:672:20)
        at tryOnImmediate (timers.js:645:5)
        at processImmediate [as _immediateCallback] (timers.js:617:5)

 PASS  test/transform.spec.js
  node-sync-glob transform
    ✓ should transform a file (23ms)

console.log test/mock/transform.js:3
    transform tmp/trans/b.txt

 PASS  test/lib/trim-quotes.spec.js
  lib/trim-quotes
    ✓ should trim single quotes
    ✓ should trim double quotes
    ✓ should not remove quotes within text (1ms)

Test Suites: 1 failed, 6 passed, 7 total
Tests:       1 failed, 1 skipped, 34 passed, 36 total
Snapshots:   0 total
Time:        16.417s

Ran all test suites.

Windows

Start at https://ci.appveyor.com/project/AndyOGo/node-sync-glob/build/1.0.107/job/wm7ualjc5nmwnlwo#L755

node --version
v7.8.0

npm --version
4.2.0

npm run test
> sync-glob@1.3.7 test C:\projects\node-sync-glob
> jest --runInBand --verbose --no-cache --config ./.jestrc.json
 PASS  test\lib\is-glob.spec.js
  lib/is-glob
    truthy
      √ should match asterisk wildcard
      √ should match globstar wildcard
      √ should match question mark wildcard
      √ should match braced sections
      √ should match range wildcards
      √ should match extended globs
    falsy
      √ should not match escaped asterisks
      √ should not match escaped globstar
      √ should not match escaped question mark
      √ should not match escaped braced sections
      √ should not match escaped range wildcards
      √ should not match escaped extended globs
      √ should not match non-globs
 PASS  test\copy.spec.js
  node-sync-glob copy
    √ should copy a file (32ms)
    √ should copy an array of files (15ms)
    √ should copy a directory (without contents) (31ms)
    √ should copy globs (16ms)
    √ should copy globstar (16ms)
    √ should copy empty sub directories (15ms)
    ○ skipped 1 test
 FAIL  test\sync.spec.js (8.941s)
  ● node-sync-glob watch › should sync empty sub directory deletion
    ENOTEMPTY: directory not empty, rmdir 'C:\projects\node-sync-glob\tmp\mock'
      
      at Object.fs.rmdirSync (fs.js:856:18)
      at rmkidsSync (node_modules\fs-extra\lib\remove\rimraf.js:292:11)
      at rmdirSync (node_modules\fs-extra\lib\remove\rimraf.js:283:7)
      at rimrafSync (node_modules\fs-extra\lib\remove\rimraf.js:252:7)
      at options.readdirSync.forEach.f (node_modules\fs-extra\lib\remove\rimraf.js:291:39)
      at Array.forEach (native)
      at rmkidsSync (node_modules\fs-extra\lib\remove\rimraf.js:291:26)
      at rmdirSync (node_modules\fs-extra\lib\remove\rimraf.js:283:7)
      at Function.rimrafSync [as sync] (node_modules\fs-extra\lib\remove\rimraf.js:252:7)
      at Object.removeSync (node_modules\fs-extra\lib\remove\index.js:6:17)
  ● node-sync-glob watch › should sync empty sub directory deletion
    EPERM: operation not permitted, mkdir 'C:\projects\node-sync-glob\tmp\mock\bar\empty'
      
      at Object.fs.mkdirSync (fs.js:895:18)
      at Object.mkdirsSync (node_modules\fs-extra\lib\mkdirs\mkdirs-sync.js:31:9)
      at Object.ensureDirSync (test\helpers.js:13:154)
      at Object.<anonymous> (test\sync.spec.js:100:45)
  node-sync-glob watch
    √ should sync a file (2183ms)
    √ should sync an array of files (2202ms)
    √ should sync a directory (2238ms)
    √ should sync globstar (2224ms)
    × should sync empty sub directory deletion
  console.log test\sync.spec.js:99
    EXISTS: tmp/mock/bar -> false
  console.log test\sync.spec.js:120
    { Error: EPERM: operation not permitted, mkdir 'C:\projects\node-sync-glob\tmp\mock\bar\empty'
        at Object.fs.mkdirSync (fs.js:895:18)
        at Object.mkdirsSync (C:\projects\node-sync-glob\node_modules\fs-extra\lib\mkdirs\mkdirs-sync.js:31:9)
        at Object.ensureDirSync (C:\projects\node-sync-glob\test\helpers.js:13:154)
        at Object.<anonymous> (C:\projects\node-sync-glob\test\sync.spec.js:100:45)
        at attemptAsync (C:\projects\node-sync-glob\node_modules\jest-jasmine2\vendor\jasmine-2.5.2.js:1984:24)
        at QueueRunner.run (C:\projects\node-sync-glob\node_modules\jest-jasmine2\vendor\jasmine-2.5.2.js:1939:9)
        at C:\projects\node-sync-glob\node_modules\jest-jasmine2\vendor\jasmine-2.5.2.js:1966:16
        at C:\projects\node-sync-glob\node_modules\jest-jasmine2\vendor\jasmine-2.5.2.js:1909:9
        at attemptAsync (C:\projects\node-sync-glob\node_modules\jest-jasmine2\vendor\jasmine-2.5.2.js:1987:9)
        at QueueRunner.run (C:\projects\node-sync-glob\node_modules\jest-jasmine2\vendor\jasmine-2.5.2.js:1939:9)
      errno: -4048,
      code: 'EPERM',
      syscall: 'mkdir',
      path: 'C:\\projects\\node-sync-glob\\tmp\\mock\\bar\\empty' }
 PASS  test\lib\sources-bases.spec.js
  lib/sources-bases
    √ should resolve the base path of globs
    √ should resolve the base path of files
    √ should leave directories unchanged
    √ should ignore exclude patterns
    √ should list multiple distinct base paths
    √ should list common base baths no more than once
 PASS  test\lib\resolve-target.spec.js
  lib/resolve-target
    √ should resolve targets
 PASS  test\transform.spec.js
  node-sync-glob transform
    √ should transform a file (15ms)
  console.log test\mock\transform.js:3
    transform tmp\trans\b.txt
 PASS  test\lib\trim-quotes.spec.js
  lib/trim-quotes
    √ should trim single quotes
    √ should trim double quotes
    √ should not remove quotes within text
Test Suites: 1 failed, 6 passed, 7 total
Tests:       1 failed, 1 skipped, 34 passed, 36 total
Snapshots:   0 total
Time:        11.363s
Ran all test suites.
npm ERR! Windows_NT 6.3.9600
npm ERR! argv "C:\\Program Files (x86)\\nodejs\\node.exe" "C:\\Program Files (x86)\\nodejs\\node_modules\\npm\\bin\\npm-cli.js" "run" "test"
npm ERR! node v7.8.0
npm ERR! npm  v4.2.0
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! sync-glob@1.3.7 test: `jest --runInBand --verbose --no-cache --config ./.jestrc.json`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the sync-glob@1.3.7 test script 'jest --runInBand --verbose --no-cache --config ./.jestrc.json'.
npm ERR! Make sure you have the latest version of node.js and npm installed.
npm ERR! If you do, this is most likely a problem with the sync-glob package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     jest --runInBand --verbose --no-cache --config ./.jestrc.json
npm ERR! You can get information on how to open an issue for this project with:
npm ERR!     npm bugs sync-glob
npm ERR! Or if that isn't available, you can get their info via:
npm ERR!     npm owner ls sync-glob
npm ERR! There is likely additional logging output above.
npm ERR! Please include the following file with any support request:
npm ERR!     C:\Users\appveyor\AppData\Roaming\npm-cache\_logs\2017-04-11T11_09_50_197Z-debug.log
Command exited with code 1

@npetruzzelli
Copy link
Author

npetruzzelli commented Apr 13, 2017

I've been able to do a little more digging. Here is what I have found.

First my environment has changed a little bit. I've upgraded to Node v6.10.2 and NPM 4.5.0. The only effect this has had is that I can't duplicate the odd behavior I previously noted by console logging both error and err. It is now consistently EPERM.

The following scenarios do not trigger an EPERM error.

  • Deleting a directory with SHIFT + DELETE (skips the recycle bin, deletes the file/directory permanently)
  • Using the command line rmdir "Directory Name" (same as SHIFT + DELETE?)
  • Using cut (CTRL + X) and paste (CTRL + V) to move the directory, even if it is out of the watched ancestor directory

Additional Notes:

This may be more of a core Node.js issue that needs to be fixed with C (C++?) rather than JavaScript.

For Chokidar's part, I haven't been able to figure out a 100% reliable work around that meets the following criteria:

  • Only ignores the error when we are 100% certain that it is an empty directory that has been moved to the Recycle Bin.
  • Correctly cleans up the internals of Chokidar
  • Correctly emits the unlinkDir event

I was stopped at the second bullet. My progress was hacky at best (manually calling watcher.unwatch() and watcher._remove()).

It didn't work quite as I had hoped because the internals of Chokidar relies on an error which is not occurring in this unique situation https://github.com/paulmillr/chokidar/blob/1.6.1/index.js#L473-L475

My code is a mess and was starting to get out of hand fast, but if your curious, here it is:

var fs = require('fs');
var path = require('path');
var globWatcher = require('glob-watcher');
var watcher = globWatcher('local_packages/**/*', {
	// ignorePermissionErrors: true
});

function createChokidarErrorHandler(chokidarInstance, options){
    options = options || {};
    var errorHandlerIsFunction = typeof options.errorHandler === 'function';
    var errorHandlerThis = options.errorHandlerThisArg || null;
    return function handleWatcherError(error){
        if(process.platform === 'win32' && error.code === 'EPERM'){
            var watcherPath = error.watcherPath;
            fs.stat(watcherPath, function(statsError, stats){
                if(!statsError && stats.isDirectory() === true){
                    fs.readdir(watcherPath, function(readdirError, filesAndFolders){
                        if(!readdirError && filesAndFolders.length === 0){
                            chokidarInstance.unwatch(watcherPath);

                            //Does unwatch do everything we want? No. `unlinkDir` still isn't triggered.
                            var watcherRemove = chokidarInstance._remove.bind(chokidarInstance);
                            watcherRemove(path.dirname(watcherPath), path.basename(watcherPath));
                        } else if(errorHandlerIsFunction){
                            options.errorHandler.call(errorHandlerThis, error);
                        } else {
                            throw error;
                        }
                    });
                } else if(errorHandlerIsFunction){
                    options.errorHandler.call(errorHandlerThis, error);
                } else {
                    throw error;
                }
            });
        } else if(errorHandlerIsFunction){
            options.errorHandler.call(errorHandlerThis, error);
        } else {
            throw error;
        }
    }
}

var fsErrorHandler = createChokidarErrorHandler(watcher);

watcher
    // .on('add', path => console.log(`File ${path} has been added`))
    // .on('change', path => console.log(`File ${path} has been changed`))
    // .on('unlink', path => console.log(`File ${path} has been removed`))
    // .on('addDir', path => console.log(`Directory ${path} has been added`))
    .on('unlinkDir', path => console.log(`Directory ${path} has been removed`))
    .on('ready', () => console.log('Initial scan complete. Ready for changes'))
    .on('raw', (event, path, details) => {
        console.log('Raw event info:', event, '::', path, '::', details);
    })
    .on('error', fsErrorHandler)
;

The same code in three places made me cringe, but since two of the the functions are async, I couldn't kick the else logic into a single place down the line. I could probably try moving part of it to a separate function and reuse that to clean it up a bit, but my code just didn't make it far enough to make taking that step worth it. Using the sync versions of the function is an option, but I don't know yet if this kind of thing should be sync or not.

To support this I had to make a small change to chokidar's code. Chokidar knows the path that we working on even if the error itself has null.

      watcher.on('error', function(error) {
++      error.watcherPath = path;
++      error.watcherFullPath = fullPath;
        // Workaround for https://github.com/joyent/node/issues/4337
        if (process.platform === 'win32' && error.code === 'EPERM') {
          fs.open(path, 'r', function(err, fd) {
            if (fd) fs.close(fd);
            if (!err) broadcastErr(error);
          });
        } else {
          broadcastErr(error);
        }
      });

An alternative recommendation I've heard is to use (from another repo pointing to nodejs/node-v0.x-archive#4337) Watchman, which I haven't explored yet. For now, I want to stick with glob-watcher/chokidar.

@npetruzzelli
Copy link
Author

Also, I think my hacky work around bypassed throttling code somewhere.

@es128
Copy link
Contributor

es128 commented Apr 13, 2017

@npetruzzelli following on from your idea of adding path information to the error object, what happens if you try something simple like this, loosely detecting the condition and then short-circuiting out to have it all re-processed from the parent on down?

diff --git a/index.js b/index.js
index e23a643..5756725 100644
--- a/index.js
+++ b/index.js
@@ -245,7 +245,11 @@ FSWatcher.prototype._emit = function(event, path, val1, val2, val3) {
 FSWatcher.prototype._handleError = function(error) {
   var code = error && error.code;
   var ipe = this.options.ignorePermissionErrors;
-  if (error &&
+
+  if (process.platform === 'win32' && code === 'EPERM' && this._watched[error.watcherPath]) {
+    // Workaround for https://github.com/paulmillr/chokidar/issues/566
+    this.add(sysPath.dirname(error.watcherPath), false, true);
+  } else if (error &&
     code !== 'ENOENT' &&
     code !== 'ENOTDIR' &&
     (!ipe || (code !== 'EPERM' && code !== 'EACCES'))

@npetruzzelli
Copy link
Author

Nothing seems to change:

(minor change for logging)

function createChokidarErrorHandler(chokidarInstance, options){
    options = options || {};
    var errorHandlerIsFunction = typeof options.errorHandler === 'function';
    var errorHandlerThis = options.errorHandlerThisArg || null;
    return function handleWatcherError(error){
        if(process.platform === 'win32' && error.code === 'EPERM'){
            var watcherPath = error.watcherPath;
            fs.stat(watcherPath, function(statsError, stats){
                if(!statsError && stats.isDirectory() === true){
                    fs.readdir(watcherPath, function(readdirError, filesAndFolders){
                        if(!readdirError && filesAndFolders.length === 0){
+                           console.log("EPERM error occured on an empty directory.");
                            chokidarInstance.unwatch(watcherPath);

                            var watcherRemove = chokidarInstance._remove.bind(chokidarInstance);
                            watcherRemove(path.dirname(watcherPath), path.basename(watcherPath));

                        } else if(errorHandlerIsFunction){
                            options.errorHandler.call(errorHandlerThis, error);
                        } else {
                            throw error;
                        }
                    });
                } else if(errorHandlerIsFunction){
                    options.errorHandler.call(errorHandlerThis, error);
                } else {
                    throw error;
                }
            });
        } else if(errorHandlerIsFunction){
            options.errorHandler.call(errorHandlerThis, error);
        } else {
            throw error;
        }
    }
}

Console:

$ node index.js
Initial scan complete. Ready for changes
EPERM error occured on an empty directory.
Raw event info: rename :: New folder :: { watchedPath: 'local_packages' }

@onexdata
Copy link

onexdata commented Mar 1, 2019

Is this seriously still an issue? Doesn't chokidar claim they power VSCode? VSCode doesn't crash when you delete an empty folder on windows... what is the way around this bug that has existed over a year?

@onexdata
Copy link

onexdata commented Mar 1, 2019

I notice that just adding this:

    watcher.on('error', error => {
        // Ignore EPERM errors in windows, which happen if you delete watched folders...
        if (error.code === 'EPERM' && require('os').platform() === 'win32') return 
    })

Makes the watch work as expected... Does anyone have any insight into this? Isn't this kind of a big issue? I feel like there is a fix and I just don't know about it.

@tingzhong666
Copy link

Unexpectedly, this problem has existed for three years.

@JAlbertoGonzalez
Copy link

Still happening on Windows. The real problem is folders not being deleted because they are locked "by another process".

@aleclarson
Copy link

Can someone confirm if this problem is fixed by filespy?

@Bistard
Copy link

Bistard commented Nov 2, 2022

Still happening on Windows. The real problem is folders not being deleted because they are locked "by another process".

In my case, I cannot delete the directories and it pops up a window shows I do not have the permissions.

@MauricePasternak
Copy link

Can confirm this is still a thing in December 2022. Only on Windows.

  1. Start watching a folder recursively

  2. Delete some branch in the folder tree structure. Works the first time.

  3. Subsequent attempts to delete some other downstream branch results in an EPERM error. However, watcher.on("error" doesn't report anything.

@rinalditomas
Copy link

July 2023, and the issue persists on Windows. The library remains unable to detect the deletion of empty folders. However, I found a workaround by following the recommended approach:

      if (process.platform === 'win32' && error.code === 'EPERM') {
      // your logic

@npetruzzelli
Copy link
Author

npetruzzelli commented Jul 21, 2023

I haven't looked at this issue in almost 7 years, but I still get notifications about it now and then. Curious, I decided to have a look today. Here are a few thoughts from me. Take them or leave them as you will, and with a heavy grain of salt as I have not worked on this since my previous comment.

I believe that it is not something Chokidar can reasonably solve since it happens in the Node.js API.

Curious? Expand the details for more.

I would like to remind new visitors about this comment:
#566 (comment)

Specifically:

  • The exact same error occurs when you remove Chokidar entirely from the picture
    var fs = require('fs');
    fs.watch('local_packages/New folder', { persistent: true}, function(event, filename){
        console.log(arguments);
    });

That snippet is the entire file. Name it example.js and execute it in the command line with node example.js. Mess with the file system using the windows file explorer or by executing another node file.

You can't make it any smaller than that.

Who knows, maybe it is specific to sending the directory to the windows recycling bin. Could node solve it or at least identify it separately from other errors? Is it a problem with the Windows file system API that Node itself uses? Is it an issue with V8? I don't know.


Back then, it was Node.js 4.x. We are now up to 20.x. The problem may still exist, but something has almost certainly changed in all this time.

If you are curious, why not have a look at Node's code?
https://github.com/nodejs/node/blob/v20.5.0/lib/internal/fs/watchers.js#L304-L378

Comfortable with C++? Try checking out src instead of lib:
https://github.com/nodejs/node/blob/v20.5.0/src/
I don't know the language well myself, but I'd guess you are interested in the node_*.cc and node_*.h files


If I were still working on this, I would open an issue in Node's repo. I could reference this issue for context (not required). It could be significantly slimed down to the 4 lines of script and an explanation of when it occurs, when it doesn't occur, and what the expected behavior is.

If I learn something new, I'd come back here to share or even open a pull request if I had the time available to actively contribute.

I may be wrong, I just have not had the time or need to pursue this any further. Hopefully the next person has more luck or at least finds some of this helpful.

@kodelio
Copy link

kodelio commented Aug 3, 2023

August 2023, issue is still existing on Windows. I can't consider using usePolling: true because it results in a high CPU usage.
Also the following workarounds doesn't work :

watcher.on('error', error => {
        // Ignore EPERM errors in windows, which happen if you delete watched folders...
        if (error.code === 'EPERM' && require('os').platform() === 'win32') return 
    })

@paulmillr
Copy link
Owner

The backlog is going to start from zero as a preparation for v4 release. v4 would bring massive rewrite to the table and drop most dependencies. All issues are being closed as preparation for v4 release.

In the future, only issues with enough community support would be considered.

See issue 1195 for more info. Thank you.

Repository owner locked and limited conversation to collaborators Jul 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests