fix: one unreadable path no longer stops the file cache tracking anything

Source/FileManager/BackgroundFileSystem.cs

@@ -107,7 +107,17 @@ private void Stop()
 		catch (ObjectDisposedException) { }
 
 		//Wait for background scanner to terminate before reinitializing.
-		backgroundScanner?.Wait();
+		try
+		{
+			backgroundScanner?.Wait();
+		}
+		// A scanner that died still has to be waited on, but its failure is not this caller's to raise: Stop() is
+		// reached from Refresh() and from Dispose(), neither of which has anything to do with whatever went wrong,
+		// and both of which are about to replace the scanner anyway.
+		catch (AggregateException ex)
+		{
+			Serilog.Log.Logger.Error(ex, "The file cache's background scanner had already stopped on an error.");
+		}
 
 		//Dispose of directoryChangesEvents after backgroundScanner exists. Clear the field first so a late event
 		//has nothing to add to. CompleteAdding has to have happened while it was still set, or the scanner would
@@ -146,8 +156,19 @@ private void BackgroundScanner()
 	{
 		while (directoryChangesEvents?.TryTake(out var change, -1) is true)
 		{
-			lock (fsCacheLocker)
-				UpdateLocalCache(change);
+			try
+			{
+				lock (fsCacheLocker)
+					UpdateLocalCache(change);
+			}
+			// One path this cannot read must not end the scan. An exception here used to be terminal twice over:
+			// it stopped the cache tracking anything further, and it was stored on the task, so the next Stop()
+			// rethrew it as an AggregateException at whoever had called Refresh() - a caller with nothing to do
+			// with the file that went missing.
+			catch (Exception ex)
+			{
+				Serilog.Log.Logger.Debug(ex, "Could not apply a file system change to the file cache: {@DebugText}", new { change.ChangeType, change.FullPath });
+			}
 		}
 	}
 
@@ -181,14 +202,36 @@ private void AddPath(LongPath path)
 	{
 		path = path.LongPathName;
 		//Temporary files created when updating the db will disappear before their attributes can be read.
-		if (Path.GetFileName(path).Contains("LibationContext.db") || !File.Exists(path) && !Directory.Exists(path))
+		if (Path.GetFileName(path).Contains("LibationContext.db"))
+			return;
+
+		// Whether it exists and what it is were two questions, and the answer to the first could stop being true
+		// before the second was asked: a download's temp file, or a folder the user has just moved or deleted, is
+		// gone by the time its attributes are read. One question instead, and its failure means the same thing
+		// the existence check meant - there is nothing here to add.
+		if (TryGetAttributes(path) is not FileAttributes attributes)
 			return;
-		if (File.GetAttributes(path).HasFlag(FileAttributes.Directory))
+
+		if (attributes.HasFlag(FileAttributes.Directory))
 			AddUniqueFiles(SafestEnumerateFiles(path));
 		else
 			AddUniqueFile(path);
 	}
 
+	/// <returns>What the path is, or null when it cannot be read and so has nothing to add.</returns>
+	internal static FileAttributes? TryGetAttributes(LongPath path)
+	{
+		try
+		{
+			return File.GetAttributes(path);
+		}
+		catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or ArgumentException or NotSupportedException)
+		{
+			Serilog.Log.Logger.Debug(ex, "Nothing to add to the file cache for {@DebugText}", new { path = (string)path });
+			return null;
+		}
+	}
+
 	private IEnumerable<LongPath> SafestEnumerateFiles(string path)
 	{
 		try

Source/FileManager/_InternalsVisible.cs

@@ -0,0 +1 @@
+[assembly: System.Runtime.CompilerServices.InternalsVisibleTo(nameof(FileManager) + ".Tests")]

Source/_Tests/FileManager.Tests/BackgroundFileSystemTests.cs

@@ -103,3 +103,132 @@ private static bool WaitFor(Func<bool> condition, int timeoutMs = 5000)
 		return false;
 	}
 }
