FileSystemWatcher and batching

[cdietrich]

we are using FileSystemWatcher feature of lsp to track changes of deleted files server side.
in general this works perfectly fine.

but in some cases not all files arrive in the didChangeWatchedFiles event on the server side when files are deleted client side with a small pause.

(there is also no second batch)

how does the watcher/client batch the local delete events on the server?
can i debug the issue locally somehow

[cdietrich]

ok reading the code i have found client/src/common/client.ts has notifyFileEvent
i wonder what happens when a new event is pushed between the start and end of the
sendNotification await in the trigger
should the trigger first replace the array and then call the sendNotification?

=> i assume a

const events = client._fileEvents;
client._fileEvents = [];
await client.sendNotification(DidChangeWatchedFilesNotification.type, { changes: events });

would help

[EhabY]

I was able to re-produce this by copy-pasting (in the file system) with some pauses, it dropped one of the changes...

[cdietrich]

hmmm the patch is not 100% sufficient.
something the events in a single filesystemwatcher onDidDelete also misses the events even before they are batched
(just logging client side)

[cdietrich]

also can see this in a naive test attempt
https://github.com/cdietrich/fs-watcher-bug/blob/main/src/test/extension.test.ts

[dbaeumer]

@cdietrich good catch. That code should definitely be fixed. The bug got introduced when making notification sending async ;-).

I looked at your test case and that definitely looks like a problem in VS Code itself if you don't receive the 1000 delete events since you are not deleting the parent folder.

[dbaeumer]

I fixed the problem on the LSP client side. For the VS Code problem I would recommend to open an issue against VS Code. Could you please mention me on it so that I know about the outcome.

[EhabY]

also can see this in a naive test attempt https://github.com/cdietrich/fs-watcher-bug/blob/main/src/test/extension.test.ts

I rewrote the test using WorkspaceEdits, these also fail on my Windows:

import * as assert from 'assert';

// You can import and use all API from the 'vscode' module
// as well as import your extension to test it
import * as vscode from 'vscode';

suite('Extension Test Suite', () => {
	vscode.window.showInformationMessage('Start all tests.');

	test('Sample test', async () => {
		const uri = vscode.workspace.workspaceFolders![0].uri;
		const wse = new vscode.WorkspaceEdit();
		for (let i = 0; i < 1000; i++) {
			wse.createFile(vscode.Uri.joinPath(uri, `test_${i}.txt`));
		}
		await vscode.workspace.applyEdit(wse);
		
		const relativePattern = new vscode.RelativePattern(uri, "*.txt");
		const files = await vscode.workspace.findFiles(relativePattern);
		assert.strictEqual(files.length, 1000);

		const watcher = vscode.workspace.createFileSystemWatcher(relativePattern);
		
		let counter = 0;
		watcher.onDidDelete((e) => {
			console.log(e);
			counter++;
		});
		
		const deleteWse = new vscode.WorkspaceEdit();
		for (let i = 0; i < 1000; i++) {
			deleteWse.deleteFile(vscode.Uri.joinPath(uri, `test_${i}.txt`));
		}	
		await vscode.workspace.applyEdit(deleteWse);
		
		const files2 = await vscode.workspace.findFiles("*.txt");
		assert.strictEqual(files2.length, 0);
		watcher.dispose();

		assert.strictEqual(1000, counter);
	});
});