diff --git a/BuildMonitor.Test/SyncFailureTests.cs b/BuildMonitor.Test/SyncFailureTests.cs
new file mode 100644
index 0000000..9e7e044
--- /dev/null
+++ b/BuildMonitor.Test/SyncFailureTests.cs
@@ -0,0 +1,174 @@
+// 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");
+ }
+
+ [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");
+ }
+
+ [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());
+ 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.
+ ///
+ public TestContext TestContext { get; set; } = null!;
+}
diff --git a/BuildMonitor/BuildMonitor.cs b/BuildMonitor/BuildMonitor.cs
index 04eb97b..1431aa2 100644
--- a/BuildMonitor/BuildMonitor.cs
+++ b/BuildMonitor/BuildMonitor.cs
@@ -1523,7 +1523,9 @@ private static async Task UpdateAsync()
}
// Update repositories concurrently (semaphore limits per-provider concurrency)
- await Task.WhenAll(allOwners.Select(x => x.Provider.UpdateRepositoriesAsync(x.Owner))).ConfigureAwait(false);
+ // 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 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 = [];
@@ -1536,7 +1538,8 @@ private static async Task UpdateAsync()
}
// Update builds concurrently (semaphore limits per-provider concurrency)
- await Task.WhenAll(allRepositories.Select(async x =>
+ // Each repository is guarded for the same reason as each owner above.
+ await SyncGuard.RunAllAsync(allRepositories, async x =>
{
await x.Provider.UpdateBuildsAsync(x.Repository).ConfigureAwait(false);
@@ -1548,7 +1551,7 @@ await Task.WhenAll(allRepositories.Select(async x =>
Build = build,
});
}
- })).ConfigureAwait(false);
+ }, x => $"{x.Provider.Name}: build discovery for {x.Repository.Owner.Name}/{x.Repository.Name}").ConfigureAwait(false);
ProviderRefreshTimer.Restart();
}
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..1a6d697
--- /dev/null
+++ b/BuildMonitor/SyncGuard.cs
@@ -0,0 +1,58 @@
+// 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;
+ }
+ }
+
+ ///
+ /// 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))));
+ }
+}