Resolves #523

[subdavis]

resolves #523

Fixes for a great number of issues on my part.

• Change express.js error handler middleware to proper pattern described here
• Return a value from POST meta and POST detections, which was causing those functions to hang indefinitely and basically DoS the express server from the electron client. After max outstanding requests is hit, the server chokes up. This was the Expose piplines and clean up deploy #1 issue.
• Implement project directory lockfile to prevent either 2 request threads or 2 instances of viame web from modifying the same data at the same time. Node isn't multi-threaded, but because we use async fs operations, these functions can be interleaved in dumb ways. For example, running save twice in quick succession could move a file into aux in thread 2 while it's still being written by thread 1. Avoid the problem with dir locks, and if the lock fails to acquire, that's fine.
• Debounce the end signal from ConfidenceFilter. It will usually be emitted twice because of how we have to catch it, but we don't want 1 change to trigger 2 requests (as it currently does).
• Add milliseconds to filename so that more than 1 savefile per second can be created (otherwise, you'll have name collisions in aux)

It's worth mentioning that there's nothing currently preventing someone from having 2 instances of dive open on the same project where the annotations diverge because they aren't loaded from disk except once at launch. This is probably not important, but a lockfile could at least yell about it. With proper-lockfile, it'd be like 10 lines of code to add this check. IDK if it's worth it.

[subdavis]

mock-fs and proper-lockfile are incompatible, but mock-fs also breaks console.log, so I can't figure out why.

tschaub/mock-fs#234

One of the two needs to be abandoned, but both are important....

[BryonLewis]

Without looking into it I assumed it was save file being accessed before the initial saved completed. I didn't think it would be the server choking up. I'll look at this in more detail tomorrow. It would make sense with the secondary thing I experienced where the machine would lock for like 2 minutes afterwards (in retrospect it was the requests that weren't working) and then everything would work again.

[subdavis]

I experienced where the machine would lock for like 2 minutes afterwards

Yep, that's express getting swamped, not an FS issue. I basically invented inverse slowloris.

[BryonLewis]

Everything looks good and seems to resolve the issues. Just a few questions that can probably be dismissed.

This comment it probably an extreme corner and something that we don't need to worry about now. Unlikely case there is a memory overflow error or a task manager kill while the file is locked will prevent the lock file from being removed. Then when you restart the app and open it again it won't allow you to save the file but then the act of properly closing it will remove the existing lock file that it couldn't acquire a lock on. So just a note if there is an error where it crashes -> user restarts and can't save -> closes and then can save after is it crashing during saving or while a lock file is active.

I would think the only remedy would be to clear any existing lock files in the directory on initial launch if they exist. In combination with ensuring there is only a single electron instance open at once - https://github.com/electron/electron/blob/master/docs/api/app.md#apprequestsingleinstancelock

[subdavis]

Unlikely case there is a memory overflow error or a task manager kill while the file is locked will prevent the lock file from being removed. Then when you restart the app and open it again it won't allow you to save the file but then the act of properly closing it will remove the existing lock file that it couldn't acquire a lock on. So just a note if there is an error where it crashes -> user restarts and can't save -> closes and then can save after is it crashing during saving or while a lock file is active.

Are you sure, and have you tested? The lockfile library has a staleness check mechanism which defaults to 10 seconds.

I'm using https://github.com/moxystudio/node-proper-lockfile, not electron's locking.

When a lock is successfully acquired, the lockfile's mtime (modified time) is periodically updated to prevent staleness. This allows to effectively check if a lock is stale by checking its mtime against a stale threshold. If the update of the mtime fails several times, the lock might be compromised. The mtime is supported in almost every filesystem.

So 10 seconds after the crash, lockfile will become stale and save will work.

[BryonLewis]

Confirmed, it was an issue in my testing and how I instigated the error.