Fix /internal/refresh 500: catch ArgumentException, not just its subclass - #126
Merged
Conversation
The post-deploy smoke test went red on main: /internal/refresh answered 500. My previous fix was incomplete in two places, and this is the case it missed. Draw.Create throws plain ArgumentException for a row that is not five distinct balls. ArgumentOutOfRangeException DERIVES from ArgumentException, so catching only the derived type let the base type straight through - and the base type is what the Socrata feed produces in the minutes after a drawing, when the row is published with placeholder numbers. The smoke test ran at 23:03 ET on a Monday, four minutes after the 22:59 Powerball draw. Second escape, in RefreshGame itself: EraValidator.Validate calls RuleEras.ForDate, which THROWS ArgumentOutOfRangeException for a date no era covers rather than returning a violation. That call sits inside the feed loop but its exception matched none of the filter's types, so a single bogus date escaped ExecuteAsync. /internal/refresh has no try/catch, so it 500s and the game after the failing one never refreshes. Both now report a feed error, which is what this class documents. Three regression tests, each verified to fail against the previous code: two placeholder-row shapes through the Socrata feed, and an out-of-era draw date through RefreshGame. 319 tests pass. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AWkVh7cyAz1gWapBH1CY8n
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The post-deploy smoke test went red on
mainafter #117 merged —/internal/refreshanswered 500. The deploy itself succeeded and 32 of 33 smoke checks passed; only the keyed refresh failed.My fix in #117 was incomplete. Two escapes remained.
1.
ArgumentOutOfRangeExceptionis a subclass ofArgumentExceptionDraw.Createthrows plainArgumentExceptionfor a row that is not five distinct balls:#117's guard caught
ArgumentOutOfRangeException, which derives fromArgumentException— so it caught the narrow type and let the base type through. And the base type is exactly what the Socrata feed produces in the minutes after a drawing, when the row is published with placeholder numbers.The timing fits: the smoke test ran at 23:03 ET on a Monday, four minutes after the 22:59 Powerball drawing. The deploy seven minutes earlier, on the same workflow, passed.
2.
RuleEras.ForDatethrows rather than returning a violationEraValidator.ValidatecallsRuleEras.ForDate, which throwsArgumentOutOfRangeExceptionfor a date no era covers instead of returning a violation. That call sits insideRefreshGame's feed loop, but its exception matched none of the filter's types, so a single bogus date escapedExecuteAsyncentirely./internal/refreshhas no try/catch of its own, so the request 500s and the game after the failing one never refreshes.Both paths now report a feed error, which is what
RefreshGame's own summary documents: "Feed failures are reported, never thrown — a broken external source must not take the app down."Tests
Three regression tests, each verified to fail against the previous code before being accepted:
APlaceholderRow_IsReportedAsAFeedError_NotThrownAtTheCaller— two placeholder row shapes ("00 00 00 00 00 00"and a repeated ball), through the real Socrata feed with a canned response.ADrawDatedBeforeAnyKnownEra_IsReportedAsAFeedError_NotThrown— an out-of-era draw date throughRefreshGame, assertingFeedErroris set rather than an exception escaping.319 tests pass (316 + 3),
dotnet build -warnaserrorwith zero warnings, locked-mode restore clean.Note on scope
The batch is still refused rather than silently delivered short — a partial batch would let gap-repair skip real draws. The change is only in how the failure surfaces: as a reported
feedErroron a 200, not an unhandled exception on a 500.🤖 Generated with Claude Code