From 7cf455c55d21736f6d85c6962fe196097114049d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 05:27:30 +0000 Subject: [PATCH 1/3] fix: keep polling intervals when a provider update throws [patch] A persistent 5xx from one repository made BuildSync/RunSync.UpdateAsync throw before restarting their timers, and faulted the Task.WhenAll over the batch, which skipped ProviderRefreshTimer.Restart and pruning. The update loop then re-ran discovery and re-polled back to back. Each sync and each discovery item now runs through SyncGuard, which logs a failure instead of faulting the batch; the sync timers and the provider refresh timer restart in finally blocks. Fixes ktsu-dev/BuildMonitor#299 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01F4JLDZ8V6BpUZBB9ZnsaNt --- BuildMonitor.Test/SyncFailureTests.cs | 111 ++++++++++++++++++++++++++ BuildMonitor/BuildMonitor.cs | 102 +++++++++++++---------- BuildMonitor/BuildSync.cs | 16 +++- BuildMonitor/RunSync.cs | 16 +++- BuildMonitor/SyncGuard.cs | 40 ++++++++++ 5 files changed, 239 insertions(+), 46 deletions(-) create mode 100644 BuildMonitor.Test/SyncFailureTests.cs create mode 100644 BuildMonitor/SyncGuard.cs diff --git a/BuildMonitor.Test/SyncFailureTests.cs b/BuildMonitor.Test/SyncFailureTests.cs new file mode 100644 index 0000000..ba73922 --- /dev/null +++ b/BuildMonitor.Test/SyncFailureTests.cs @@ -0,0 +1,111 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.BuildMonitor.Test; + +using ktsu.Semantics.Strings; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Covers what happens when a provider update throws — a persistent GitHub 5xx, for example. The +/// sync's timer used to be left running, so ShouldUpdate stayed true and the update loop +/// polled the same item again back to back instead of on its interval, and the exception faulted +/// the whole batch (ktsu-dev/BuildMonitor#299). +/// +[TestClass] +public sealed class SyncFailureTests +{ + /// + /// Long enough for a timer that was not restarted to show a visibly shorter time remaining. + /// + private static readonly TimeSpan Elapse = TimeSpan.FromMilliseconds(1100); + + /// + /// A provider whose build and run updates throw for the items named in + /// and record the rest. + /// + private sealed class FailingProvider : BuildProvider + { + internal override BuildProviderName Name { get; } = "Failing".As(); + + internal HashSet Failing { get; } = []; + + internal List Updated { get; } = []; + + internal override Task UpdateRepositoriesAsync(Owner owner) => Task.CompletedTask; + internal override Task UpdateBuildsAsync(Repository repository) => Task.CompletedTask; + internal override Task UpdateBuildAsync(Build build) => Record(build.Name); + internal override Task UpdateRunAsync(Run run) => Record(run.Name); + + private Task Record(string name) + { + if (Failing.Contains(name)) + { + throw new HttpRequestException("502 Bad Gateway"); + } + + lock (Updated) + { + Updated.Add(name); + } + + return Task.CompletedTask; + } + } + + private static Build AddBuild(Repository repository, string name) + { + Build build = repository.CreateBuild(name.As()); + Assert.IsTrue(repository.Builds.TryAdd(build.Id, build)); + return build; + } + + private static Repository CreateRepository(FailingProvider provider) + { + Owner owner = provider.CreateOwner("owner".As()); + return owner.CreateRepository("repo".As()); + } + + [TestMethod] + public async Task AFailedBuildUpdateStillRestartsItsTimer() + { + // Arrange + FailingProvider provider = new(); + provider.Failing.Add("broken"); + BuildSync sync = new() { Build = AddBuild(CreateRepository(provider), "broken") }; + await Task.Delay(Elapse, TestContext.CancellationToken).ConfigureAwait(false); + TimeSpan remainingBefore = sync.TimeRemaining; + + // Act + await sync.UpdateAsync().ConfigureAwait(false); + + // Assert + Assert.IsGreaterThan(remainingBefore, sync.TimeRemaining, "A failed update should restart the timer, so the build waits out its interval before the next poll"); + Assert.IsFalse(sync.ShouldUpdate); + } + + [TestMethod] + public async Task AFailedBuildUpdateDoesNotStopTheOthersInTheBatch() + { + // Arrange + FailingProvider provider = new(); + provider.Failing.Add("broken"); + Repository repository = CreateRepository(provider); + BuildSync broken = new() { Build = AddBuild(repository, "broken") }; + BuildSync healthy = new() { Build = AddBuild(repository, "healthy") }; + + // Act + Task batch = Task.WhenAll(broken.UpdateAsync(), healthy.UpdateAsync()); + await batch.ConfigureAwait(false); + + // Assert + Assert.IsFalse(batch.IsFaulted, "One failing build should not fault the batch, which skips the timer restarts and pruning after it"); + Assert.HasCount(1, provider.Updated); + Assert.AreEqual("healthy", provider.Updated[0], "The healthy build should still be polled"); + } + + /// + /// Gets or sets the test context MSTest injects, used for its cancellation token. + /// + public TestContext TestContext { get; set; } = null!; +} diff --git a/BuildMonitor/BuildMonitor.cs b/BuildMonitor/BuildMonitor.cs index 04eb97b..ba60997 100644 --- a/BuildMonitor/BuildMonitor.cs +++ b/BuildMonitor/BuildMonitor.cs @@ -1504,53 +1504,16 @@ private static async Task UpdateAsync() { if (!ProviderRefreshTimer.IsRunning || ProviderRefreshTimer.Elapsed.TotalSeconds >= ProviderRefreshTimeout) { - // Gather all owners across all providers, skipping providers with low budget - // Discovery operations are low priority and should be skipped when budget is constrained - List<(BuildProvider Provider, Owner Owner)> allOwners = []; - foreach ((BuildProviderName _, BuildProvider? provider) in AppData.BuildProviders) + try { - // Skip discovery for providers in low budget mode - if (provider.IsLowBudget) - { - Log.Info($"{provider.Name}: Skipping discovery due to low budget ({provider.BudgetPercentage:P0} remaining)"); - continue; - } - - foreach ((OwnerName _, Owner? owner) in provider.Owners) - { - allOwners.Add((provider, owner)); - } + await DiscoverAsync().ConfigureAwait(false); } - - // Update repositories concurrently (semaphore limits per-provider concurrency) - await Task.WhenAll(allOwners.Select(x => x.Provider.UpdateRepositoriesAsync(x.Owner))).ConfigureAwait(false); - - // Gather all repositories across all owners - List<(BuildProvider Provider, Repository Repository)> allRepositories = []; - foreach ((BuildProvider provider, Owner owner) in allOwners) + finally { - foreach ((RepositoryId _, Repository? repository) in owner.Repositories) - { - allRepositories.Add((provider, repository)); - } + // Restarted even when discovery fails, so a failure waits out the refresh interval + // instead of re-running discovery straight away (ktsu-dev/BuildMonitor#299). + ProviderRefreshTimer.Restart(); } - - // Update builds concurrently (semaphore limits per-provider concurrency) - await Task.WhenAll(allRepositories.Select(async x => - { - await x.Provider.UpdateBuildsAsync(x.Repository).ConfigureAwait(false); - - // Add new builds to sync collection - foreach ((BuildId _, Build? build) in x.Repository.Builds) - { - _ = BuildSyncCollection.TryAdd(build.Id, new() - { - Build = build, - }); - } - })).ConfigureAwait(false); - - ProviderRefreshTimer.Restart(); } // Update builds and runs concurrently @@ -1562,6 +1525,59 @@ await Task.WhenAll( PruneOrphanedAndCompletedSyncs(); } + private static async Task DiscoverAsync() + { + // Gather all owners across all providers, skipping providers with low budget + // Discovery operations are low priority and should be skipped when budget is constrained + List<(BuildProvider Provider, Owner Owner)> allOwners = []; + foreach ((BuildProviderName _, BuildProvider? provider) in AppData.BuildProviders) + { + // Skip discovery for providers in low budget mode + if (provider.IsLowBudget) + { + Log.Info($"{provider.Name}: Skipping discovery due to low budget ({provider.BudgetPercentage:P0} remaining)"); + continue; + } + + foreach ((OwnerName _, Owner? owner) in provider.Owners) + { + allOwners.Add((provider, owner)); + } + } + + // Update repositories concurrently (semaphore limits per-provider concurrency) + // Each owner is guarded so one that fails does not stop discovery for the rest. + await Task.WhenAll(allOwners.Select(x => SyncGuard.RunAsync( + () => x.Provider.UpdateRepositoriesAsync(x.Owner), + $"{x.Provider.Name}: repository discovery for {x.Owner.Name}"))).ConfigureAwait(false); + + // Gather all repositories across all owners + List<(BuildProvider Provider, Repository Repository)> allRepositories = []; + foreach ((BuildProvider provider, Owner owner) in allOwners) + { + foreach ((RepositoryId _, Repository? repository) in owner.Repositories) + { + allRepositories.Add((provider, repository)); + } + } + + // Update builds concurrently (semaphore limits per-provider concurrency) + // Each repository is guarded so one that fails does not stop discovery for the rest. + await Task.WhenAll(allRepositories.Select(x => SyncGuard.RunAsync(async () => + { + await x.Provider.UpdateBuildsAsync(x.Repository).ConfigureAwait(false); + + // Add new builds to sync collection + foreach ((BuildId _, Build? build) in x.Repository.Builds) + { + _ = BuildSyncCollection.TryAdd(build.Id, new() + { + Build = build, + }); + } + }, $"{x.Provider.Name}: build discovery for {x.Repository.Owner.Name}/{x.Repository.Name}"))).ConfigureAwait(false); + } + private static void PruneOrphanedAndCompletedSyncs() { // Prune orphaned build syncs (builds that no longer exist in their repository) diff --git a/BuildMonitor/BuildSync.cs b/BuildMonitor/BuildSync.cs index a761be4..5a0d8a2 100644 --- a/BuildMonitor/BuildSync.cs +++ b/BuildMonitor/BuildSync.cs @@ -80,6 +80,20 @@ internal RequestPriority Priority internal void ResetTimer() => UpdateTimer.Restart(); internal async Task UpdateAsync() + { + // Restart the timer whether or not the update succeeds. A failed update that left it running + // kept ShouldUpdate true, so the build was polled again back to back (ktsu-dev/BuildMonitor#299). + try + { + _ = await SyncGuard.RunAsync(UpdateBuildAsync, $"BuildSync: {Build.Owner.Name}/{Build.Repository.Name}/{Build.Name}").ConfigureAwait(false); + } + finally + { + UpdateTimer.Restart(); + } + } + + private async Task UpdateBuildAsync() { int runsBefore = Build.Runs.Count; await Build.Owner.BuildProvider.UpdateBuildAsync(Build).ConfigureAwait(false); @@ -101,7 +115,5 @@ internal async Task UpdateAsync() Run = run, }); } - - UpdateTimer.Restart(); } } diff --git a/BuildMonitor/RunSync.cs b/BuildMonitor/RunSync.cs index 3bc9532..baa8f4c 100644 --- a/BuildMonitor/RunSync.cs +++ b/BuildMonitor/RunSync.cs @@ -29,6 +29,21 @@ internal sealed class RunSync internal RunSync() => UpdateTimer.Start(); internal async Task UpdateAsync() + { + // Restart the timer whether or not the update succeeds, so a run whose update keeps failing + // is retried on its interval rather than back to back (ktsu-dev/BuildMonitor#299). A run that + // is no longer ongoing never updates again, so restarting its timer changes nothing. + try + { + _ = await SyncGuard.RunAsync(UpdateRunAsync, $"RunSync: {Run.Owner.Name}/{Run.Repository.Name}/{Run.Build.Name}/{Run.Name}").ConfigureAwait(false); + } + finally + { + UpdateTimer.Restart(); + } + } + + private async Task UpdateRunAsync() { await Run.Owner.BuildProvider.UpdateRunAsync(Run).ConfigureAwait(false); @@ -36,7 +51,6 @@ internal async Task UpdateAsync() { UpdateIntervalCurrent = (int)Run.CalculateETA().TotalSeconds; UpdateIntervalCurrent = Math.Clamp(UpdateIntervalCurrent, UpdateIntervalMin, UpdateIntervalMax); - UpdateTimer.Restart(); } } } diff --git a/BuildMonitor/SyncGuard.cs b/BuildMonitor/SyncGuard.cs new file mode 100644 index 0000000..a5c484b --- /dev/null +++ b/BuildMonitor/SyncGuard.cs @@ -0,0 +1,40 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.BuildMonitor; + +/// +/// Runs one item of a batched provider update so that its failure is logged instead of faulting +/// the whole batch. +/// +/// +/// The update loop awaits each batch with . A single +/// exception — a persistent 5xx from one repository, say — used to fault that batch, which skipped +/// the timer restarts and pruning after it and made the loop start over straight away instead of on +/// its interval (ktsu-dev/BuildMonitor#299). +/// +internal static class SyncGuard +{ + /// + /// Runs , logging any exception it throws rather than propagating it. + /// + /// The update to run. + /// What is being updated, for the log line. + /// True when the update completed, false when it threw. + internal static async Task RunAsync(Func update, string description) + { + Ensure.NotNull(update); + + try + { + await update().ConfigureAwait(false); + return true; + } +#pragma warning disable CA1031 // Do not catch general exception types - one failing item must not stop the rest of the batch + catch (Exception ex) +#pragma warning restore CA1031 // Do not catch general exception types + { + Log.Error($"{description} failed: {ex.Message}"); + return false; + } + } +} From ac34d67a37a49c56ddd785c1ab9234d0e26f4250 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 05:36:31 +0000 Subject: [PATCH 2/3] fix: keep the discovery change in place and cover RunSync [patch] Narrow the BuildMonitor.cs change to guarding each discovery item: once no item can fault the batch, the ProviderRefreshTimer restart after it is always reached, so the method no longer needs moving. Add a test that a failing run update neither faults its batch nor stays due. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01F4JLDZ8V6BpUZBB9ZnsaNt --- BuildMonitor.Test/SyncFailureTests.cs | 29 +++++++ BuildMonitor/BuildMonitor.cs | 107 ++++++++++++-------------- 2 files changed, 77 insertions(+), 59 deletions(-) diff --git a/BuildMonitor.Test/SyncFailureTests.cs b/BuildMonitor.Test/SyncFailureTests.cs index ba73922..1840d3a 100644 --- a/BuildMonitor.Test/SyncFailureTests.cs +++ b/BuildMonitor.Test/SyncFailureTests.cs @@ -104,6 +104,35 @@ public async Task AFailedBuildUpdateDoesNotStopTheOthersInTheBatch() Assert.AreEqual("healthy", provider.Updated[0], "The healthy build should still be polled"); } + [TestMethod] + public async Task AFailedRunUpdateDoesNotStopTheOthersInTheBatch() + { + // Arrange + FailingProvider provider = new(); + provider.Failing.Add("broken"); + Build build = AddBuild(CreateRepository(provider), "build"); + RunSync broken = new() { Run = AddRun(build, "broken") }; + RunSync healthy = new() { Run = AddRun(build, "healthy") }; + + // Act + Task batch = Task.WhenAll(broken.UpdateAsync(), healthy.UpdateAsync()); + await batch.ConfigureAwait(false); + + // Assert + Assert.IsFalse(batch.IsFaulted, "One failing run should not fault the batch, which skips the pruning after it"); + Assert.HasCount(1, provider.Updated); + Assert.AreEqual("healthy", provider.Updated[0], "The healthy run should still be polled"); + Assert.IsFalse(broken.ShouldUpdate, "The failed run should wait out its interval before the next poll"); + } + + private static Run AddRun(Build build, string name) + { + Run run = build.CreateRun(name.As()); + run.Status = RunStatus.Running; + Assert.IsTrue(build.Runs.TryAdd(run.Id, run)); + return run; + } + /// /// Gets or sets the test context MSTest injects, used for its cancellation token. /// diff --git a/BuildMonitor/BuildMonitor.cs b/BuildMonitor/BuildMonitor.cs index ba60997..ab9eddc 100644 --- a/BuildMonitor/BuildMonitor.cs +++ b/BuildMonitor/BuildMonitor.cs @@ -1504,78 +1504,67 @@ private static async Task UpdateAsync() { if (!ProviderRefreshTimer.IsRunning || ProviderRefreshTimer.Elapsed.TotalSeconds >= ProviderRefreshTimeout) { - try - { - await DiscoverAsync().ConfigureAwait(false); - } - finally + // Gather all owners across all providers, skipping providers with low budget + // Discovery operations are low priority and should be skipped when budget is constrained + List<(BuildProvider Provider, Owner Owner)> allOwners = []; + foreach ((BuildProviderName _, BuildProvider? provider) in AppData.BuildProviders) { - // Restarted even when discovery fails, so a failure waits out the refresh interval - // instead of re-running discovery straight away (ktsu-dev/BuildMonitor#299). - ProviderRefreshTimer.Restart(); - } - } + // Skip discovery for providers in low budget mode + if (provider.IsLowBudget) + { + Log.Info($"{provider.Name}: Skipping discovery due to low budget ({provider.BudgetPercentage:P0} remaining)"); + continue; + } - // Update builds and runs concurrently - await Task.WhenAll( - UpdateBuildsAsync(), - UpdateRunsAsync() - ).ConfigureAwait(false); + foreach ((OwnerName _, Owner? owner) in provider.Owners) + { + allOwners.Add((provider, owner)); + } + } - PruneOrphanedAndCompletedSyncs(); - } + // Update repositories concurrently (semaphore limits per-provider concurrency) + // Each owner is guarded so one that fails does not fault the batch, which would skip the + // ProviderRefreshTimer restart below and re-run discovery straight away (ktsu-dev/BuildMonitor#299). + await Task.WhenAll(allOwners.Select(x => SyncGuard.RunAsync( + () => x.Provider.UpdateRepositoriesAsync(x.Owner), + $"{x.Provider.Name}: repository discovery for {x.Owner.Name}"))).ConfigureAwait(false); - private static async Task DiscoverAsync() - { - // Gather all owners across all providers, skipping providers with low budget - // Discovery operations are low priority and should be skipped when budget is constrained - List<(BuildProvider Provider, Owner Owner)> allOwners = []; - foreach ((BuildProviderName _, BuildProvider? provider) in AppData.BuildProviders) - { - // Skip discovery for providers in low budget mode - if (provider.IsLowBudget) + // Gather all repositories across all owners + List<(BuildProvider Provider, Repository Repository)> allRepositories = []; + foreach ((BuildProvider provider, Owner owner) in allOwners) { - Log.Info($"{provider.Name}: Skipping discovery due to low budget ({provider.BudgetPercentage:P0} remaining)"); - continue; + foreach ((RepositoryId _, Repository? repository) in owner.Repositories) + { + allRepositories.Add((provider, repository)); + } } - foreach ((OwnerName _, Owner? owner) in provider.Owners) + // Update builds concurrently (semaphore limits per-provider concurrency) + // Each repository is guarded for the same reason as each owner above. + await Task.WhenAll(allRepositories.Select(x => SyncGuard.RunAsync(async () => { - allOwners.Add((provider, owner)); - } - } + await x.Provider.UpdateBuildsAsync(x.Repository).ConfigureAwait(false); - // Update repositories concurrently (semaphore limits per-provider concurrency) - // Each owner is guarded so one that fails does not stop discovery for the rest. - await Task.WhenAll(allOwners.Select(x => SyncGuard.RunAsync( - () => x.Provider.UpdateRepositoriesAsync(x.Owner), - $"{x.Provider.Name}: repository discovery for {x.Owner.Name}"))).ConfigureAwait(false); + // Add new builds to sync collection + foreach ((BuildId _, Build? build) in x.Repository.Builds) + { + _ = BuildSyncCollection.TryAdd(build.Id, new() + { + Build = build, + }); + } + }, $"{x.Provider.Name}: build discovery for {x.Repository.Owner.Name}/{x.Repository.Name}"))).ConfigureAwait(false); - // Gather all repositories across all owners - List<(BuildProvider Provider, Repository Repository)> allRepositories = []; - foreach ((BuildProvider provider, Owner owner) in allOwners) - { - foreach ((RepositoryId _, Repository? repository) in owner.Repositories) - { - allRepositories.Add((provider, repository)); - } + ProviderRefreshTimer.Restart(); } - // Update builds concurrently (semaphore limits per-provider concurrency) - // Each repository is guarded so one that fails does not stop discovery for the rest. - await Task.WhenAll(allRepositories.Select(x => SyncGuard.RunAsync(async () => - { - await x.Provider.UpdateBuildsAsync(x.Repository).ConfigureAwait(false); + // Update builds and runs concurrently + await Task.WhenAll( + UpdateBuildsAsync(), + UpdateRunsAsync() + ).ConfigureAwait(false); - // Add new builds to sync collection - foreach ((BuildId _, Build? build) in x.Repository.Builds) - { - _ = BuildSyncCollection.TryAdd(build.Id, new() - { - Build = build, - }); - } - }, $"{x.Provider.Name}: build discovery for {x.Repository.Owner.Name}/{x.Repository.Name}"))).ConfigureAwait(false); + PruneOrphanedAndCompletedSyncs(); } private static void PruneOrphanedAndCompletedSyncs() From e10133b2497fda6e8897aae925343a79fd679387 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 30 Sep 2026 05:44:42 +0000 Subject: [PATCH 3/3] refactor: guard discovery batches through SyncGuard.RunAllAsync [patch] Moves the per-item guarding out of the static update loop into a helper that can be tested directly, and covers it with a test that a failing discovery item is logged while the rest still run. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01F4JLDZ8V6BpUZBB9ZnsaNt --- BuildMonitor.Test/SyncFailureTests.cs | 34 +++++++++++++++++++++++++++ BuildMonitor/BuildMonitor.cs | 8 +++---- BuildMonitor/SyncGuard.cs | 18 ++++++++++++++ 3 files changed, 55 insertions(+), 5 deletions(-) diff --git a/BuildMonitor.Test/SyncFailureTests.cs b/BuildMonitor.Test/SyncFailureTests.cs index 1840d3a..9e7e044 100644 --- a/BuildMonitor.Test/SyncFailureTests.cs +++ b/BuildMonitor.Test/SyncFailureTests.cs @@ -125,6 +125,40 @@ public async Task AFailedRunUpdateDoesNotStopTheOthersInTheBatch() Assert.IsFalse(broken.ShouldUpdate, "The failed run should wait out its interval before the next poll"); } + [TestMethod] + public async Task AFailedDiscoveryItemDoesNotStopTheRest() + { + // Arrange + List discovered = []; + + // Act + Task batch = SyncGuard.RunAllAsync( + ["first", "broken", "last"], + name => + { + if (name == "broken") + { + throw new HttpRequestException("503 Service Unavailable"); + } + + lock (discovered) + { + discovered.Add(name); + } + + return Task.CompletedTask; + }, + name => $"discovery for {name}"); + await batch.ConfigureAwait(false); + + // Assert + Assert.IsFalse(batch.IsFaulted, "One failing item should not fault discovery, which would skip the refresh timer restart after it"); + Assert.HasCount(2, discovered); + Assert.Contains("first", discovered); + Assert.Contains("last", discovered); + Assert.Contains(e => e.Message.Contains("discovery for broken", StringComparison.Ordinal), Log.GetEntries(), "The failure should be logged"); + } + private static Run AddRun(Build build, string name) { Run run = build.CreateRun(name.As()); diff --git a/BuildMonitor/BuildMonitor.cs b/BuildMonitor/BuildMonitor.cs index ab9eddc..1431aa2 100644 --- a/BuildMonitor/BuildMonitor.cs +++ b/BuildMonitor/BuildMonitor.cs @@ -1525,9 +1525,7 @@ private static async Task UpdateAsync() // Update repositories concurrently (semaphore limits per-provider concurrency) // Each owner is guarded so one that fails does not fault the batch, which would skip the // ProviderRefreshTimer restart below and re-run discovery straight away (ktsu-dev/BuildMonitor#299). - await Task.WhenAll(allOwners.Select(x => SyncGuard.RunAsync( - () => x.Provider.UpdateRepositoriesAsync(x.Owner), - $"{x.Provider.Name}: repository discovery for {x.Owner.Name}"))).ConfigureAwait(false); + await SyncGuard.RunAllAsync(allOwners, x => x.Provider.UpdateRepositoriesAsync(x.Owner), x => $"{x.Provider.Name}: repository discovery for {x.Owner.Name}").ConfigureAwait(false); // Gather all repositories across all owners List<(BuildProvider Provider, Repository Repository)> allRepositories = []; @@ -1541,7 +1539,7 @@ await Task.WhenAll(allOwners.Select(x => SyncGuard.RunAsync( // Update builds concurrently (semaphore limits per-provider concurrency) // Each repository is guarded for the same reason as each owner above. - await Task.WhenAll(allRepositories.Select(x => SyncGuard.RunAsync(async () => + await SyncGuard.RunAllAsync(allRepositories, async x => { await x.Provider.UpdateBuildsAsync(x.Repository).ConfigureAwait(false); @@ -1553,7 +1551,7 @@ await Task.WhenAll(allRepositories.Select(x => SyncGuard.RunAsync(async () => Build = build, }); } - }, $"{x.Provider.Name}: build discovery for {x.Repository.Owner.Name}/{x.Repository.Name}"))).ConfigureAwait(false); + }, x => $"{x.Provider.Name}: build discovery for {x.Repository.Owner.Name}/{x.Repository.Name}").ConfigureAwait(false); ProviderRefreshTimer.Restart(); } diff --git a/BuildMonitor/SyncGuard.cs b/BuildMonitor/SyncGuard.cs index a5c484b..1a6d697 100644 --- a/BuildMonitor/SyncGuard.cs +++ b/BuildMonitor/SyncGuard.cs @@ -37,4 +37,22 @@ internal static async Task RunAsync(Func update, string description) return false; } } + + /// + /// Runs for every item concurrently, each guarded by + /// , so one item that throws is logged and the rest still run. + /// + /// The type of item being updated. + /// The items to update. + /// The update to run for each item. + /// Describes an item for the log line when its update fails. + /// A task that completes, without faulting, once every update has finished. + internal static Task RunAllAsync(IEnumerable items, Func update, Func describe) + { + Ensure.NotNull(items); + Ensure.NotNull(update); + Ensure.NotNull(describe); + + return Task.WhenAll(items.Select(item => RunAsync(() => update(item), describe(item)))); + } }