refactor: guard reddit client DI, self-heal wedged feed job, centralize user-agent setting

Author: chutch3

api/API/Discovery/RedditFeedClient.cs

@@ -1,3 +1,4 @@
+using API.Services;
 using System.Xml.Linq;
 
 namespace API.Discovery;
@@ -8,10 +9,8 @@ public class RedditFeedClient(HttpClient http) : IRedditFeedClient
     /// <summary>Reddit's recommended User-Agent shape (platform:appid (comment)).</summary>
     public const string UserAgent = "kenku:discovery (self-hosted manga manager)";
 
-    /// <summary>Sets the reddit User-Agent as a raw header — the typed <c>UserAgent.ParseAdd</c> rejects
-    /// this otherwise-valid value and throws at client construction.</summary>
-    public static void ConfigureClient(HttpClient client) =>
-        client.DefaultRequestHeaders.TryAddWithoutValidation("User-Agent", UserAgent);
+    /// <summary>Sets the reddit User-Agent (a raw header — see <see cref="API.Services.HttpClientExtensions.SetUserAgent"/>).</summary>
+    public static void ConfigureClient(HttpClient client) => client.SetUserAgent(UserAgent);
 
 
     public async Task<List<DiscoveryEntry>> GetHotAsync(string subreddit, int limit, CancellationToken ct)

api/API/HttpRequesters/HttpRequester.cs

@@ -1,4 +1,5 @@
 using API.HttpRequesters.Interfaces;
+using API.Services;
 using System.Net;
 using log4net;
 
@@ -19,8 +20,8 @@ public HttpRequester(RateLimitHandler rateLimitHandler, KenkuSettings settings)
         {
             Timeout = Timeout.InfiniteTimeSpan,
             DefaultVersionPolicy = HttpVersionPolicy.RequestVersionOrHigher,
-            DefaultRequestHeaders = { { "User-Agent", settings.UserAgent } }
         };
+        _client.SetUserAgent(settings.UserAgent);
         _flareSolverrClient = new FlareSolverrRequester(_client, settings);
     }
 

api/API/JobRuntime/Reconcilers/DiscoveryFeedReconciler.cs

@@ -20,6 +20,22 @@ public class DiscoveryFeedReconciler(IServiceScopeFactory scopeFactory, IClock c
     protected override Task TickAsync(IServiceProvider scope, CancellationToken ct) =>
         EnqueueAsync(scope.GetRequiredService<IJobStore>(), clock.UtcNow, ct);
 
-    public static Task EnqueueAsync(IJobStore store, DateTime now, CancellationToken ct) =>
-        store.EnqueueAsync(new Job(RefreshDiscoveryFeedHandler.Type, "{}", now, dedupKey: DedupKey), ct);
+    public static async Task EnqueueAsync(IJobStore store, DateTime now, CancellationToken ct)
+    {
+        // A periodic refresh must not stay wedged: enqueue dedups onto an active job, and a parked
+        // (NeedsAttention) run counts as active — so a single past hard failure would disable the feed
+        // forever. Re-arm that parked job instead so the next tick runs it.
+        if ((await store.GetAllAsync(ct)).FirstOrDefault(j =>
+                j.DedupKey == DedupKey && j.Status == JobStatus.NeedsAttention) is { } parked)
+        {
+            parked.Status = JobStatus.Queued;
+            parked.Attempts = 0;
+            parked.ScheduledFor = now;
+            parked.Error = null;
+            await store.UpdateAsync(parked, ct);
+            return;
+        }
+
+        await store.EnqueueAsync(new Job(RefreshDiscoveryFeedHandler.Type, "{}", now, dedupKey: DedupKey), ct);
+    }
 }

api/API/Schema/NotificationsContext/NotificationConnectors/NotificationConnector.cs

@@ -1,6 +1,7 @@
 using System.ComponentModel.DataAnnotations;
 using System.ComponentModel.DataAnnotations.Schema;
 using System.Text;
+using API.Services;
 using log4net;
 using Microsoft.EntityFrameworkCore;
 using Newtonsoft.Json;
