From 1d83133d859f5ed59c26a7d0614d0d541f1c3581 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 03:39:41 +0000 Subject: [PATCH] fix(api): guard /internal/refresh per game so a feed escape cannot 500 it Twice now a feed has thrown a type RefreshGame's catch filter did not match - XmlException from MegaMillions, then plain ArgumentException from Draw.Create - and both times this endpoint answered 500. It is what the keep-alive workflow calls, so a 500 reads as the whole instance being down. Each of those was fixed at its source, but the endpoint itself had no guard, so the next escape would do the same thing. This closes the class rather than the instances. Per game, not around the loop: the unguarded loop aborted on the first throw, so every game after the failing one was silently skipped. The catch is not a swallow - it logs the exception with its game, and records a RefreshResult carrying the message as feedError, which is the same shape the response already uses for the failures RefreshGame does anticipate. Callers see a 200 with the error named per game. OperationCanceledException is deliberately excluded: a shutdown or a disconnected client is not a feed failure and must keep propagating. Two tests, both verified to fail without the guard: an unanticipated exception type is reported rather than 500ing, and one game failing does not cost the other its refresh. 321 tests pass. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01AWkVh7cyAz1gWapBH1CY8n --- src/Lottery.Api/LotteryEndpoints.cs | 30 ++++++++- .../Lottery.Api.Tests/RefreshEndpointTests.cs | 65 +++++++++++++++++++ 2 files changed, 93 insertions(+), 2 deletions(-) diff --git a/src/Lottery.Api/LotteryEndpoints.cs b/src/Lottery.Api/LotteryEndpoints.cs index 8866d8c..abd1399 100644 --- a/src/Lottery.Api/LotteryEndpoints.cs +++ b/src/Lottery.Api/LotteryEndpoints.cs @@ -145,7 +145,7 @@ await WithGameAsync(game, async g => // Optionally guarded by a shared key: set Refresh:Key in the environment // (never in a committed file) and callers send X-Refresh-Key. app.MapPost("/internal/refresh", async (HttpRequest request, RefreshGame refresh, - IConfiguration configuration, CancellationToken ct) => + IConfiguration configuration, ILoggerFactory loggerFactory, CancellationToken ct) => { var requiredKey = configuration["Refresh:Key"]; if (!string.IsNullOrEmpty(requiredKey) @@ -154,9 +154,35 @@ await WithGameAsync(game, async g => return Results.Unauthorized(); } + var logger = loggerFactory.CreateLogger("Lottery.Api.Refresh"); var results = new List(); + foreach (var game in Enum.GetValues()) - results.Add(await refresh.ExecuteAsync(game, ct)); + { + try + { + results.Add(await refresh.ExecuteAsync(game, ct)); + } + // Per game rather than around the loop: one game's source + // failing must not cost the other game its refresh, which is + // what an unguarded loop did. + // + // RefreshGame reports the failure modes it anticipates. This + // catches the ones it does not, so a new escape degrades to a + // reported error instead of a 500 - and this is the endpoint + // the keep-alive workflow calls, so a 500 here reads as the + // whole instance being down. + // + // OperationCanceledException is deliberately excluded: a + // shutdown or a disconnected client is not a feed failure and + // must keep propagating. + catch (Exception ex) when (ex is not OperationCanceledException) + { + logger.LogError(ex, "{Game}: refresh failed, reporting it as a feed error.", game); + results.Add(new RefreshResult(game, UpToDate: false, NewDraws: 0, + SkippedInvalid: 0, JackpotUpdated: false, FeedError: ex.Message)); + } + } return Results.Ok(results.Select(r => new { diff --git a/tests/Lottery.Api.Tests/RefreshEndpointTests.cs b/tests/Lottery.Api.Tests/RefreshEndpointTests.cs index ff3523f..8936bd4 100644 --- a/tests/Lottery.Api.Tests/RefreshEndpointTests.cs +++ b/tests/Lottery.Api.Tests/RefreshEndpointTests.cs @@ -1,7 +1,11 @@ using System.Net; using System.Net.Http.Json; using System.Text.Json; +using Lottery.Application.Abstractions; +using Lottery.Domain; using Microsoft.AspNetCore.Hosting; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; namespace Lottery.Api.Tests; @@ -125,3 +129,64 @@ public async Task ARejectedCall_StillCarriesTheHardeningHeaders() Assert.Equal("nosniff", Assert.Single(response.Headers.GetValues("X-Content-Type-Options"))); } } + +/// A numbers feed that throws a type RefreshGame's catch filter does not +/// match, for the game named. Stands in for a future escape of the kind that took +/// /internal/refresh to 500 twice. +public sealed class ExplodingNumbersFeed : IWinningNumbersFeed +{ + public Task> GetDrawsAfterAsync(Game game, DateOnly after, CancellationToken ct) => + game == Game.Powerball + ? throw new NotSupportedException("feed exploded") + : Task.FromResult>([]); +} + +public sealed class ExplodingFeedFactory : LotteryApiFactory +{ + protected override void ConfigureWebHost(IWebHostBuilder builder) + { + base.ConfigureWebHost(builder); + builder.ConfigureServices(services => + { + services.RemoveAll(); + services.AddSingleton(); + }); + } +} + +public sealed class RefreshEndpointResilienceTests : IClassFixture +{ + private readonly HttpClient _client; + + public RefreshEndpointResilienceTests(ExplodingFeedFactory factory) => _client = factory.CreateClient(); + + [Fact] + public async Task AnUnanticipatedFeedFailure_IsReported_NotA500() + { + // Twice now a feed has thrown a type RefreshGame's filter did not match, + // and this endpoint - which the keep-alive workflow calls - answered 500, + // reading as the whole instance being down. + var response = await _client.PostAsync("/internal/refresh", null); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var body = await response.Content.ReadFromJsonAsync(); + var powerball = body.EnumerateArray().Single(r => r.GetProperty("game").GetString() == "Powerball"); + Assert.Contains("exploded", powerball.GetProperty("feedError").GetString()); + } + + [Fact] + public async Task OneGameFailing_DoesNotCostTheOtherItsRefresh() + { + // The loop used to abort on the first throw, so every game after the + // failing one was silently skipped. + var body = await (await _client.PostAsync("/internal/refresh", null)) + .Content.ReadFromJsonAsync(); + + var games = body.EnumerateArray().Select(r => r.GetProperty("game").GetString()!).ToArray(); + Assert.Equal(["Powerball", "MegaMillions"], games); + + var megaMillions = body.EnumerateArray().Single(r => r.GetProperty("game").GetString() == "MegaMillions"); + Assert.Equal(JsonValueKind.Null, megaMillions.GetProperty("feedError").ValueKind); + } +}