fix(version check): remember a failed lookup and keep it off the render path - #688
Merged
Conversation
…er path The startup version check asks the unauthenticated GitHub API for the release list, which allows 60 requests an hour per address. A developer restarting the app, or several instances behind one address, run through those and get a 403 -- and the way that failure was handled made it worse than the rate limit itself. A failed lookup was not remembered: `latestVersion` stayed null and only a successful lookup was cached, so the home page re-issued the request on every render, which is what kept the limit spent. The lookup also ran inline while the page was being built, making a slow or unreachable api.github.com a slow home page -- the kind of wait PR #600 set out to remove, missed there because this is not an API-cache query. And a cosmetic failure was logged as an error with sixty lines of Wicket, Jetty and Maven frames, which reads like a broken startup when nothing is wrong. A lookup is now claimed at most once an hour, whether it succeeds or fails, and a rate-limited answer is waited out for as long as its `x-ratelimit-reset` says. Startup still looks the version up in the foreground, since that is where the banner needs it and nothing is being served yet; `getLatestVersion` answers with what that left behind and hands a due lookup to the background. A failure is a warning with its message. Not knowing the latest version now reads as "unknown" in the banner, and leaves the sentence unsaid on the home page, rather than telling the user "the latest public version is null". Closes #686 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LqfrG3VXH5tzWXPvYJzXML
Contributor
|
🎉 This PR is included in version 5.14.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
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.
Closes #686.
What was wrong
WicketApplication.getLatestVersion()asks the unauthenticated GitHub API for the release list — 60 requests an hour per address. Once those are spent, the 403 was handled in three unhelpful ways:latestVersionstayed null and only success was cached, soHomePage.java:64re-issued the request on every render — which is what kept the limit spent. Caching success forever and failure not at all is the wrong way round.The change
claimVersionLookup), whether it succeeds or fails; a rate-limited answer is waited out for as long as itsx-ratelimit-resetsays.getLatestVersion()answers with whatever that left behind and hands a due lookup to the background.warnwith its message.unknownin the banner and leaves the sentence unsaid on the home page, instead of "The latest public version is null."Verification
Ran a second jetty on :37374 both ways.
Unreachable release API (constant pointed at a dead port for the run):
one line, no trace — and across three home-page renders: all 200, 1 lookup attempted in total (before this change each render would have made another). The home page said "You are running a temporary snapshot version of Nanodash (5.13.1-SNAPSHOT)." with no trailing "null".
Real API: banner
Latest public version: 5.13.0, home page "…(5.13.1-SNAPSHOT). The latest public version is 5.13.0." — unchanged behaviour when GitHub answers.Tests: 1297 pass, 6 new in
LatestVersionLookupTestcovering the once-an-hour claim, no lookup once the version is known, an unknown version answered with null rather than waited for, and thex-ratelimit-resethandling (honoured when later, ignored when past or unparseable).Branched from master, independent of #687.
🤖 Generated with Claude Code
https://claude.ai/code/session_01LqfrG3VXH5tzWXPvYJzXML