@@ -28,10 +29,7 @@ public class NotificationConnector(string name, string url, Dictionary<string, s
     internal NotificationConnector(string name, string url, string httpMethod, string body)
         : this(name, url, new Dictionary<string, string>(), httpMethod, body) { }
 
-    [NotMapped] private readonly HttpClient Client = new()
-    {
-        DefaultRequestHeaders = { { "User-Agent", KenkuSettings.DefaultUserAgent } }
-    };
+    [NotMapped] private readonly HttpClient Client = new HttpClient().SetUserAgent(KenkuSettings.DefaultUserAgent);
 
     [JsonIgnore] protected ILog Log = LogManager.GetLogger(name);
 

api/API/Services/HttpClientExtensions.cs

@@ -8,11 +8,19 @@ public static class HttpClientExtensions
     /// </summary>
     public const string KenkuUserAgent = "Kenku/1.0 (+https://github.com/chutch3/kenku)";
 
-    /// <summary>Sets the Kenku User-Agent if one isn't already present, then returns the client.</summary>
-    public static HttpClient WithKenkuUserAgent(this HttpClient client)
+    /// <summary>
+    /// Sets the User-Agent as a raw header, replacing any existing one. Raw because the typed
+    /// <c>DefaultRequestHeaders.UserAgent</c>/<c>Add</c> validate against product-token syntax and
+    /// throw on otherwise-fine values (a user's custom agent, reddit's "platform:appid (comment)").
+    /// </summary>
+    public static HttpClient SetUserAgent(this HttpClient client, string userAgent)
     {
-        if (client.DefaultRequestHeaders.UserAgent.Count == 0)
-            client.DefaultRequestHeaders.UserAgent.ParseAdd(KenkuUserAgent);
+        client.DefaultRequestHeaders.Remove("User-Agent");
+        client.DefaultRequestHeaders.TryAddWithoutValidation("User-Agent", userAgent);
         return client;
     }
+
+    /// <summary>Sets the Kenku User-Agent if one isn't already present, then returns the client.</summary>
+    public static HttpClient WithKenkuUserAgent(this HttpClient client) =>
+        client.DefaultRequestHeaders.UserAgent.Count == 0 ? client.SetUserAgent(KenkuUserAgent) : client;
 }

api/Tests/Unit/Extensions/DiscoveryRegistrationTests.cs

@@ -0,0 +1,27 @@
+using API.Discovery;
+using API.Extensions;
+using Microsoft.Extensions.DependencyInjection;
+using Xunit;
+
+namespace API.Tests.Unit.Extensions;
+
+/// <summary>
+/// The discovery HTTP clients must construct through the real DI graph. The reddit client's
+/// User-Agent was once set with the strict typed parser, which threw at construction — invisible to
+/// tests that stub <see cref="IRedditFeedClient"/> (the feed end-to-end test does), so it only
+/// surfaced when the live job resolved the client. This resolves the real registration instead.
+/// </summary>
+public class DiscoveryRegistrationTests
+{
+    [Fact]
+    public void RedditFeedClient_ResolvesFromTheRealContainer_WithoutThrowing()
+    {
+        using ServiceProvider provider = new ServiceCollection().AddKenkuServices().BuildServiceProvider();
+
+        // Resolution builds the typed HttpClient and runs the registration's configure callback —
+        // the exact step that threw on the bad User-Agent.
+        var client = provider.GetRequiredService<IRedditFeedClient>();
+
+        Assert.NotNull(client);
+    }
+}

api/Tests/Unit/HttpRequesters/HttpRequesterTests.cs

@@ -14,6 +14,20 @@ private static HttpRequester CreateClient(Func<HttpRequestMessage, HttpResponseM
         return new HttpRequester(rateLimit, settings);
     }
 
+    [Fact]
+    public void Constructs_WithACustomUserAgent_TheTypedParserWouldReject()
+    {
+        // Users can set any User-Agent; a reddit-style value ("kenku:discovery (…)") trips the typed
+        // header parser, which would throw at requester construction and break every outbound call.
+        var settings = new KenkuSettings { AppData = Path.GetTempPath() };
+        settings.SetUserAgent("kenku:discovery (self-hosted manga manager)");
+        var rateLimit = new RateLimitHandler(settings, new FakeHttpMessageHandler(_ => new HttpResponseMessage(HttpStatusCode.OK)));
+
+        var ex = Record.Exception(() => new HttpRequester(rateLimit, settings));
+
+        Assert.Null(ex);
+    }
+
     [Fact]
     public async Task MakeRequest_Success_ReturnsResponse()
     {

api/Tests/Unit/JobRuntime/DiscoveryFeedReconcilerTests.cs

@@ -1,6 +1,7 @@
 using API.JobRuntime;
 using API.JobRuntime.Handlers;
 using API.JobRuntime.Reconcilers;
+using API.Schema.JobsContext;
 using Xunit;
 
 namespace API.Tests.Unit.JobRuntime;
@@ -20,4 +21,24 @@ public async Task Tick_EnqueuesOneDedupedRefreshJob()
         Assert.Equal(RefreshDiscoveryFeedHandler.Type, job.Type);
         Assert.Equal(DiscoveryFeedReconciler.DedupKey, job.DedupKey);
     }
+
+    [Fact]
+    public async Task Tick_ReArmsAParkedRefreshJob_SoAHardFailureCannotWedgeTheFeedForever()
+    {
+        var store = new InMemoryJobStore();
+        // A prior run failed its way to NeedsAttention; dedup would otherwise coalesce onto it forever.
+        var parked = await store.EnqueueAsync(new Job(RefreshDiscoveryFeedHandler.Type, "{}", DateTime.UtcNow,
+            dedupKey: DiscoveryFeedReconciler.DedupKey));
+        parked.Status = JobStatus.NeedsAttention;
+        parked.Attempts = 5;
+        parked.Error = "boom";
+        await store.UpdateAsync(parked);
+
+        await DiscoveryFeedReconciler.EnqueueAsync(store, DateTime.UtcNow, default);
+
+        var job = Assert.Single(await store.GetAllAsync());
+        Assert.Equal(JobStatus.Queued, job.Status);
+        Assert.Equal(0, job.Attempts);
+        Assert.Null(job.Error);
+    }
 }