From 86927a0eaef46a7c7a21ea8b72f49693ec0964ff Mon Sep 17 00:00:00 2001 From: GabrielDuf Date: Fri, 2 Oct 2026 11:05:56 -0400 Subject: [PATCH 1/2] Announce that a load finished even when it failed When ReloadPackages threw, the catch logged the exception and set IsLoading to false, but never raised FinishedLoading. The packages pages read "LoadsOnStart && !IsLoaded" as still-pending, so the page kept the "Loading packages" illustration forever -- and unlike the race fixed in #5387, navigating away and back re-ran FilterPackages against the same flags, so nothing short of hitting Reload cleared it. Raise the event from the catch too, and have the pages ask HasPendingInitialLoad instead: a load that threw has concluded, even though it produced nothing. IsLoaded stays false on that path, so the callers that read it as "this list was never loaded, go fetch it" -- the IPC package list, the automatic updates editor, the maintenance scheduler's backup -- keep retrying exactly as they did before. A FinishedLoading subscriber that throws unwinds into that same catch, so the announcement is guarded by a flag: a successful load whose subscriber threw must not fire the event a second time and re-run every handler that already ran, nor undo the state the success path just set. The announcement made from the catch is itself wrapped, so ReloadPackages still swallows what it swallowed before. DiscoverablePackagesLoader re-arms its queued search from FinishedLoading, so a thrown load no longer strands that either. --- .../SoftwarePages/PackagesPageViewModel.cs | 4 +- .../AbstractPackageLoader.cs | 23 ++++++++- .../PackageLoaderPipelineTests.cs | 51 +++++++++++++++++++ 3 files changed, 75 insertions(+), 3 deletions(-) diff --git a/src/UniGetUI.Avalonia/ViewModels/SoftwarePages/PackagesPageViewModel.cs b/src/UniGetUI.Avalonia/ViewModels/SoftwarePages/PackagesPageViewModel.cs index 30ce1259ca..a834ea28a6 100644 --- a/src/UniGetUI.Avalonia/ViewModels/SoftwarePages/PackagesPageViewModel.cs +++ b/src/UniGetUI.Avalonia/ViewModels/SoftwarePages/PackagesPageViewModel.cs @@ -683,7 +683,7 @@ public void FilterPackages(bool fromQuery = false) UpdateSubtitle(); PackageCountUpdated?.Invoke(); - bool loadingOrPending = Loader.IsLoading || (LoadsOnStart && !Loader.IsLoaded); + bool loadingOrPending = Loader.IsLoading || (LoadsOnStart && Loader.HasPendingInitialLoad); if (loadingOrPending && FilteredPackages.Count == 0) { @@ -1022,7 +1022,7 @@ public void UpdatePackageCount() // ─── Subtitle ───────────────────────────────────────────────────────────── public void UpdateSubtitle() { - if (Loader.IsLoading || (LoadsOnStart && !Loader.IsLoaded)) + if (Loader.IsLoading || (LoadsOnStart && Loader.HasPendingInitialLoad)) { Subtitle = _stillLoadingSubtitle; return; diff --git a/src/UniGetUI.PackageEngine.PackageLoader/AbstractPackageLoader.cs b/src/UniGetUI.PackageEngine.PackageLoader/AbstractPackageLoader.cs index 44e82c43aa..65c03e2ebc 100644 --- a/src/UniGetUI.PackageEngine.PackageLoader/AbstractPackageLoader.cs +++ b/src/UniGetUI.PackageEngine.PackageLoader/AbstractPackageLoader.cs @@ -37,6 +37,8 @@ public abstract class AbstractPackageLoader public bool LastLoadReportedFailures { get; private set; } + public bool HasPendingInitialLoad => !IsLoaded && !LastLoadReportedFailures; + public DateTime? LastLoadFinishedUtc { get; private set; } private TaskCompletionSource? _loadCompletion; @@ -157,6 +159,8 @@ protected void InvokeFinishedLoadingEvent() public virtual async Task ReloadPackages() { TaskCompletionSource? completion = null; + int current_identifier = 0; + bool finishWasAnnounced = false; try { if (DISABLE_RELOAD) @@ -172,7 +176,7 @@ public virtual async Task ReloadPackages() } LoadOperationIdentifier = new Random().Next(); - int current_identifier = LoadOperationIdentifier; + current_identifier = LoadOperationIdentifier; completion = new TaskCompletionSource( TaskCreationOptions.RunContinuationsAsynchronously ); @@ -258,6 +262,7 @@ public virtual async Task ReloadPackages() { LastLoadFinishedUtc = DateTime.UtcNow; IsLoaded = true; + finishWasAnnounced = true; InvokeFinishedLoadingEvent(); } } @@ -266,6 +271,22 @@ public virtual async Task ReloadPackages() Logger.Error(ex); LastLoadReportedFailures = true; IsLoading = false; + + if ( + !finishWasAnnounced + && completion is not null + && LoadOperationIdentifier == current_identifier + ) + { + try + { + InvokeFinishedLoadingEvent(); + } + catch (Exception announceEx) + { + Logger.Error(announceEx); + } + } } finally { diff --git a/src/UniGetUI.PackageEngine.Tests/PackageLoaderPipelineTests.cs b/src/UniGetUI.PackageEngine.Tests/PackageLoaderPipelineTests.cs index d455850bbe..42ba3861d6 100644 --- a/src/UniGetUI.PackageEngine.Tests/PackageLoaderPipelineTests.cs +++ b/src/UniGetUI.PackageEngine.Tests/PackageLoaderPipelineTests.cs @@ -290,4 +290,55 @@ public async Task ReloadPackages_ReportsSettledLoadState_WhenFinishedLoadingIsRa Assert.False(isLoadingWhenFinished); Assert.True(isLoadedWhenFinished); } + + [Fact] + public async Task ReloadPackages_SettlesAndAnnouncesCompletion_WhenTheLoadFails() + { + var manager = new PackageManagerBuilder() + .WithInstalledPackages(testManager => + [ + new PackageBuilder() + .WithManager(testManager) + .WithId("Contoso.Tool") + .WithVersion("1.0.0") + .Build(), + ]) + .Build(); + TestPackageLoader? loaderReference = null; + Task? waitTakenDuringLoad = null; + var loader = new TestPackageLoader( + [manager], + isPackageValid: _ => + { + waitTakenDuringLoad = loaderReference!.WaitForCurrentLoadAsync(); + throw new InvalidOperationException("the load blew up"); + } + ); + loaderReference = loader; + var recorder = new LoaderEventRecorder(loader); + + await loader.ReloadPackages(); + + Assert.Equal(1, recorder.FinishedLoadingCount); + Assert.False(loader.IsLoading); + Assert.True(loader.LastLoadReportedFailures); + Assert.False(loader.HasPendingInitialLoad); + Assert.NotNull(waitTakenDuringLoad); + Assert.True(waitTakenDuringLoad.IsCompleted); + } + + [Fact] + public async Task ReloadPackages_AnnouncesCompletionOnce_WhenAFinishedLoadingSubscriberThrows() + { + var manager = new PackageManagerBuilder().Build(); + var loader = new TestPackageLoader([manager], loadPackages: _ => []); + var recorder = new LoaderEventRecorder(loader); + loader.FinishedLoading += (_, _) => throw new InvalidOperationException("the subscriber blew up"); + + await loader.ReloadPackages(); + + Assert.Equal(1, recorder.FinishedLoadingCount); + Assert.True(loader.IsLoaded); + Assert.NotNull(loader.LastLoadFinishedUtc); + } } From 3d7f8b2e7cab32558bf92d7c2a76d627a70587fd Mon Sep 17 00:00:00 2001 From: GabrielDuf Date: Fri, 2 Oct 2026 11:40:48 -0400 Subject: [PATCH 2/2] Gate the catch-side load state on the load that failed --- .../AbstractPackageLoader.cs | 5 +-- .../PackageLoaderPipelineTests.cs | 34 +++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/src/UniGetUI.PackageEngine.PackageLoader/AbstractPackageLoader.cs b/src/UniGetUI.PackageEngine.PackageLoader/AbstractPackageLoader.cs index 65c03e2ebc..cf5c2eb491 100644 --- a/src/UniGetUI.PackageEngine.PackageLoader/AbstractPackageLoader.cs +++ b/src/UniGetUI.PackageEngine.PackageLoader/AbstractPackageLoader.cs @@ -269,8 +269,6 @@ public virtual async Task ReloadPackages() catch (Exception ex) { Logger.Error(ex); - LastLoadReportedFailures = true; - IsLoading = false; if ( !finishWasAnnounced @@ -278,6 +276,9 @@ public virtual async Task ReloadPackages() && LoadOperationIdentifier == current_identifier ) { + LastLoadReportedFailures = true; + IsLoading = false; + try { InvokeFinishedLoadingEvent(); diff --git a/src/UniGetUI.PackageEngine.Tests/PackageLoaderPipelineTests.cs b/src/UniGetUI.PackageEngine.Tests/PackageLoaderPipelineTests.cs index 42ba3861d6..1a29a824d3 100644 --- a/src/UniGetUI.PackageEngine.Tests/PackageLoaderPipelineTests.cs +++ b/src/UniGetUI.PackageEngine.Tests/PackageLoaderPipelineTests.cs @@ -340,5 +340,39 @@ public async Task ReloadPackages_AnnouncesCompletionOnce_WhenAFinishedLoadingSub Assert.Equal(1, recorder.FinishedLoadingCount); Assert.True(loader.IsLoaded); Assert.NotNull(loader.LastLoadFinishedUtc); + Assert.False(loader.LastLoadReportedFailures); + } + + [Fact] + public async Task ReloadPackages_LeavesAQueuedReloadUntouched_WhenAFinishedLoadingSubscriberThrows() + { + using var release = new ManualResetEventSlim(true); + var manager = new PackageManagerBuilder().Build(); + var loader = new TestPackageLoader( + [manager], + loadPackages: _ => + { + release.Wait(); + return []; + } + ); + + Task? queued = null; + loader.FinishedLoading += (_, _) => + { + if (queued is not null) return; + release.Reset(); + queued = loader.ReloadPackages(); + }; + loader.FinishedLoading += (_, _) => throw new InvalidOperationException("the subscriber blew up"); + + await loader.ReloadPackages(); + + Assert.NotNull(queued); + Assert.True(loader.IsLoading); + Assert.False(loader.LastLoadReportedFailures); + + release.Set(); + await queued; } }