From 0aa9cb0019eca3f6c2b72e43713af428f9619500 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 16:05:45 +0000 Subject: [PATCH 1/8] Heal a search index Lucene cannot open, instead of retrying it as a lock conflict A truncated or zero-length segments file is reported by Lucene 3 as a plain IOException ("read past EOF") rather than a CorruptIndexException, so it was misclassified as a write.lock conflict: CreateNewIndex burned its whole backoff budget and rethrew, and the delete-and-rebuild recovery never ran. Passing create/overwrite to IndexWriter does not repair it either, because IndexFileDeleter reads every segments_* file in the directory and tolerates only missing ones, so a single unreadable segments file -- even a stale one from an older commit -- leaves the index permanently unopenable. The user's only cure was deleting the SearchEngine folder by hand. Retries are now reserved for genuine lock conflicts (LockObtainFailedException, which derives from IOException, and UnauthorizedAccessException), and any other open failure gets one delete-and-rebuild pass before giving up with a message that says which folder to remove. The query path recovers too, since IsRecoverableCorruptIndexException now recognizes the truncated-segments signature. Search index updates are also no longer allowed to fail the library change that triggered them. Both events fire after the database is committed, so an escaping exception reported a successful scan as "Error importing library" and, being the first subscriber, stopped the handlers that refresh the grid and backup counts. Co-authored-by: rmcrackan --- Source/AppScaffolding/LibationScaffolding.cs | 21 +- Source/LibationSearchEngine/SearchEngine.cs | 136 ++++++------ .../CorruptIndexRecoveryTests.cs | 204 ++++++++++++++++++ 3 files changed, 297 insertions(+), 64 deletions(-) create mode 100644 Source/_Tests/LibationSearchEngine.Tests/CorruptIndexRecoveryTests.cs diff --git a/Source/AppScaffolding/LibationScaffolding.cs b/Source/AppScaffolding/LibationScaffolding.cs index c52ca0a1..5de6a0d9 100644 --- a/Source/AppScaffolding/LibationScaffolding.cs +++ b/Source/AppScaffolding/LibationScaffolding.cs @@ -341,10 +341,27 @@ public static class LibationScaffolding private static void wireUpSystemEvents(Configuration configuration) { LibraryCommands.LibrarySizeChanged += (_, libraryBooks) - => SearchEngineCommands.FullReIndex(libraryBooks); + => updateSearchIndex(() => SearchEngineCommands.FullReIndex(libraryBooks)); LibraryCommands.BookUserDefinedItemCommitted += (_, books) - => SearchEngineCommands.UpdateBooks(books); + => updateSearchIndex(() => SearchEngineCommands.UpdateBooks(books)); + } + + /// + /// The search index is a cache derived from the database, and these events fire after the database change is + /// already committed. Letting a search index failure escape would report a successful scan as a failed import and + /// would stop the remaining event handlers -- the ones that refresh the grid and the backup counts -- from running. + /// + private static void updateSearchIndex(Action update) + { + try + { + update(); + } + catch (Exception ex) + { + Serilog.Log.Logger.Error(ex, "Failed to update the search index. Library changes are saved; search and filter results may be stale until the next update succeeds."); + } } public static VersionCheckResult GetLatestRelease() diff --git a/Source/LibationSearchEngine/SearchEngine.cs b/Source/LibationSearchEngine/SearchEngine.cs index b05ab9be..b824a06f 100644 --- a/Source/LibationSearchEngine/SearchEngine.cs +++ b/Source/LibationSearchEngine/SearchEngine.cs @@ -80,58 +80,99 @@ public class SearchEngine /// create new. ie: full re-index public void CreateNewIndex(IEnumerable library, bool overwrite = true) { - const int maxRetries = 5; - const int baseDelayMs = 400; var libraryList = library.ToList(); - // Corruption (e.g. checksum mismatch in segments) is not fixed by waiting; clear and rebuild immediately. - var corruptRebuildAttemptsRemaining = 2; - // Exponential backoff retry: 400 ms, 800 ms, 1600 ms, etc - // Total wait time before giving up: 12.4 sec - for (var attempt = 0; attempt < maxRetries; attempt++) + // location of index/create the index + using var index = getIndex(); + + // analyzer for tokenizing text. same analyzer should be used for indexing and searching + using var analyzer = new StandardAnalyzer(Version); + using var ixWriter = openWriterForRebuild(index, analyzer, overwrite); + + foreach (var libraryBook in libraryList) + { + var doc = createBookIndexDocument(libraryBook); + ixWriter.AddDocument(doc); + } + } + + /// + /// Opens the writer for a full re-index, waiting out lock conflicts and repairing an index Lucene cannot open. + /// + private IndexWriter openWriterForRebuild(Lucene.Net.Store.Directory index, StandardAnalyzer analyzer, bool overwrite) + { + // Exponential backoff for lock conflicts: 400 ms, 800 ms, 1600 ms, 3200 ms. 6.4 sec total before giving up. + const int maxAttempts = 5; + const int baseDelayMs = 400; + var repairsRemaining = 1; + + for (var attempt = 1; ; attempt++) { try { - createNewIndexCore(libraryList, overwrite); - return; + var createNewIndex = overwrite || !IndexReader.IndexExists(index); + return new IndexWriter(index, analyzer, createNewIndex, IndexWriter.MaxFieldLength.UNLIMITED); } - catch (CorruptIndexException ex) when (corruptRebuildAttemptsRemaining-- > 0) + catch (Exception ex) when (isTransientLockConflict(ex) && attempt < maxAttempts) { - Serilog.Log.Logger.Warning(ex, "Lucene search index corrupt at {Path}. Clearing for rebuild.", SearchEngineDirectory); - deleteAllSearchIndexFiles(SearchEngineDirectory); - attempt--; - } - catch (IOException ex) when (attempt < maxRetries - 1 && ex is not CorruptIndexException) - { - var delayMs = baseDelayMs * (1 << attempt); - // write.lock can be held by another process (e.g. second Libation instance, antivirus) or a prior writer that did not release. Retry after delay. - Serilog.Log.Logger.Warning(ex, "Search index lock conflict (attempt {Attempt}/{Max}), retrying in {Delay}ms", attempt + 1, maxRetries, delayMs); + var delayMs = baseDelayMs * (1 << (attempt - 1)); + Serilog.Log.Logger.Warning(ex, "Search index lock conflict (attempt {Attempt}/{Max}), retrying in {Delay}ms", attempt, maxAttempts, delayMs); Thread.Sleep(delayMs); } - catch (UnauthorizedAccessException ex) when (attempt < maxRetries - 1) + catch (Exception ex) when (!isTransientLockConflict(ex) && repairsRemaining-- > 0) { - var delayMs = baseDelayMs * (1 << attempt); - // Windows may report "file in use" as UnauthorizedAccessException - Serilog.Log.Logger.Warning(ex, "Search index lock conflict (attempt {Attempt}/{Max}), retrying in {Delay}ms", attempt + 1, maxRetries, delayMs); - Thread.Sleep(delayMs); + // Passing overwrite/create to IndexWriter does not repair an unreadable index: its IndexFileDeleter + // reads every segments_* file in the directory and only tolerates missing ones, so a single unreadable + // segments file -- even a stale one from an older commit -- keeps the whole directory unusable until it + // is deleted. This is a full re-index, so nothing on disk is worth preserving. + Serilog.Log.Logger.Error(ex, "Search index at {Path} could not be opened. Deleting it and rebuilding from the library.", SearchEngineDirectory); + + if (deleteAllSearchIndexFiles(SearchEngineDirectory) is { Count: > 0 } undeletable) + throw new IOException($"The search index at '{SearchEngineDirectory}' is damaged and could not be deleted automatically. Close Libation, delete that folder, then restart. Undeletable file(s): {string.Join(", ", undeletable)}", ex); + + // the repair, not the failed open, gets a fresh allowance of lock retries + attempt = 0; } } } /// - /// Lucene 3 parses segments_* filenames in the index directory. Cloud sync (e.g. OneDrive) can leave debris - /// or conflict copies whose names break that parser, throwing with this message shape. - /// Actual error is likely to be something like: Invalid or unsupported character in number, hence this string check. - /// (e.g. checksum mismatch in segments) is also recoverable by deleting the index and rebuilding. + /// True when the index could not be opened because something else is holding it, which is worth waiting out. + /// derives from , so a bare + /// IOException check cannot tell a lock conflict apart from a damaged index. + /// + private static bool isTransientLockConflict(Exception ex) + => ex is LockObtainFailedException + // Windows may report "file in use" as UnauthorizedAccessException + || ex is UnauthorizedAccessException; + + /// + /// True when Lucene cannot read the index and the only cure is deleting it and rebuilding from the database. + /// + /// : eg. checksum mismatch in the segments file. + /// A truncated or zero-length segments_* file, which Lucene 3 reports as a plain + /// from BufferedIndexInput.Refill rather than as a + /// . Matched by message because the type is shared with + /// , which must keep retrying instead. + /// Lucene 3 parses segments_* filenames in the index directory. Cloud sync (eg. OneDrive) can leave + /// debris or conflict copies whose names break that parser, throwing with a + /// message like "Invalid or unsupported character in number", hence this string check. + /// /// public static bool IsRecoverableCorruptIndexException(Exception ex) => ex is CorruptIndexException + || (ex is IOException && !isTransientLockConflict(ex) && ex.Message.Contains("read past EOF", StringComparison.OrdinalIgnoreCase)) || (ex is ArgumentException aex && aex.Message.Contains("character in number", StringComparison.OrdinalIgnoreCase)); - private static void deleteAllSearchIndexFiles(string searchEngineDirectory) + /// + /// Best-effort delete of everything under the index directory. Returns the segments_* files that survived: + /// Lucene rebuilds happily in an empty directory, and ignores leftover segment data, but any segments file it + /// cannot read keeps the directory permanently unusable. + /// + private static List deleteAllSearchIndexFiles(string searchEngineDirectory) { if (!System.IO.Directory.Exists(searchEngineDirectory)) - return; + return []; foreach (var file in System.IO.Directory.GetFiles(searchEngineDirectory, "*", SearchOption.AllDirectories)) FileUtility.TrySaferDelete(file); @@ -147,39 +188,10 @@ public class SearchEngine Serilog.Log.Logger.Warning(ex, "Could not remove search index subdirectory {Dir}", dir); } } - } - private void createNewIndexCore(List library, bool overwrite) - { - bool indexExists; - using (var indexProbe = getIndex()) - { - try - { - indexExists = IndexReader.IndexExists(indexProbe); - } - catch (Exception ex) when (IsRecoverableCorruptIndexException(ex)) - { - Serilog.Log.Logger.Warning(ex, "Lucene search index at {Path} is unreadable or corrupt (often cloud-sync debris or a partial write). Clearing it for rebuild.", SearchEngineDirectory); - indexExists = false; - } - } - - if (!indexExists) - deleteAllSearchIndexFiles(SearchEngineDirectory); - - // location of index/create the index - using var index = getIndex(); - var createNewIndex = overwrite || !indexExists; - - // analyzer for tokenizing text. same analyzer should be used for indexing and searching - using var analyzer = new StandardAnalyzer(Version); - using var ixWriter = new IndexWriter(index, analyzer, createNewIndex, IndexWriter.MaxFieldLength.UNLIMITED); - foreach (var libraryBook in library) - { - var doc = createBookIndexDocument(libraryBook); - ixWriter.AddDocument(doc); - } + return System.IO.Directory.Exists(searchEngineDirectory) + ? [.. System.IO.Directory.GetFiles(searchEngineDirectory, "segments*", SearchOption.AllDirectories)] + : []; } public SearchEngine(string? directory = null) diff --git a/Source/_Tests/LibationSearchEngine.Tests/CorruptIndexRecoveryTests.cs b/Source/_Tests/LibationSearchEngine.Tests/CorruptIndexRecoveryTests.cs new file mode 100644 index 00000000..2ff3b0a0 --- /dev/null +++ b/Source/_Tests/LibationSearchEngine.Tests/CorruptIndexRecoveryTests.cs @@ -0,0 +1,204 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using AssertionHelper; +using DataLayer; +using LibationSearchEngine; +using Lucene.Net.Index; +using Lucene.Net.Store; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Directory = System.IO.Directory; + +namespace SearchEngineTests; + +/// +/// A damaged search index used to block library imports forever: Lucene 3 reports an unreadable segments file as a +/// plain , which Libation mistook for a write.lock conflict and retried instead of repairing, +/// and passing create/overwrite to does not repair it either. +/// +[TestClass] +public class CorruptIndexRecoveryTests +{ + private string indexDirectory = null!; + + [TestInitialize] + public void Initialize() + { + indexDirectory = Path.Combine(Path.GetTempPath(), "LibationSearchEngineTests", Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(indexDirectory); + } + + [TestCleanup] + public void Cleanup() + { + if (Directory.Exists(indexDirectory)) + Directory.Delete(indexDirectory, recursive: true); + } + + private static LibraryBook book(string asin, string title) + { + var contributor = Contributor.GetEmpty(); + var b = new Book(new AudibleProductId(asin), title, null, null, 1, ContentType.Product, [contributor], [contributor], "us"); + return new LibraryBook(b, new DateTime(2026, 8, 15), "account"); + } + + private static readonly List library + = [book("B0TEST0001", "Hound of the Baskervilles"), book("B0TEST0002", "Sign of the Four")]; + + private SearchEngine reIndex() + { + var engine = new SearchEngine(indexDirectory); + engine.CreateNewIndex(library); + return engine; + } + + private void assertIndexIsUsable() + { + var engine = new SearchEngine(indexDirectory); + engine.Search("Baskervilles").Docs.Select(d => d.ProductId).Should().BeEquivalentTo(["B0TEST0001"]); + engine.Search(SearchEngine.ALL_QUERY).Docs.Should().HaveCount(2); + } + + private string currentSegmentsFile() + => Directory.GetFiles(indexDirectory, "segments_*").OrderBy(f => f.Length).ThenBy(f => f).Last(); + + /// Reproduces the state Lucene leaves behind when a commit is interrupted, eg. by a crash or power loss. + private void truncateCurrentSegmentsFile(int keepBytes) + { + var file = currentSegmentsFile(); + File.WriteAllBytes(file, File.ReadAllBytes(file).Take(keepBytes).ToArray()); + } + + private static Exception exceptionFromOpeningWriter(string directory) + { + using var index = FSDirectory.Open(directory); + using var analyzer = new Lucene.Net.Analysis.Standard.StandardAnalyzer(SearchEngine.Version); + return Assert.ThrowsExactly(() => new IndexWriter(index, analyzer, true, IndexWriter.MaxFieldLength.UNLIMITED).Dispose()); + } + + [TestMethod] + public void full_reindex_builds_a_searchable_index() + { + reIndex(); + assertIndexIsUsable(); + } + + [TestMethod] + public void full_reindex_repairs_a_zero_length_segments_file() + { + reIndex(); + truncateCurrentSegmentsFile(0); + + reIndex(); + assertIndexIsUsable(); + } + + [TestMethod] + public void full_reindex_repairs_a_partially_written_segments_file() + { + reIndex(); + truncateCurrentSegmentsFile(File.ReadAllBytes(currentSegmentsFile()).Length / 2); + + reIndex(); + assertIndexIsUsable(); + } + + /// + /// The nastiest variant: the current commit is intact, so the index looks fine, but IndexWriter's IndexFileDeleter + /// reads every segments_* file it finds and one unreadable leftover poisons the whole directory. + /// + [TestMethod] + public void full_reindex_repairs_a_stale_unreadable_segments_file_beside_a_valid_commit() + { + reIndex(); + File.WriteAllBytes(Path.Combine(indexDirectory, "segments_9"), []); + + reIndex(); + assertIndexIsUsable(); + } + + [TestMethod] + public void full_reindex_repairs_a_checksum_mismatch() + { + reIndex(); + var file = currentSegmentsFile(); + var bytes = File.ReadAllBytes(file); + bytes[^1] ^= 0xFF; + File.WriteAllBytes(file, bytes); + + reIndex(); + assertIndexIsUsable(); + } + + /// Cloud sync (eg. OneDrive) leaves conflict copies whose names Lucene 3's segments parser rejects. + [TestMethod] + public void full_reindex_repairs_cloud_sync_debris() + { + reIndex(); + File.WriteAllText(Path.Combine(indexDirectory, "segments_2 (1)"), "conflict copy"); + + reIndex(); + assertIndexIsUsable(); + } + + [TestMethod] + public void search_repairs_a_zero_length_segments_file() + { + reIndex(); + truncateCurrentSegmentsFile(0); + + // the query path recovers via SearchEngineCommands, which keys off IsRecoverableCorruptIndexException + var ex = Assert.ThrowsExactly(() => new SearchEngine(indexDirectory).Search("Baskervilles")); + SearchEngine.IsRecoverableCorruptIndexException(ex).Should().BeTrue(); + } + + [TestMethod] + public void an_unreadable_segments_file_is_classified_as_recoverable_corruption() + { + reIndex(); + truncateCurrentSegmentsFile(0); + + var ex = exceptionFromOpeningWriter(indexDirectory); + + (ex is CorruptIndexException).Should().BeFalse(); + ex.Message.Should().Be("read past EOF"); + SearchEngine.IsRecoverableCorruptIndexException(ex).Should().BeTrue(); + } + + /// + /// A held write.lock is transient and must keep being retried. It derives from , so + /// widening the corruption check to cover IOException must not swallow it and delete a healthy index. + /// + [TestMethod] + public void a_write_lock_conflict_is_not_classified_as_corruption() + { + SearchEngine.IsRecoverableCorruptIndexException(new LockObtainFailedException("Lock obtain timed out")).Should().BeFalse(); + SearchEngine.IsRecoverableCorruptIndexException(new UnauthorizedAccessException()).Should().BeFalse(); + } + + [TestMethod] + public void full_reindex_waits_out_a_write_lock_conflict_without_deleting_the_index() + { + reIndex(); + + using var index = FSDirectory.Open(indexDirectory); + var writeLock = index.MakeLock(IndexWriter.WRITE_LOCK_NAME); + writeLock.Obtain().Should().BeTrue(); + + // released inside the retry budget (400 + 800 + 1600 + 3200 ms) so the re-index should succeed on a later attempt + var releaser = System.Threading.Tasks.Task.Run(() => + { + System.Threading.Thread.Sleep(600); + // a competing NativeFSLock.Obtain deletes the lock file it failed to take, so by now Release may have + // nothing left to delete. The point of this test is the re-index, not Lucene 3's lock bookkeeping. + try { writeLock.Release(); } + catch (LockReleaseFailedException) { } + }); + + reIndex(); + releaser.Wait(); + + assertIndexIsUsable(); + } +} From cacff3c71b53a05e62780f8c59808070a9b54bbd Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 16:30:46 +0000 Subject: [PATCH 2/8] Cover a garbled segments.gen in the search index recovery tests Lucene 3's base-36 filename formatter overruns its buffer when segments.gen names an absurd generation, so the rebuild path has to survive an IndexOutOfRangeException as well as the IOException shapes. Damage does not always announce itself as IO. Co-authored-by: rmcrackan --- .../CorruptIndexRecoveryTests.cs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/Source/_Tests/LibationSearchEngine.Tests/CorruptIndexRecoveryTests.cs b/Source/_Tests/LibationSearchEngine.Tests/CorruptIndexRecoveryTests.cs index 2ff3b0a0..53b58963 100644 --- a/Source/_Tests/LibationSearchEngine.Tests/CorruptIndexRecoveryTests.cs +++ b/Source/_Tests/LibationSearchEngine.Tests/CorruptIndexRecoveryTests.cs @@ -142,6 +142,25 @@ public class CorruptIndexRecoveryTests assertIndexIsUsable(); } + /// + /// A garbled segments.gen sends Lucene looking for an absurd generation, and Lucene 3's base-36 filename + /// formatter overruns its buffer on the way. Damage does not always announce itself as an IOException. + /// + [TestMethod] + public void full_reindex_repairs_a_segments_gen_pointing_at_a_bogus_generation() + { + reIndex(); + var gen = Path.Combine(indexDirectory, "segments.gen"); + // format(int) then the generation as a long, written twice; corrupt the high bytes of both copies + var bytes = File.ReadAllBytes(gen); + bytes[7] = 0x63; + bytes[15] = 0x63; + File.WriteAllBytes(gen, bytes); + + reIndex(); + assertIndexIsUsable(); + } + [TestMethod] public void search_repairs_a_zero_length_segments_file() { From 79bdbe6d76270c0c75468bb4ad3ce7da62b1a9ee Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 16:44:58 +0000 Subject: [PATCH 3/8] Tell the user how to delete the search index when repair fails Ported from #1949, which surfaces the manual recovery steps the maintainer had been giving out by hand instead of leaving the user with a raw Lucene error. Adapted to the failure now being contained: with the exception no longer escaping into the library change, the scan-failure catch blocks #1949 hooked would never see it, and hooking only those would still miss every other trigger -- removing books is what crashed the GUI. So the guard moves from AppScaffolding into SearchEngineCommands next to the update commands it protects, and raises UpdateFailed from there. Both GUIs subscribe, so any trigger is covered, and the event carries the exception rather than needing #1949's StackTrace string sniffing to find it. The dialog is shown once per session: a damaged index fails on every library change and these steps only need following once. Co-authored-by: rmcrackan --- Source/AppScaffolding/LibationScaffolding.cs | 21 +--- .../SearchEngineCommands.cs | 32 +++++++ .../ViewModels/MainVM.SearchIndex.cs | 33 +++++++ Source/LibationAvalonia/ViewModels/MainVM.cs | 1 + Source/LibationUiBase/SearchIndexRecovery.cs | 29 ++++++ Source/LibationWinForms/Form1.SearchIndex.cs | 42 ++++++++ Source/LibationWinForms/Form1.cs | 1 + .../SearchIndexUpdateGuardTests.cs | 95 +++++++++++++++++++ .../SearchIndexRecoveryTests.cs | 26 +++++ 9 files changed, 261 insertions(+), 19 deletions(-) create mode 100644 Source/LibationAvalonia/ViewModels/MainVM.SearchIndex.cs create mode 100644 Source/LibationUiBase/SearchIndexRecovery.cs create mode 100644 Source/LibationWinForms/Form1.SearchIndex.cs create mode 100644 Source/_Tests/ApplicationServices.Tests/SearchIndexUpdateGuardTests.cs create mode 100644 Source/_Tests/LibationUiBase.Tests/SearchIndexRecoveryTests.cs diff --git a/Source/AppScaffolding/LibationScaffolding.cs b/Source/AppScaffolding/LibationScaffolding.cs index 5de6a0d9..f7fc6a09 100644 --- a/Source/AppScaffolding/LibationScaffolding.cs +++ b/Source/AppScaffolding/LibationScaffolding.cs @@ -341,27 +341,10 @@ public static class LibationScaffolding private static void wireUpSystemEvents(Configuration configuration) { LibraryCommands.LibrarySizeChanged += (_, libraryBooks) - => updateSearchIndex(() => SearchEngineCommands.FullReIndex(libraryBooks)); + => SearchEngineCommands.OnLibrarySizeChanged(libraryBooks); LibraryCommands.BookUserDefinedItemCommitted += (_, books) - => updateSearchIndex(() => SearchEngineCommands.UpdateBooks(books)); - } - - /// - /// The search index is a cache derived from the database, and these events fire after the database change is - /// already committed. Letting a search index failure escape would report a successful scan as a failed import and - /// would stop the remaining event handlers -- the ones that refresh the grid and the backup counts -- from running. - /// - private static void updateSearchIndex(Action update) - { - try - { - update(); - } - catch (Exception ex) - { - Serilog.Log.Logger.Error(ex, "Failed to update the search index. Library changes are saved; search and filter results may be stale until the next update succeeds."); - } + => SearchEngineCommands.OnBookUserDefinedItemCommitted(books); } public static VersionCheckResult GetLatestRelease() diff --git a/Source/ApplicationServices/SearchEngineCommands.cs b/Source/ApplicationServices/SearchEngineCommands.cs index ebf46368..00d1089b 100644 --- a/Source/ApplicationServices/SearchEngineCommands.cs +++ b/Source/ApplicationServices/SearchEngineCommands.cs @@ -44,9 +44,41 @@ public static class SearchEngineCommands public static event EventHandler? SearchEngineUpdated; + /// + /// Occurs when the index could not be updated even after automatic repair, so it needs the user's help. + /// + public static event EventHandler? UpdateFailed; + #region Update private static bool isUpdating; + /// Updates the index after books were added to or removed from the library. + public static void OnLibrarySizeChanged(List libraryBooks) + => tryUpdate(() => FullReIndex(libraryBooks)); + + /// Updates the index after book details, tags or statuses were committed. + public static void OnBookUserDefinedItemCommitted(IEnumerable books) + => tryUpdate(() => UpdateBooks(books)); + + /// + /// The database change that triggers an update is committed before the update runs, and this index is derived + /// from that database, so a failure here is reported instead of propagated. Letting it escape reported a + /// successful scan as "Error importing library", and, since this is the first subscriber to those events, + /// stopped the handlers that refresh the grid and the backup counts from running at all. + /// + private static void tryUpdate(Action update) + { + try + { + update(); + } + catch (Exception ex) + { + Log.Error(ex, "Failed to update the search index. Library changes are saved; search and filter results may be stale until the next update succeeds."); + UpdateFailed?.Invoke(null, ex); + } + } + public static void UpdateBooks(IEnumerable books) { // Semi-arbitrary. At some point it's more worth it to do a full re-index than to do one offs. diff --git a/Source/LibationAvalonia/ViewModels/MainVM.SearchIndex.cs b/Source/LibationAvalonia/ViewModels/MainVM.SearchIndex.cs new file mode 100644 index 00000000..1e2a5a5c --- /dev/null +++ b/Source/LibationAvalonia/ViewModels/MainVM.SearchIndex.cs @@ -0,0 +1,33 @@ +using ApplicationServices; +using LibationUiBase; +using LibationUiBase.Forms; +using System; + +namespace LibationAvalonia.ViewModels; + +partial class MainVM +{ + private void Configure_SearchIndex() + => SearchEngineCommands.UpdateFailed += searchIndexUpdateFailed; + + private async void searchIndexUpdateFailed(object? sender, Exception ex) + { + try + { + if (!SearchIndexRecovery.ShouldNotify()) + return; + + await MessageBox.Show( + MainWindow, + SearchIndexRecovery.ManualRecoveryInstructions, + SearchIndexRecovery.Caption, + MessageBoxButtons.OK, + MessageBoxIcon.Warning); + } + catch (Exception dialogEx) + { + // nothing above this is allowed to fail: the library change that got us here already succeeded + Serilog.Log.Logger.Error(dialogEx, "Could not show the search index recovery instructions"); + } + } +} diff --git a/Source/LibationAvalonia/ViewModels/MainVM.cs b/Source/LibationAvalonia/ViewModels/MainVM.cs index 27e5d5bb..02972a7e 100644 --- a/Source/LibationAvalonia/ViewModels/MainVM.cs +++ b/Source/LibationAvalonia/ViewModels/MainVM.cs @@ -34,6 +34,7 @@ public partial class MainVM : ViewModelBase Configure_Liberate(); Configure_ProcessQueue(); Configure_ScanAuto(); + Configure_SearchIndex(); Configure_Settings(); Configure_VisibleBooks(); } diff --git a/Source/LibationUiBase/SearchIndexRecovery.cs b/Source/LibationUiBase/SearchIndexRecovery.cs new file mode 100644 index 00000000..5817e852 --- /dev/null +++ b/Source/LibationUiBase/SearchIndexRecovery.cs @@ -0,0 +1,29 @@ +using System.Threading; + +namespace LibationUiBase; + +/// +/// Shared copy for telling the user that Libation's search index needs to be deleted by hand. +/// Reached only after the automatic delete-and-rebuild has already failed. +/// +public static class SearchIndexRecovery +{ + public const string Caption = "Search index needs attention"; + + public const string ManualRecoveryInstructions + = "Libation could not update its search index, and could not repair it automatically. " + + "Your library itself is fine -- only searching and filtering are affected.\n\n" + + "To fix it by hand:\n" + + "1. In Settings, click 'Open log folder'\n" + + "2. Close Libation\n" + + "3. Delete the SearchEngine folder you find there\n" + + "4. Start Libation again"; + + private static int notified; + + /// + /// True the first time the index fails in this session, false afterwards. A damaged index fails on every + /// library change, and these steps only need following once, so repeats are left to the log. + /// + public static bool ShouldNotify() => Interlocked.Exchange(ref notified, 1) == 0; +} diff --git a/Source/LibationWinForms/Form1.SearchIndex.cs b/Source/LibationWinForms/Form1.SearchIndex.cs new file mode 100644 index 00000000..91460e0c --- /dev/null +++ b/Source/LibationWinForms/Form1.SearchIndex.cs @@ -0,0 +1,42 @@ +using ApplicationServices; +using LibationUiBase; +using System; +using System.Windows.Forms; + +namespace LibationWinForms; + +public partial class Form1 +{ + private void Configure_SearchIndex() + => SearchEngineCommands.UpdateFailed += searchIndexUpdateFailed; + + private void searchIndexUpdateFailed(object? sender, Exception ex) + { + try + { + if (!SearchIndexRecovery.ShouldNotify()) + return; + + if (InvokeRequired) + { + BeginInvoke(showSearchIndexRecoveryInstructions); + return; + } + + showSearchIndexRecoveryInstructions(); + } + catch (Exception dialogEx) + { + // nothing above this is allowed to fail: the library change that got us here already succeeded + Serilog.Log.Logger.Error(dialogEx, "Could not show the search index recovery instructions"); + } + } + + private void showSearchIndexRecoveryInstructions() + => MessageBox.Show( + this, + SearchIndexRecovery.ManualRecoveryInstructions, + SearchIndexRecovery.Caption, + MessageBoxButtons.OK, + MessageBoxIcon.Warning); +} diff --git a/Source/LibationWinForms/Form1.cs b/Source/LibationWinForms/Form1.cs index 92a56e9d..5f9c7c06 100644 --- a/Source/LibationWinForms/Form1.cs +++ b/Source/LibationWinForms/Form1.cs @@ -37,6 +37,7 @@ public partial class Form1 : Form // eg: if one of these init'd productsGrid, then another can't reliably subscribe to it Configure_BackupCounts(); Configure_ScanAuto(); + Configure_SearchIndex(); Configure_ScanNotification(); Configure_VisibleBooks(); Configure_QuickFilters(); diff --git a/Source/_Tests/ApplicationServices.Tests/SearchIndexUpdateGuardTests.cs b/Source/_Tests/ApplicationServices.Tests/SearchIndexUpdateGuardTests.cs new file mode 100644 index 00000000..838b2840 --- /dev/null +++ b/Source/_Tests/ApplicationServices.Tests/SearchIndexUpdateGuardTests.cs @@ -0,0 +1,95 @@ +using ApplicationServices; +using AssertionHelper; +using DataLayer; +using LibationFileManager; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using System; +using System.Collections.Generic; +using System.IO; + +namespace SearchIndexUpdateGuardTests; + +/// +/// A library change is committed to the database before the search index is updated, so a failure to update that +/// index must be reported rather than propagated. Letting it escape reported a successful scan as +/// "Error importing library" and starved the event's remaining subscribers. +/// +[TestClass] +[DoNotParallelize] +public class SearchIndexUpdateGuardTests +{ + private string tempLibationFiles = string.Empty; + private readonly List reportedFailures = []; + + [TestInitialize] + public void Initialize() + { + tempLibationFiles = Path.Combine(Path.GetTempPath(), $"libation-search-index-guard-tests-{Guid.NewGuid():N}"); + Directory.CreateDirectory(tempLibationFiles); + + // A fresh Configuration resolves LibationFiles from this variable, so the index lands in the temp dir. + Environment.SetEnvironmentVariable(LibationFiles.LIBATION_FILES_DIR, tempLibationFiles); + Configuration.CreateMockInstance(); + + SearchEngineCommands.UpdateFailed += recordFailure; + } + + [TestCleanup] + public void Cleanup() + { + SearchEngineCommands.UpdateFailed -= recordFailure; + Configuration.RestoreSingletonInstance(); + Environment.SetEnvironmentVariable(LibationFiles.LIBATION_FILES_DIR, null); + + try + { + Directory.Delete(tempLibationFiles, recursive: true); + } + catch (IOException) + { + // a leftover temp directory is not worth failing a test over + } + } + + private void recordFailure(object? sender, Exception ex) => reportedFailures.Add(ex); + + /// Occupying the SearchEngine path with a file leaves the engine nowhere to build its index. + private void blockTheIndexDirectory() + => File.WriteAllText(Path.Combine(tempLibationFiles, "SearchEngine"), "not a directory"); + + private static List library() + { + var contributor = Contributor.GetEmpty(); + var book = new Book(new AudibleProductId("B0TEST0001"), "Hound of the Baskervilles", null, null, 1, ContentType.Product, [contributor], [contributor], "us"); + return [new LibraryBook(book, new DateTime(2026, 8, 15), "account")]; + } + + [TestMethod] + public void a_library_size_change_survives_an_unusable_search_index() + { + blockTheIndexDirectory(); + + SearchEngineCommands.OnLibrarySizeChanged(library()); + + reportedFailures.Should().HaveCount(1); + } + + [TestMethod] + public void a_book_detail_change_survives_an_unusable_search_index() + { + blockTheIndexDirectory(); + + SearchEngineCommands.OnBookUserDefinedItemCommitted(library()); + + reportedFailures.Should().HaveCount(1); + } + + [TestMethod] + public void a_successful_update_reports_no_failure() + { + SearchEngineCommands.OnLibrarySizeChanged(library()); + + reportedFailures.Should().HaveCount(0); + Directory.Exists(Path.Combine(tempLibationFiles, "SearchEngine")).Should().BeTrue(); + } +} diff --git a/Source/_Tests/LibationUiBase.Tests/SearchIndexRecoveryTests.cs b/Source/_Tests/LibationUiBase.Tests/SearchIndexRecoveryTests.cs new file mode 100644 index 00000000..0cac5430 --- /dev/null +++ b/Source/_Tests/LibationUiBase.Tests/SearchIndexRecoveryTests.cs @@ -0,0 +1,26 @@ +using LibationUiBase; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace LibationUiBase.Tests; + +[TestClass] +public class SearchIndexRecoveryTests +{ + [TestMethod] + public void instructions_name_the_folder_and_how_to_find_it() + { + StringAssert.Contains(SearchIndexRecovery.ManualRecoveryInstructions, "SearchEngine"); + StringAssert.Contains(SearchIndexRecovery.ManualRecoveryInstructions, "Open log folder"); + // the whole point of the guard is that the library survived + StringAssert.Contains(SearchIndexRecovery.ManualRecoveryInstructions, "library itself is fine"); + } + + /// A damaged index fails on every library change, and these steps only need following once. + [TestMethod] + public void the_user_is_told_once_per_session() + { + Assert.IsTrue(SearchIndexRecovery.ShouldNotify()); + Assert.IsFalse(SearchIndexRecovery.ShouldNotify()); + Assert.IsFalse(SearchIndexRecovery.ShouldNotify()); + } +} From 0f4cfac3b0ebd67e6195c5b5b2778ba963313f40 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 16:55:46 +0000 Subject: [PATCH 4/8] Name the account and the real cause when auto-scan pauses for a login Ported from #1949. The reporter's log paused auto-scan on a second account that had never been logged in, while the dialog blamed an expired session and named no account, so there was nothing to act on. AccountCredentialStatus tells a never-registered account apart from one holding an expired access token, by looking for a refresh token to renew from. AutoScanRunner now hands the AuthenticationRequiredException to the notification so the prompt can name the account, which means digging that exception back out of the wrappers the scan adds on the way up. Same distinction in the log line and in the exception message ApiExtended throws when interactive login is unavailable, which is what the CLI and Docker users see. Co-authored-by: rmcrackan --- .../AccountCredentialStatus.cs | 34 +++++++++ Source/AudibleUtilities/ApiExtended.cs | 14 +++- .../AuthenticationExceptionHelper.cs | 24 ++++++ .../ViewModels/MainVM.ScanAuto.cs | 7 +- Source/LibationUiBase/AutoScanAuthPrompt.cs | 23 ++++++ Source/LibationUiBase/AutoScanRunner.cs | 18 +++-- Source/LibationUiBase/SearchIndexRecovery.cs | 2 +- Source/LibationWinForms/Form1.ScanAuto.cs | 13 ++-- .../AccountCredentialStatusTests.cs | 74 +++++++++++++++++++ .../FindAuthenticationRequiredTests.cs | 46 ++++++++++++ .../AutoScanAuthPromptTests.cs | 71 ++++++++++++++++++ 11 files changed, 305 insertions(+), 21 deletions(-) create mode 100644 Source/AudibleUtilities/AccountCredentialStatus.cs create mode 100644 Source/LibationUiBase/AutoScanAuthPrompt.cs create mode 100644 Source/_Tests/AudibleUtilities.Tests/AccountCredentialStatusTests.cs create mode 100644 Source/_Tests/AudibleUtilities.Tests/FindAuthenticationRequiredTests.cs create mode 100644 Source/_Tests/LibationUiBase.Tests/AutoScanAuthPromptTests.cs diff --git a/Source/AudibleUtilities/AccountCredentialStatus.cs b/Source/AudibleUtilities/AccountCredentialStatus.cs new file mode 100644 index 00000000..d6b463e1 --- /dev/null +++ b/Source/AudibleUtilities/AccountCredentialStatus.cs @@ -0,0 +1,34 @@ +using Dinah.Core; + +namespace AudibleUtilities; + +/// Describes whether an account has usable stored Audible tokens. +public static class AccountCredentialStatus +{ + /// + /// True when identity tokens are absent or carry no refresh token, meaning the account was never fully logged + /// in or its credentials were cleared, rather than merely holding an expired access token. + /// + public static bool LooksLikeMissingCredentials(Account? account) + { + if (account?.IdentityTokens is not { } tokens) + return true; + + if (tokens.IsValid) + return false; + + // without a refresh token there is nothing to renew from, so this needs a fresh login rather than a retry + return string.IsNullOrWhiteSpace(tokens.RefreshToken?.Value); + } + + /// Account label for dialogs and log messages. + public static string FormatAccountLabel(Account? account) + { + if (account is null) + return "an Audible account"; + + return string.IsNullOrWhiteSpace(account.AccountName) || account.AccountName.EqualsInsensitive(account.AccountId) + ? $"'{account.AccountId}'" + : $"'{account.AccountName}' ({account.AccountId})"; + } +} diff --git a/Source/AudibleUtilities/ApiExtended.cs b/Source/AudibleUtilities/ApiExtended.cs index 8155b445..2832add0 100644 --- a/Source/AudibleUtilities/ApiExtended.cs +++ b/Source/AudibleUtilities/ApiExtended.cs @@ -59,19 +59,27 @@ public class ApiExtended { if (!allowInteractiveLogin || LoginChoiceFactory is null) { + // an account that was never logged in needs a first login, not a renewal, and saying so saves the + // user hunting for an expired session that never existed + var missingCredentials = AccountCredentialStatus.LooksLikeMissingCredentials(account); + Serilog.Log.Logger.Error(ex, - "Stored credentials could not be used and interactive login is not available. {@DebugInfo}", + missingCredentials + ? "Stored credentials are missing or incomplete and interactive login is not available. {@DebugInfo}" + : "Stored credentials could not be used and interactive login is not available. {@DebugInfo}", new { Account = account.MaskedLogEntry ?? "[null]", LocaleName = locale.Name, AllowInteractiveLogin = allowInteractiveLogin, - LoginChoiceFactorySet = LoginChoiceFactory is not null + LoginChoiceFactorySet = LoginChoiceFactory is not null, + LooksLikeMissingCredentials = missingCredentials }); throw new AuthenticationRequiredException( account, - message: $"Stored credentials for '{account.AccountId}' could not be used" + message: $"Stored credentials for '{account.AccountId}' " + + (missingCredentials ? "are missing or incomplete" : "could not be used") + (LoginChoiceFactory is null ? " and interactive login is not available in this context (CLI/Docker)." : " and interactive login was not allowed.") diff --git a/Source/AudibleUtilities/AuthenticationExceptionHelper.cs b/Source/AudibleUtilities/AuthenticationExceptionHelper.cs index 1608b7d3..a86dcdfe 100644 --- a/Source/AudibleUtilities/AuthenticationExceptionHelper.cs +++ b/Source/AudibleUtilities/AuthenticationExceptionHelper.cs @@ -24,4 +24,28 @@ public static class AuthenticationExceptionHelper return false; } + + /// + /// Finds the in or its inner chain, which is + /// the one that knows which account needs a login. + /// + public static AuthenticationRequiredException? FindAuthenticationRequired(Exception ex) + { + for (var current = ex; current is not null; current = current.InnerException) + { + if (current is AuthenticationRequiredException auth) + return auth; + } + + if (ex is AggregateException aggregate) + { + foreach (var inner in aggregate.InnerExceptions) + { + if (FindAuthenticationRequired(inner) is { } found) + return found; + } + } + + return null; + } } diff --git a/Source/LibationAvalonia/ViewModels/MainVM.ScanAuto.cs b/Source/LibationAvalonia/ViewModels/MainVM.ScanAuto.cs index b548b902..ce7147af 100644 --- a/Source/LibationAvalonia/ViewModels/MainVM.ScanAuto.cs +++ b/Source/LibationAvalonia/ViewModels/MainVM.ScanAuto.cs @@ -34,13 +34,12 @@ partial class MainVM Configuration.Instance.PropertyChanged += startAutoScan; } - private async Task notifyAutoScanAuthRequiredAsync() + private async Task notifyAutoScanAuthRequiredAsync(AuthenticationRequiredException ex) { await MessageBox.Show( MainWindow, - "Libation could not refresh your Audible library because your login session expired.\n\n" - + "Background auto-scan has been paused. Use Import > Scan Library to log in again to resume periodic scans.", - "Auto-scan paused - login required", + AutoScanAuthPrompt.FormatBody(ex), + AutoScanAuthPrompt.Caption, MessageBoxButtons.OK, MessageBoxIcon.Warning); } diff --git a/Source/LibationUiBase/AutoScanAuthPrompt.cs b/Source/LibationUiBase/AutoScanAuthPrompt.cs new file mode 100644 index 00000000..7603edf5 --- /dev/null +++ b/Source/LibationUiBase/AutoScanAuthPrompt.cs @@ -0,0 +1,23 @@ +using AudibleUtilities; +using System; + +namespace LibationUiBase; + +/// Shared copy for the dialog shown when auto-scan pauses itself waiting for a login. +public static class AutoScanAuthPrompt +{ + public const string Caption = "Auto-scan paused - login required"; + + public static string FormatBody(AuthenticationRequiredException ex) + { + ArgumentNullException.ThrowIfNull(ex); + + var account = AccountCredentialStatus.FormatAccountLabel(ex.Account); + var cause = AccountCredentialStatus.LooksLikeMissingCredentials(ex.Account) + ? "that account has never been logged in, or its stored credentials are missing" + : "the stored login for that account expired"; + + return $"Libation could not refresh the Audible library for {account} because {cause}.\n\n" + + "Background auto-scan has been paused. Use Import > Scan Library to log in for that account and resume periodic scans."; + } +} diff --git a/Source/LibationUiBase/AutoScanRunner.cs b/Source/LibationUiBase/AutoScanRunner.cs index 4e96885b..3390f881 100644 --- a/Source/LibationUiBase/AutoScanRunner.cs +++ b/Source/LibationUiBase/AutoScanRunner.cs @@ -16,7 +16,7 @@ public sealed class AutoScanRunner private readonly Func isAutoScanEnabled; private readonly Action pauseTimer; private readonly Action resumeTimer; - private readonly Func? notifyAuthRequired; + private readonly Func? notifyAuthRequired; private bool pausedForAuthentication; @@ -24,7 +24,7 @@ public sealed class AutoScanRunner Func isAutoScanEnabled, Action pauseTimer, Action resumeTimer, - Func? notifyAuthRequired = null) + Func? notifyAuthRequired = null) { this.isAutoScanEnabled = isAutoScanEnabled; this.pauseTimer = pauseTimer; @@ -72,7 +72,11 @@ public sealed class AutoScanRunner } catch (Exception ex) when (AuthenticationExceptionHelper.IsAuthenticationFailure(ex)) { - await pauseForAuthenticationAsync(ex); + // LoginFailedException and the "ADP token is null" case do not name an account, so fall back to one + // that at least carries the original failure + await pauseForAuthenticationAsync( + AuthenticationExceptionHelper.FindAuthenticationRequired(ex) + ?? new AuthenticationRequiredException(account: null, message: ex.Message, innerException: ex)); } catch (Exception ex) { @@ -80,7 +84,7 @@ public sealed class AutoScanRunner } } - private async Task pauseForAuthenticationAsync(Exception ex) + private async Task pauseForAuthenticationAsync(AuthenticationRequiredException ex) { if (pausedForAuthentication) return; @@ -88,9 +92,11 @@ public sealed class AutoScanRunner pausedForAuthentication = true; pauseTimer(); - Log.Warning(ex, "Auto-scan paused: Audible login is required. Log in with Import > Scan Library to resume background scans."); + Log.Warning(ex, + "Auto-scan paused: Audible login is required for {AccountLabel}. Log in with Import > Scan Library to resume background scans.", + AccountCredentialStatus.FormatAccountLabel(ex.Account)); if (notifyAuthRequired is not null) - await notifyAuthRequired(); + await notifyAuthRequired(ex); } } diff --git a/Source/LibationUiBase/SearchIndexRecovery.cs b/Source/LibationUiBase/SearchIndexRecovery.cs index 5817e852..3d98808f 100644 --- a/Source/LibationUiBase/SearchIndexRecovery.cs +++ b/Source/LibationUiBase/SearchIndexRecovery.cs @@ -12,7 +12,7 @@ public static class SearchIndexRecovery public const string ManualRecoveryInstructions = "Libation could not update its search index, and could not repair it automatically. " - + "Your library itself is fine -- only searching and filtering are affected.\n\n" + + "Your library itself is fine; only searching and filtering are affected.\n\n" + "To fix it by hand:\n" + "1. In Settings, click 'Open log folder'\n" + "2. Close Libation\n" diff --git a/Source/LibationWinForms/Form1.ScanAuto.cs b/Source/LibationWinForms/Form1.ScanAuto.cs index cdb9a5cd..55b4d8e5 100644 --- a/Source/LibationWinForms/Form1.ScanAuto.cs +++ b/Source/LibationWinForms/Form1.ScanAuto.cs @@ -39,26 +39,25 @@ public partial class Form1 Configuration.Instance.PropertyChanged += Configuration_PropertyChanged; } - private void notifyAutoScanAuthRequired() + private void notifyAutoScanAuthRequired(AuthenticationRequiredException ex) { MessageBox.Show( this, - "Libation could not refresh your Audible library because your login session expired.\n\n" - + "Background auto-scan has been paused. Use Import > Scan Library to log in again to resume periodic scans.", - "Auto-scan paused - login required", + AutoScanAuthPrompt.FormatBody(ex), + AutoScanAuthPrompt.Caption, MessageBoxButtons.OK, MessageBoxIcon.Warning); } - private Task notifyAutoScanAuthRequiredAsync() + private Task notifyAutoScanAuthRequiredAsync(AuthenticationRequiredException ex) { if (InvokeRequired) { - Invoke(notifyAutoScanAuthRequired); + Invoke(() => notifyAutoScanAuthRequired(ex)); return Task.CompletedTask; } - notifyAutoScanAuthRequired(); + notifyAutoScanAuthRequired(ex); return Task.CompletedTask; } diff --git a/Source/_Tests/AudibleUtilities.Tests/AccountCredentialStatusTests.cs b/Source/_Tests/AudibleUtilities.Tests/AccountCredentialStatusTests.cs new file mode 100644 index 00000000..1adb115e --- /dev/null +++ b/Source/_Tests/AudibleUtilities.Tests/AccountCredentialStatusTests.cs @@ -0,0 +1,74 @@ +using AssertionHelper; +using AudibleApi; +using AudibleApi.Authorization; +using AudibleApi.Cryptography; +using AudibleUtilities; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using System; +using System.Collections.Generic; +using System.Security.Cryptography; + +namespace AudibleUtilities.Tests; + +[TestClass] +public class AccountCredentialStatusTests +{ + private static Account registeredAccount() + { + var identity = new Identity(Localization.Get("us")); + identity.Update( + new PrivateKey(RSA.Create(2048).ExportRSAPrivateKeyPem()), + new AdpToken("{enc:abcdefg}{key:1234}{iv:56789}{name:QURQVG9rZW5FbmNyeXB0aW9uS2V5}{serial:Mg==}"), + new AccessToken("Atna|_CHAR_ACCESS_", new DateTime(2200, 1, 1, 12, 0, 0, DateTimeKind.Utc)), + new RefreshToken("Atnr|_CHAR_REFRESH_"), + new List> { new("session-id", "cookie-value") }, + deviceSerialNumber: "device-serial", + deviceType: "device-type", + amazonAccountId: "amzn-account", + deviceName: "device-name", + storeAuthenticationCookie: "store-auth-cookie"); + + return new Account("user@example.com") { AccountName = "Jade", IdentityTokens = identity }; + } + + [TestMethod] + public void no_account_looks_like_missing_credentials() + => AccountCredentialStatus.LooksLikeMissingCredentials(null).Should().BeTrue(); + + [TestMethod] + public void an_account_that_was_never_logged_in_looks_like_missing_credentials() + => AccountCredentialStatus.LooksLikeMissingCredentials(new Account("user@example.com")).Should().BeTrue(); + + /// A bare Identity has a locale but no tokens to renew from, so it needs a first login. + [TestMethod] + public void tokens_without_a_refresh_token_look_like_missing_credentials() + { + var account = new Account("user@example.com") { IdentityTokens = new Identity(Localization.Get("us")) }; + + AccountCredentialStatus.LooksLikeMissingCredentials(account).Should().BeTrue(); + } + + [TestMethod] + public void a_registered_account_does_not_look_like_missing_credentials() + => AccountCredentialStatus.LooksLikeMissingCredentials(registeredAccount()).Should().BeFalse(); + + [TestMethod] + public void the_label_pairs_a_friendly_name_with_the_id() + => AccountCredentialStatus.FormatAccountLabel(new Account("user@example.com") { AccountName = "Jade" }) + .Should().Be("'Jade' (user@example.com)"); + + [TestMethod] + public void the_label_falls_back_to_the_id_alone() + => AccountCredentialStatus.FormatAccountLabel(new Account("user@example.com")) + .Should().Be("'user@example.com'"); + + /// Accounts default their name to their id, and "'x' (x)" reads like a mistake. + [TestMethod] + public void the_label_does_not_repeat_an_id_used_as_the_name() + => AccountCredentialStatus.FormatAccountLabel(new Account("user@example.com") { AccountName = "USER@example.com" }) + .Should().Be("'user@example.com'"); + + [TestMethod] + public void the_label_stays_readable_without_an_account() + => AccountCredentialStatus.FormatAccountLabel(null).Should().Be("an Audible account"); +} diff --git a/Source/_Tests/AudibleUtilities.Tests/FindAuthenticationRequiredTests.cs b/Source/_Tests/AudibleUtilities.Tests/FindAuthenticationRequiredTests.cs new file mode 100644 index 00000000..e72f3a18 --- /dev/null +++ b/Source/_Tests/AudibleUtilities.Tests/FindAuthenticationRequiredTests.cs @@ -0,0 +1,46 @@ +using AssertionHelper; +using AudibleUtilities; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using System; + +namespace AudibleUtilities.Tests; + +/// +/// The scan wraps failures on the way up, so the exception that knows which account needs a login has to be dug +/// back out before the auto-scan dialog can name it. +/// +[TestClass] +public class FindAuthenticationRequiredTests +{ + [TestMethod] + public void the_exception_itself_is_returned() + { + var auth = new AuthenticationRequiredException(new Account("user@example.com")); + + AuthenticationExceptionHelper.FindAuthenticationRequired(auth).Should().BeSameAs(auth); + } + + [TestMethod] + public void a_wrapped_exception_is_found() + { + var auth = new AuthenticationRequiredException(new Account("user@example.com"), "need login"); + var wrapped = new Exception("Error importing library", new Exception("inner", auth)); + + AuthenticationExceptionHelper.FindAuthenticationRequired(wrapped).Should().BeSameAs(auth); + } + + /// Scanning several accounts at once surfaces failures as an AggregateException. + [TestMethod] + public void an_aggregated_exception_is_found() + { + var auth = new AuthenticationRequiredException(new Account("user@example.com"), "need login"); + var aggregate = new AggregateException(new InvalidOperationException("unrelated"), auth); + + AuthenticationExceptionHelper.FindAuthenticationRequired(aggregate).Should().BeSameAs(auth); + } + + [TestMethod] + public void an_unrelated_exception_yields_nothing() + => AuthenticationExceptionHelper.FindAuthenticationRequired(new InvalidOperationException("unrelated")) + .Should().BeNull(); +} diff --git a/Source/_Tests/LibationUiBase.Tests/AutoScanAuthPromptTests.cs b/Source/_Tests/LibationUiBase.Tests/AutoScanAuthPromptTests.cs new file mode 100644 index 00000000..e2286649 --- /dev/null +++ b/Source/_Tests/LibationUiBase.Tests/AutoScanAuthPromptTests.cs @@ -0,0 +1,71 @@ +using AudibleApi; +using AudibleApi.Authorization; +using AudibleApi.Cryptography; +using AudibleUtilities; +using LibationUiBase; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using System; +using System.Collections.Generic; +using System.Security.Cryptography; + +namespace LibationUiBase.Tests; + +[TestClass] +public class AutoScanAuthPromptTests +{ + private static Account registeredAccount() + { + var identity = new Identity(Localization.Get("us")); + identity.Update( + new PrivateKey(RSA.Create(2048).ExportRSAPrivateKeyPem()), + new AdpToken("{enc:abcdefg}{key:1234}{iv:56789}{name:QURQVG9rZW5FbmNyeXB0aW9uS2V5}{serial:Mg==}"), + new AccessToken("Atna|_CHAR_ACCESS_", new DateTime(2200, 1, 1, 12, 0, 0, DateTimeKind.Utc)), + new RefreshToken("Atnr|_CHAR_REFRESH_"), + new List> { new("session-id", "cookie-value") }, + deviceSerialNumber: "device-serial", + deviceType: "device-type", + amazonAccountId: "amzn-account", + deviceName: "device-name", + storeAuthenticationCookie: "store-auth-cookie"); + + return new Account("jade@example.com") { AccountName = "Jade", IdentityTokens = identity }; + } + + /// + /// The reporter's log paused auto-scan on a second account that had never been logged in, while the dialog + /// blamed an expired session and named no account at all. + /// + [TestMethod] + public void an_account_that_was_never_logged_in_is_named_and_explained() + { + var account = new Account("jade@example.com") { AccountName = "Jade" }; + + var body = AutoScanAuthPrompt.FormatBody(new AuthenticationRequiredException(account, "missing")); + + StringAssert.Contains(body, "'Jade' (jade@example.com)"); + StringAssert.Contains(body, "never been logged in"); + StringAssert.Contains(body, "Import > Scan Library"); + } + + [TestMethod] + public void an_account_with_stored_tokens_is_told_its_login_expired() + { + var body = AutoScanAuthPrompt.FormatBody(new AuthenticationRequiredException(registeredAccount(), "expired")); + + StringAssert.Contains(body, "'Jade' (jade@example.com)"); + StringAssert.Contains(body, "expired"); + } + + [TestMethod] + public void an_unattributed_failure_still_reads_sensibly() + { + var body = AutoScanAuthPrompt.FormatBody(new AuthenticationRequiredException(account: null, "auth failed")); + + StringAssert.Contains(body, "an Audible account"); + StringAssert.Contains(body, "Import > Scan Library"); + } + + [TestMethod] + public void a_missing_exception_is_rejected() + => Assert.ThrowsExactly(() => AutoScanAuthPrompt.FormatBody(null!)); +} From 4409fb6801a2bf262d607811f8da55bc9a2a6e50 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 17:14:57 +0000 Subject: [PATCH 5/8] Make the lock conflict test deterministic on Windows Releasing the write.lock part way through the retry budget raced with Lucene 3's own lock bookkeeping: on Windows a competing Obtain left a handle on the file, so Release and the temp directory cleanup both failed with a sharing violation. Hold the lock for the whole budget instead and assert what actually matters, that a lock conflict is retried and leaves the index files alone. Co-authored-by: rmcrackan --- .../CorruptIndexRecoveryTests.cs | 50 +++++++++++++------ 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/Source/_Tests/LibationSearchEngine.Tests/CorruptIndexRecoveryTests.cs b/Source/_Tests/LibationSearchEngine.Tests/CorruptIndexRecoveryTests.cs index 53b58963..70a634a5 100644 --- a/Source/_Tests/LibationSearchEngine.Tests/CorruptIndexRecoveryTests.cs +++ b/Source/_Tests/LibationSearchEngine.Tests/CorruptIndexRecoveryTests.cs @@ -32,8 +32,16 @@ public class CorruptIndexRecoveryTests [TestCleanup] public void Cleanup() { - if (Directory.Exists(indexDirectory)) - Directory.Delete(indexDirectory, recursive: true); + try + { + if (Directory.Exists(indexDirectory)) + Directory.Delete(indexDirectory, recursive: true); + } + catch (IOException) + { + // Windows refuses to delete a file Lucene still holds open, and a leftover temp directory is not + // worth failing a test over + } } private static LibraryBook book(string asin, string title) @@ -196,28 +204,40 @@ public class CorruptIndexRecoveryTests SearchEngine.IsRecoverableCorruptIndexException(new UnauthorizedAccessException()).Should().BeFalse(); } + /// + /// The risk in repairing anything that is not a lock conflict is over-reach, so a held write.lock has to keep + /// being retried and must leave the index alone. The lock is held for the whole retry budget rather than + /// released part way through, which would race with Lucene 3's own lock bookkeeping. + /// [TestMethod] - public void full_reindex_waits_out_a_write_lock_conflict_without_deleting_the_index() + public void a_write_lock_conflict_is_retried_and_leaves_the_index_intact() { reIndex(); + var indexedBefore = indexFiles(); using var index = FSDirectory.Open(indexDirectory); var writeLock = index.MakeLock(IndexWriter.WRITE_LOCK_NAME); writeLock.Obtain().Should().BeTrue(); - // released inside the retry budget (400 + 800 + 1600 + 3200 ms) so the re-index should succeed on a later attempt - var releaser = System.Threading.Tasks.Task.Run(() => + try { - System.Threading.Thread.Sleep(600); - // a competing NativeFSLock.Obtain deletes the lock file it failed to take, so by now Release may have - // nothing left to delete. The point of this test is the re-index, not Lucene 3's lock bookkeeping. + Assert.ThrowsExactly(() => reIndex()); + + indexFiles().Should().BeEquivalentTo(indexedBefore); + } + finally + { + // a competing NativeFSLock.Obtain can delete the lock file it failed to take, leaving Release nothing + // to delete. Lucene 3's lock bookkeeping is not what this test is about. try { writeLock.Release(); } - catch (LockReleaseFailedException) { } - }); - - reIndex(); - releaser.Wait(); - - assertIndexIsUsable(); + catch (Exception ex) when (ex is LockReleaseFailedException or IOException) { } + } } + + private List indexFiles() + => [.. Directory.GetFiles(indexDirectory) + .Select(Path.GetFileName) + .OfType() + .Where(f => f != IndexWriter.WRITE_LOCK_NAME) + .OrderBy(f => f)]; } From 77bbc1b0a09966f69670df73af596cce93f480b2 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 17:25:01 +0000 Subject: [PATCH 6/8] Treat a Windows sharing violation on write.lock as a lock conflict Windows CI caught real over-reach. When another holder has write.lock, Windows raises the sharing violation before Lucene can turn it into a LockObtainFailedException, so it arrives as a plain IOException. Repairing anything that is not a recognised lock conflict then meant deleting the index the other holder was using -- exactly the second-instance case the retry exists for. An IOException naming Lucene's write lock now counts as a lock conflict. Matching the file name rather than the message wording keeps it working on non-English Windows. The end-to-end test asserts the property instead of the exception type, since the type legitimately differs by platform. Co-authored-by: rmcrackan --- Source/LibationSearchEngine/SearchEngine.cs | 9 +++++++-- .../CorruptIndexRecoveryTests.cs | 12 +++++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/Source/LibationSearchEngine/SearchEngine.cs b/Source/LibationSearchEngine/SearchEngine.cs index b824a06f..6b3c6d9c 100644 --- a/Source/LibationSearchEngine/SearchEngine.cs +++ b/Source/LibationSearchEngine/SearchEngine.cs @@ -137,12 +137,17 @@ public class SearchEngine } /// - /// True when the index could not be opened because something else is holding it, which is worth waiting out. + /// True when the index could not be opened because something else is holding it -- a second Libation instance, + /// antivirus, a backup agent -- which is worth waiting out rather than repairing. /// derives from , so a bare - /// IOException check cannot tell a lock conflict apart from a damaged index. + /// IOException check cannot tell a lock conflict apart from a damaged index. Nor can the type alone: + /// Windows raises a sharing violation on the lock file before Lucene turns it into a + /// , so a plain naming the lock file counts + /// too. Matching the file name rather than the wording keeps that working on non-English Windows. /// private static bool isTransientLockConflict(Exception ex) => ex is LockObtainFailedException + || (ex is IOException && ex.Message.Contains(IndexWriter.WRITE_LOCK_NAME, StringComparison.OrdinalIgnoreCase)) // Windows may report "file in use" as UnauthorizedAccessException || ex is UnauthorizedAccessException; diff --git a/Source/_Tests/LibationSearchEngine.Tests/CorruptIndexRecoveryTests.cs b/Source/_Tests/LibationSearchEngine.Tests/CorruptIndexRecoveryTests.cs index 70a634a5..82146211 100644 --- a/Source/_Tests/LibationSearchEngine.Tests/CorruptIndexRecoveryTests.cs +++ b/Source/_Tests/LibationSearchEngine.Tests/CorruptIndexRecoveryTests.cs @@ -202,6 +202,13 @@ public class CorruptIndexRecoveryTests { SearchEngine.IsRecoverableCorruptIndexException(new LockObtainFailedException("Lock obtain timed out")).Should().BeFalse(); SearchEngine.IsRecoverableCorruptIndexException(new UnauthorizedAccessException()).Should().BeFalse(); + + // Windows raises the sharing violation on the lock file before Lucene can turn it into a + // LockObtainFailedException, so this arrives as a plain IOException. Mistaking it for corruption would + // delete the index the other holder is using. + SearchEngine.IsRecoverableCorruptIndexException( + new IOException(@"The process cannot access the file 'C:\Users\me\Libation\SearchEngine\write.lock' because it is being used by another process.")) + .Should().BeFalse(); } /// @@ -221,8 +228,11 @@ public class CorruptIndexRecoveryTests try { - Assert.ThrowsExactly(() => reIndex()); + // Linux surfaces this as Lucene's LockObtainFailedException, Windows as a sharing violation on the + // lock file. Either way it must not be read as corruption, and the index must survive. + var ex = Assert.Throws(() => reIndex()); + SearchEngine.IsRecoverableCorruptIndexException(ex).Should().BeFalse(); indexFiles().Should().BeEquivalentTo(indexedBefore); } finally From bb55fb40a3391f8d9a93933016cc9f0b983c1d6f Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 18:06:40 +0000 Subject: [PATCH 7/8] Stop the filter box looping on dialogs when the search index is at fault Both grids restored the last good filter by recursing into the filter handler, which never terminated once the search index rather than the query was the problem: the restore fails the same way, and the retry uses the same filter. The user got an endless run of dialogs, each of them blaming a filter string that was fine. Only an empty last-good filter broke the loop, because that short-circuits before reaching the search engine. The fallback is now a bounded sequence -- last good filter, then no filter -- and the message distinguishes an index Libation cannot reach from a query it cannot parse. Only the first failure is reported, so restoring is quiet. A malformed query never surfaces as an IO-family exception, which QueryFailureShapeTests pins against the real engine, so a typo is never mistaken for index trouble or made to trigger a rebuild. Co-authored-by: rmcrackan --- .../ViewModels/MainVM.Filters.cs | 37 +++++++--- Source/LibationSearchEngine/SearchEngine.cs | 3 +- Source/LibationUiBase/SearchIndexRecovery.cs | 18 ++++- Source/LibationWinForms/Form1.Filter.cs | 31 ++++++-- .../QueryFailureShapeTests.cs | 71 +++++++++++++++++++ .../SearchIndexRecoveryTests.cs | 23 ++++++ 6 files changed, 167 insertions(+), 16 deletions(-) create mode 100644 Source/_Tests/LibationSearchEngine.Tests/QueryFailureShapeTests.cs diff --git a/Source/LibationAvalonia/ViewModels/MainVM.Filters.cs b/Source/LibationAvalonia/ViewModels/MainVM.Filters.cs index 825e7cc4..d1dac4f1 100644 --- a/Source/LibationAvalonia/ViewModels/MainVM.Filters.cs +++ b/Source/LibationAvalonia/ViewModels/MainVM.Filters.cs @@ -4,6 +4,7 @@ using Avalonia.Controls; using Avalonia.Data; using Avalonia.Input; using LibationFileManager; +using LibationUiBase; using LibationUiBase.Forms; using ReactiveUI; using System; @@ -56,22 +57,42 @@ partial class MainVM public async Task EditQuickFiltersAsync() => await new LibationAvalonia.Dialogs.EditQuickFilters().ShowDialog(MainWindow); public async Task PerformFilter(QuickFilters.NamedFilter? namedFilter) { - SelectedNamedFilter = namedFilter; var tryFilter = namedFilter?.Filter; + var failure = await applyFilterAsync(namedFilter); + if (failure is null) + return; + + Serilog.Log.Logger.Error(failure, "Error performing filtering. {@namedFilter} {@lastGoodFilter}", namedFilter, lastGoodFilter); + + if (SearchIndexRecovery.IsIndexUnavailable(failure)) + await MessageBox.Show(SearchIndexRecovery.ManualRecoveryInstructions, SearchIndexRecovery.Caption, MessageBoxButtons.OK, MessageBoxIcon.Warning); + else + await MessageBox.Show($"Bad filter string: \"{tryFilter}\"\r\n\r\n{failure.Message}", "Bad filter string", MessageBoxButtons.OK, MessageBoxIcon.Error); + + // Restore the last filter that worked, then give up on filtering entirely. Recursing into PerformFilter + // here never terminated when the search index rather than the query was at fault, because that fails for + // every filter including the one being restored. An empty filter never reaches the search engine. + if (lastGoodSearch.Length > 0 && await applyFilterAsync(lastGoodFilter) is null) + return; + + await applyFilterAsync(new(string.Empty, null)); + } + + /// Applies a filter, returning the exception that stopped it, or null when it worked. + private async Task applyFilterAsync(QuickFilters.NamedFilter? namedFilter) + { + SelectedNamedFilter = namedFilter; + try { - await ProductsDisplay.Filter(tryFilter); + await ProductsDisplay.Filter(namedFilter?.Filter); lastGoodSearch = namedFilter?.Filter ?? ""; + return null; } catch (Exception ex) { - Serilog.Log.Logger.Error(ex, "Error performing filtering. {@namedFilter} {@lastGoodFilter}", namedFilter, lastGoodFilter); - await MessageBox.Show($"Bad filter string: \"{tryFilter}\"\r\n\r\n{ex.Message}", "Bad filter string", MessageBoxButtons.OK, MessageBoxIcon.Error); - - // re-apply last good filter - namedFilter = (namedFilter ?? new(string.Empty, null)) with { Filter = lastGoodSearch }; - await PerformFilter(namedFilter); + return ex; } } diff --git a/Source/LibationSearchEngine/SearchEngine.cs b/Source/LibationSearchEngine/SearchEngine.cs index 6b3c6d9c..5ab1d4a0 100644 --- a/Source/LibationSearchEngine/SearchEngine.cs +++ b/Source/LibationSearchEngine/SearchEngine.cs @@ -125,7 +125,8 @@ public class SearchEngine // reads every segments_* file in the directory and only tolerates missing ones, so a single unreadable // segments file -- even a stale one from an older commit -- keeps the whole directory unusable until it // is deleted. This is a full re-index, so nothing on disk is worth preserving. - Serilog.Log.Logger.Error(ex, "Search index at {Path} could not be opened. Deleting it and rebuilding from the library.", SearchEngineDirectory); + // warning, not error: the index is a cache of the database, so rebuilding it resolves this + Serilog.Log.Logger.Warning(ex, "Search index at {Path} could not be opened. Deleting it and rebuilding from the library.", SearchEngineDirectory); if (deleteAllSearchIndexFiles(SearchEngineDirectory) is { Count: > 0 } undeletable) throw new IOException($"The search index at '{SearchEngineDirectory}' is damaged and could not be deleted automatically. Close Libation, delete that folder, then restart. Undeletable file(s): {string.Join(", ", undeletable)}", ex); diff --git a/Source/LibationUiBase/SearchIndexRecovery.cs b/Source/LibationUiBase/SearchIndexRecovery.cs index 3d98808f..79b3951e 100644 --- a/Source/LibationUiBase/SearchIndexRecovery.cs +++ b/Source/LibationUiBase/SearchIndexRecovery.cs @@ -1,3 +1,6 @@ +using LibationSearchEngine; +using System; +using System.IO; using System.Threading; namespace LibationUiBase; @@ -11,7 +14,7 @@ public static class SearchIndexRecovery public const string Caption = "Search index needs attention"; public const string ManualRecoveryInstructions - = "Libation could not update its search index, and could not repair it automatically. " + = "Libation could not use its search index, and could not repair it automatically. " + "Your library itself is fine; only searching and filtering are affected.\n\n" + "To fix it by hand:\n" + "1. In Settings, click 'Open log folder'\n" @@ -19,11 +22,22 @@ public static class SearchIndexRecovery + "3. Delete the SearchEngine folder you find there\n" + "4. Start Libation again"; + /// + /// True when a search failed because the index could not be reached, rather than because the query was + /// malformed. A damaged index, a held write.lock and a permission problem all arrive as IO-family exceptions, + /// plus the cloud-sync debris that Lucene reports as an ; a query Lucene cannot + /// parse arrives as none of those. Getting this backwards would send the user hunting for a typo in a query + /// that was fine. + /// + public static bool IsIndexUnavailable(Exception ex) + => ex is IOException or UnauthorizedAccessException + || SearchEngine.IsRecoverableCorruptIndexException(ex); + private static int notified; /// /// True the first time the index fails in this session, false afterwards. A damaged index fails on every - /// library change, and these steps only need following once, so repeats are left to the log. + /// library change, and these steps only need following once. /// public static bool ShouldNotify() => Interlocked.Exchange(ref notified, 1) == 0; } diff --git a/Source/LibationWinForms/Form1.Filter.cs b/Source/LibationWinForms/Form1.Filter.cs index 4b0350f0..3c3da6ca 100644 --- a/Source/LibationWinForms/Form1.Filter.cs +++ b/Source/LibationWinForms/Form1.Filter.cs @@ -1,4 +1,5 @@ -using LibationWinForms.Dialogs; +using LibationUiBase; +using LibationWinForms.Dialogs; using System; using System.Windows.Forms; @@ -29,6 +30,28 @@ public partial class Form1 private string? lastGoodFilter = null; private void performFilter(string? filterString) + { + if (applyFilter(filterString) is not Exception failure) + return; + + Serilog.Log.Logger.Error(failure, "Error performing filtering. {@DebugInfo}", new { filterString, lastGoodFilter }); + + if (SearchIndexRecovery.IsIndexUnavailable(failure)) + MessageBox.Show(this, SearchIndexRecovery.ManualRecoveryInstructions, SearchIndexRecovery.Caption, MessageBoxButtons.OK, MessageBoxIcon.Warning); + else + MessageBox.Show(this, $"Bad filter string:\r\n\r\n{failure.Message}", "Bad filter string", MessageBoxButtons.OK, MessageBoxIcon.Error); + + // Restore the last filter that worked, then give up on filtering entirely. Recursing into performFilter + // here never terminated when the search index rather than the query was at fault, because that fails for + // every filter including the one being restored. An empty filter never reaches the search engine. + if (!string.IsNullOrEmpty(lastGoodFilter) && applyFilter(lastGoodFilter) is null) + return; + + applyFilter(string.Empty); + } + + /// Applies a filter, returning the exception that stopped it, or null when it worked. + private Exception? applyFilter(string? filterString) { this.filterSearchTb.Text = filterString; @@ -36,13 +59,11 @@ public partial class Form1 { productsDisplay.Filter(filterString); lastGoodFilter = filterString; + return null; } catch (Exception ex) { - MessageBox.Show($"Bad filter string:\r\n\r\n{ex.Message}", "Bad filter string", MessageBoxButtons.OK, MessageBoxIcon.Error); - - // re-apply last good filter - performFilter(lastGoodFilter); + return ex; } } diff --git a/Source/_Tests/LibationSearchEngine.Tests/QueryFailureShapeTests.cs b/Source/_Tests/LibationSearchEngine.Tests/QueryFailureShapeTests.cs new file mode 100644 index 00000000..d739b375 --- /dev/null +++ b/Source/_Tests/LibationSearchEngine.Tests/QueryFailureShapeTests.cs @@ -0,0 +1,71 @@ +using AssertionHelper; +using LibationSearchEngine; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using System; +using System.Collections.Generic; +using System.IO; +using DataLayer; +using Directory = System.IO.Directory; + +namespace SearchEngineTests; + +/// +/// The filter box has to tell a query the user mistyped apart from an index Libation cannot read: one deserves +/// "bad filter string", the other deserves the recovery steps, and only the latter is worth rebuilding over. +/// +[TestClass] +public class QueryFailureShapeTests +{ + private string indexDirectory = null!; + + [TestInitialize] + public void Initialize() + { + indexDirectory = Path.Combine(Path.GetTempPath(), "LibationSearchEngineTests", Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(indexDirectory); + + var contributor = Contributor.GetEmpty(); + var book = new Book(new AudibleProductId("B0TEST0001"), "Hound of the Baskervilles", null, null, 1, ContentType.Product, [contributor], [contributor], "us"); + new SearchEngine(indexDirectory).CreateNewIndex(new List { new(book, new DateTime(2026, 8, 15), "account") }); + } + + [TestCleanup] + public void Cleanup() + { + try + { + if (Directory.Exists(indexDirectory)) + Directory.Delete(indexDirectory, recursive: true); + } + catch (IOException) { } + } + + [TestMethod] + [DataRow("title:[unclosed")] + [DataRow("*")] + [DataRow("AND OR")] + [DataRow("(((")] + [DataRow(@"title:""unbalanced")] + public void a_malformed_query_does_not_look_like_an_unreachable_index(string searchString) + { + var engine = new SearchEngine(indexDirectory); + + Exception? thrown = null; + try + { + engine.Search(searchString); + } + catch (Exception ex) + { + thrown = ex; + } + + // some of these parse fine and simply match nothing, which is also acceptable + if (thrown is null) + return; + + // what must never happen is a parse failure being read as index trouble and triggering a rebuild + (thrown is IOException).Should().BeFalse(); + SearchEngine.IsRecoverableCorruptIndexException(thrown).Should().BeFalse(); + } +} diff --git a/Source/_Tests/LibationUiBase.Tests/SearchIndexRecoveryTests.cs b/Source/_Tests/LibationUiBase.Tests/SearchIndexRecoveryTests.cs index 0cac5430..24ab3204 100644 --- a/Source/_Tests/LibationUiBase.Tests/SearchIndexRecoveryTests.cs +++ b/Source/_Tests/LibationUiBase.Tests/SearchIndexRecoveryTests.cs @@ -1,5 +1,10 @@ using LibationUiBase; +using Lucene.Net.Index; +using Lucene.Net.QueryParsers; +using Lucene.Net.Store; using Microsoft.VisualStudio.TestTools.UnitTesting; +using System; +using System.IO; namespace LibationUiBase.Tests; @@ -15,6 +20,24 @@ public class SearchIndexRecoveryTests StringAssert.Contains(SearchIndexRecovery.ManualRecoveryInstructions, "library itself is fine"); } + /// + /// Every way of failing to reach the index. Reporting one of these as a bad filter string sends the user + /// looking for a typo in a query that was fine. + /// + [TestMethod] + public void failures_to_reach_the_index_are_told_apart_from_a_bad_query() + { + Assert.IsTrue(SearchIndexRecovery.IsIndexUnavailable(new IOException("read past EOF"))); + Assert.IsTrue(SearchIndexRecovery.IsIndexUnavailable(new CorruptIndexException("checksum mismatch in segments file"))); + Assert.IsTrue(SearchIndexRecovery.IsIndexUnavailable(new LockObtainFailedException("Lock obtain timed out"))); + Assert.IsTrue(SearchIndexRecovery.IsIndexUnavailable(new UnauthorizedAccessException())); + // cloud-sync debris, which Lucene reports while parsing segments file names + Assert.IsTrue(SearchIndexRecovery.IsIndexUnavailable(new ArgumentException("Invalid or unsupported character in number: )"))); + + Assert.IsFalse(SearchIndexRecovery.IsIndexUnavailable(new ParseException("Cannot parse '[unclosed'"))); + Assert.IsFalse(SearchIndexRecovery.IsIndexUnavailable(new ArgumentException("some other argument problem"))); + } + /// A damaged index fails on every library change, and these steps only need following once. [TestMethod] public void the_user_is_told_once_per_session() From 9afcb098851bc6404eedea0ffe4b919be68cba53 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 18:32:37 +0000 Subject: [PATCH 8/8] Keep account addresses out of the auto-scan pause log line The dialog names the account in full because it is shown to whoever owns it, but log files get attached to public issue reports, which is why the codebase has MaskedLogEntry. Naming the account in the log the same way the dialog does would have put real email addresses into every shared log. Co-authored-by: rmcrackan --- Source/AudibleUtilities/AccountCredentialStatus.cs | 5 ++++- Source/LibationUiBase/AutoScanRunner.cs | 5 +++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/Source/AudibleUtilities/AccountCredentialStatus.cs b/Source/AudibleUtilities/AccountCredentialStatus.cs index d6b463e1..eb304353 100644 --- a/Source/AudibleUtilities/AccountCredentialStatus.cs +++ b/Source/AudibleUtilities/AccountCredentialStatus.cs @@ -21,7 +21,10 @@ public static class AccountCredentialStatus return string.IsNullOrWhiteSpace(tokens.RefreshToken?.Value); } - /// Account label for dialogs and log messages. + /// + /// Account label for dialogs shown to the person who owns the account. Not for logs: those get attached to + /// public issue reports, which is what is for. + /// public static string FormatAccountLabel(Account? account) { if (account is null) diff --git a/Source/LibationUiBase/AutoScanRunner.cs b/Source/LibationUiBase/AutoScanRunner.cs index 3390f881..20b4c52e 100644 --- a/Source/LibationUiBase/AutoScanRunner.cs +++ b/Source/LibationUiBase/AutoScanRunner.cs @@ -92,9 +92,10 @@ public sealed class AutoScanRunner pausedForAuthentication = true; pauseTimer(); + // masked, not the label the dialog uses: log files get attached to public issue reports Log.Warning(ex, - "Auto-scan paused: Audible login is required for {AccountLabel}. Log in with Import > Scan Library to resume background scans.", - AccountCredentialStatus.FormatAccountLabel(ex.Account)); + "Auto-scan paused: Audible login is required for {Account}. Log in with Import > Scan Library to resume background scans.", + ex.Account?.MaskedLogEntry ?? "[unknown account]"); if (notifyAuthRequired is not null) await notifyAuthRequired(ex);