+
+/// <summary>
+/// A second failure mode in the same class, from a CI run where all three Windows legs failed and the other six
+/// passed: every test in FileLiberator.Tests' PDF path suite failed in TestInitialize with an AggregateException
+/// wrapping <c>FileNotFoundException: Could not find file ...</c>, naming a path none of those tests had
+/// anything to do with. The watcher had raised Created for a folder an earlier test's cleanup then deleted; the
+/// scanner asked whether it existed, was told yes, asked what it was, and got an exception. That killed the
+/// scanner and was stored on its task, and the next Stop() - reached from AudibleFileStorage.Audio.Refresh() by
+/// way of Dispose() - rethrew it at that caller.
+/// <para>
+/// The trigger is a disagreement between Exists and GetAttributes over a long <c>\\?\</c> path, which cannot be
+/// staged on the platform these tests usually run on. So the guard itself is asserted directly, and the rest
+/// covers what the death cost: a cache that stops tracking, and an exception handed to an unrelated caller.
+/// </para>
+/// </summary>
+[TestClass]
+[DoNotParallelize]
+public class PathsThatVanishBeforeTheyAreRead
+{
+	private string tempDir = string.Empty;
+
+	[TestInitialize]
+	public void Initialize()
+	{
+		tempDir = Path.Combine(Path.GetTempPath(), $"libation-bfs-vanishing-{Guid.NewGuid():N}");
+		Directory.CreateDirectory(tempDir);
+	}
+
+	[TestCleanup]
+	public void Cleanup()
+	{
+		try
+		{
+			Directory.Delete(tempDir, recursive: true);
+		}
+		catch (IOException)
+		{
+			// A leftover temp directory is not worth failing a test over.
+		}
+	}
+
+	/// <summary>Creates a batch of files and deletes the tree while their events are still queued.</summary>
+	private void ChurnVanishingFiles()
+	{
+		for (var batch = 0; batch < 5; batch++)
+		{
+			var directory = Path.Combine(tempDir, $"vanishing-{batch}");
+			Directory.CreateDirectory(directory);
+
+			for (var i = 0; i < 40; i++)
+				File.WriteAllText(Path.Combine(directory, $"book-{i}.m4b"), "audio");
+
+			// No wait: the scanner should reach these paths after they have gone.
+			Directory.Delete(directory, recursive: true);
+		}
+	}
+
+	[TestMethod]
+	public void a_path_that_is_not_there_reads_as_nothing_rather_than_throwing()
+	{
+		// The fix, on its own terms. Asking what a path is used to be allowed to throw, on the strength of having
+		// asked a moment earlier whether it was there.
+		Assert.IsNull(BackgroundFileSystem.TryGetAttributes(Path.Combine(tempDir, "never-existed.m4b")));
+		Assert.IsNull(BackgroundFileSystem.TryGetAttributes(Path.Combine(tempDir, "no", "such", "folder", "book.m4b")));
+	}
+
+	[TestMethod]
+	public void a_path_that_is_there_still_reads_as_what_it_is()
+	{
+		var file = Path.Combine(tempDir, "real.m4b");
+		File.WriteAllText(file, "audio");
+
+		Assert.IsFalse(BackgroundFileSystem.TryGetAttributes(file)!.Value.HasFlag(FileAttributes.Directory));
+		Assert.IsTrue(BackgroundFileSystem.TryGetAttributes(tempDir)!.Value.HasFlag(FileAttributes.Directory));
+	}
+
+	[TestMethod]
+	public void the_scanner_keeps_going_after_a_path_it_cannot_read()
+	{
+		using var sut = new BackgroundFileSystem(tempDir, "*.*", SearchOption.AllDirectories);
+
+		ChurnVanishingFiles();
+
+		// The scanner is still doing its job, which is what dying used to cost: the first unreadable path ended
+		// the loop, and every later change to the Books directory went unnoticed for the rest of the session.
+		File.WriteAllText(Path.Combine(tempDir, "survivor.m4b"), "audio");
+
+		var found = WaitFor(() => sut.FindFile(new Regex(@"survivor\.m4b$")) is not null);
+		Assert.IsTrue(found, "the scanner stopped tracking changes after a path it could not read");
+	}
+
+	[TestMethod]
+	public void disposing_afterwards_does_not_hand_the_failure_to_the_caller()
+	{
+		// This is the CI stack: Refresh() found the Books directory changed, disposed the old file system, and
+		// Stop() waited on a scanner that had already faulted.
+		var sut = new BackgroundFileSystem(tempDir, "*.*", SearchOption.AllDirectories);
+
+		ChurnVanishingFiles();
+
+		sut.Dispose();
+	}
+
+	[TestMethod]
+	public void a_file_that_outlives_its_event_is_still_tracked()
+	{
+		// The guard must not have been widened into ignoring everything: what is really there still lands.
+		using var sut = new BackgroundFileSystem(tempDir, "*.*", SearchOption.AllDirectories);
+
+		var directory = Path.Combine(tempDir, "kept");
+		Directory.CreateDirectory(directory);
+		File.WriteAllText(Path.Combine(directory, "kept.m4b"), "audio");
+
+		var found = WaitFor(() => sut.FindFile(new Regex(@"kept\.m4b$")) is not null);
+		Assert.IsTrue(found, "a file that was never deleted did not reach the cache");
+	}
+
+	private static bool WaitFor(Func<bool> condition, int timeoutMs = 5000)
+	{
+		for (var waited = 0; waited < timeoutMs; waited += 50)
+		{
+			if (condition())
+				return true;
+			Thread.Sleep(50);
+		}
+
+		return false;
+	}
+}