Karma crashes on editing files in Visual Studio (due to temp files?)

[DanTup]

When I run Karma and edit a js file in Visual Studio, it crashes like this (edit: seems like this is intermittent rather than consistent!):

ERROR [karma]: { [Error: ENOENT, stat 'M:\Coding\TestApps\AngularAppTest\Tests\tqxo1gsf.xxl']
  errno: 34,
  code: 'ENOENT',
  path: 'M:\\Coding\\TestApps\\AngularAppTest\\Tests\\tqxo1gsf.xxl' }
Error: ENOENT, stat 'M:\Coding\TestApps\AngularAppTest\Tests\tqxo1gsf.xxl'
INFO [watcher]: Changed file "M:/Coding/TestApps/AngularAppTest/Tests/SampleSpec.js".

It seems that when you save in Visual Studio, what happens is:

1. New (temp) file is created with the new contents
2. Old file is deleted
3. Temp file is renamed to the original filename

When I set autoWatchBatchDelay to 1000ms the error still seemed to occur, and appeared to occur immediately (not after 1s), however with it changed to 5000ms, I'm unable to repro (but still trying!), so not completely sure whether it's related to the threshold or not. My machine is very fast and I'm saving to SSDs, so I would be surprised if anything could take 1s, but Ill continue investigating!

(I can't find the code that actually uses autoWatchBatchDelay in GitHub search; so I'm struggling to dig into the code that does this)

[DanTup]

Ok, a little more info...

1. Karma's watcher.js adds the entire folder to the Chokidar watcher (regardless of the pattern).
2. Chokidar gets an event from Windows about the (temp) file VS just created
3. (in a short space of time, VS deletes the old file, renames the temp file in its place)
4. Chokidar then tries to fs.stat() the file by its old name (in FSWatcher.prototype._handle) and gets back an error, which it seems to throw upwards (it calls this.emit, I'm not familiar with how errors bubble up in node, but I presume this is doing up unhandled)

You could possibly call this a bug in Chokidar? A file that is created and quickly renamed before the created event fires causes the crash?

Either way, it doesn't feel like the IDE or the user is at fault here; the process shouldn't die!

[DanTup]

Ok, I can "fix" this issue by simply handling error events from Chokidar and dumping them to the console.

In watcher.js I changed:

// register events
chokidarWatcher.on('add', bind(fileList.addFile))
    .on('change', bind(fileList.changeFile))
    .on('unlink', bind(fileList.removeFile));

to:

// register events
chokidarWatcher.on('add', bind(fileList.addFile))
    .on('change', bind(fileList.changeFile))
    .on('unlink', bind(fileList.removeFile))
    .on('error', console.error);

This might not be the best fix, because:

a) it's now routing all errors to the console, not just those that are rogue (based on this case)
b) it's now spamming the console with errors that are basically not useful to a user

However; it does stop Karma from crashing out randomly when editing with Visual Studio; so I'm tempted to say it's better than the current implementation.

Thoughts? I'm happy to send a PR if you think it's valid.

[vojtajina]

@DanTup thanks for investigation!

I think we should fix this in Chokidar. What do you think @paulmillr? If there is a new file that gets removed before Chokidar stats it, it should either ignore it (or emit some special event like "added but already gone" if somebody really cares about this).

Also, @DanTup can you try usePolling: true (if you are on Karma 0.12, polling is off by default, on Windows).

[vojtajina]

Also, @DanTup what are the other errors that you see from chokidar if you register the "error" handler? In the meantime, we might just register the handler and ignore the errors, something like this:

// ...
    .on('error', function(e) {
      log.debug(e);
    });

[DanTup]

I think fixing it in Chokidar makes sense. It feels clunky to have its own event, but I'm not sure it can be avoided without hiding info from the calling module :(

@vojtajina I haven't seen any other errors in my limited testing; I'm just assuming the error handler in Chokidar was there to handle some sort of errors. In any case, I believe stopping errors from bringing down Karma is the most correct thing to do; so I think the proposed change makes sense, even if Chokidar is updated.

I'll try out polling next time I get back to this. For now, I just hacked in my change to avoid the crash and was able to continue :)

[paulmillr]

Definitely.

[DanTup]

@vojtajina Will you take a PR to handle the error event and route it to log.debug in the meantime? I think it's valuable to have, even with any Chokidar changes; as I don't think it makes sense to let filesystem watching events ever bring the whole or Karma down?

[vojtajina]

Of course, please do.

Don't you wanna fix it in chokidar?

[DanTup]

@vojtajina I think it's correct to have this handling/logging in even if Chokidar is fixed; as it never really makes sense to bring Karma down for any sort of file-watching error? (it's better to fail to detect a change than terminate with no obvious reason why).

I did have a look though Chokidar while I was debugging, but there were several places that did stats and it was hard to track down which one it was - I'm not completely confident I could fix it well without understanding how it worked a little more.