Bypass updater local and CDN HTTP cache on check for updates#891
Bypass updater local and CDN HTTP cache on check for updates#891MineraleYT wants to merge 8 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Sparkle updater delegate now supplies a timestamp-based ChangesSparkle feed refresh
Estimated code review effort: 1 (Trivial) | ~2 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
SeoFood
left a comment
There was a problem hiding this comment.
Requesting changes because the added delegate method is not invoked by the pinned Sparkle version.
| AppConstants.effectiveUpdateChannel.sparkleChannels | ||
| } | ||
|
|
||
| nonisolated func updater(_ updater: SPUUpdater, feedURLRequest originalRequest: URLRequest) -> URLRequest { |
There was a problem hiding this comment.
This method is never called. Sparkle 2.8.1 does not declare updater(_:feedURLRequest:) in SPUUpdaterDelegate, and the framework contains no corresponding call site. Swift allows this extra method, so the build and CI still pass even though the update requests remain unchanged.
Sparkle already applies .reloadIgnoringLocalCacheData internally to appcast requests. If custom headers are required, please use a supported API such as SPUUpdater.httpHeaders or address the caching behavior at the feed host, then verify the actual outgoing request. As written, this PR does not change manual or automatic update checks.
| AppConstants.effectiveUpdateChannel.sparkleChannels | ||
| } | ||
|
|
||
| nonisolated func feedParameters(for updater: SPUUpdater, sendingSystemProfile: Bool) -> [[String : String]] { |
There was a problem hiding this comment.
This is now a real Sparkle delegate callback, but it still does not bypass the CDN in front of the actual feed. I sent three requests to https://typewhisper.github.io/typewhisper-mac/appcast.xml with different UUID values for nocache; GitHub Pages returned the same cached object for all three (x-cache: HIT with an identical x-github-request-id). The query string is therefore not part of the effective cache key for this feed, so this changes every updater URL without making the appcast fresh.
Please remove this cache-buster and solve freshness at the hosting/release layer, or move the feed to an endpoint whose cache behavior can be controlled. The fix needs an end-to-end check that can distinguish stale and newly published appcast content.
|
The new Please drop the |
|
Closing this PR because the current timestamp query parameter does not bypass the effective GitHub Pages CDN cache, so it does not solve the appcast freshness problem end to end. The Sparkle 2.9.4 update and required MainActor migration have landed separately in #894. The remaining hosting and release-layer cache work is now tracked in #895. Thank you for iterating on the original implementation. |
Summary
This PR bypasses local client-side and CDN edge caching when checking for application updates via the Sparkle framework.
Problem
When newly compiled releases are pushed to the server, checking for updates manually inside the app can sometimes fail to detect them for up to a few hours. This delay occurs because macOS URLSession cache or intermediate CDN servers (like GitHub Releases or Cloudflare) serve cached feed XML documents.
Solution
SPUUpdaterDelegate'supdater(_:feedURLRequest:)delegate method insideAppDelegate..reloadIgnoringLocalCacheData.Cache-Control: no-cacheandPragma: no-cache). This guarantees that manual checks always request fresh version metadata directly from the source server.Test Plan
scripts/pr-preflight.shSummary by CodeRabbit