Handle documents being deleted before processing their requests

[vinistock]

Motivation

We sometimes get Errno::ENOENT in our telemetry and I believe I finally figured out why.

Consider this sequence of events:

1. Someone opens a file and the editor fires a bunch of requests
2. All of those requests are placed into the incoming queue
3. Then the user deletes the file
4. The server will receive a textDocument/didClose notification. This is handled immediately, removing the document from the store
5. Then the worker picks up a request for the deleted file. Maybe it fell behind processing because the file was large or something caused it to lag
6. Since the document is no longer present in the store, it will try to read it from disk, but the file no longer exists either so it then raises ENOENT

Implementation

I believe we have two options:

Stop processing textDocument/didClose synchronously and throw it in the incoming queue. This will guarantee that we only close the document after processing all requests received for it. However, we will waste time processing requests for a file that was already deleted

The second option is what I implemented: we raise a specific error for when documents no longer exist and avoid reporting those to our telemetry.

Automated Tests

Added a test that reproduces the behaviour. Notice that it's difficult to control the order of execution of messages, so the test itself is not 100% reliable, but it does catch the bug.

[andyw8]

I worry that these tests being non-deterministic may cause some pain in future (e.g. intermittent failures). Could we test server.rb by stubbing something to raise NonExistingDocumentError?

[vinistock]

I'm not sure that will be testing the behaviour we want though. We want to ensure that requests that have been enqueued for a file that gets deleted won't break in unexpected ways.

Stubbing a method in server to raise is not really replicating that.