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); + } +}