Stop reporting Aviation Weather outages as app defects - #14
Merged
Conversation
SF50-TOLD-2F reported a gzip failure with dataSize 0 on the TAF cache: aviationweather.gov serves an empty cache file while regenerating it, so the app tried to gunzip nothing. SF50-TOLD-2G is the same shape with a 502 from the CDN on the winds-aloft endpoint. Neither is a defect here, yet both reached Sentry as warnings. An upstream 500 is not automatically retryable. That the AWC answers one at all is a flaw on their end, and working around a third party's flawed behavior with a backoff loop is not this app's job. So the app reports nothing to Sentry for third-party failures and leaves the retry decision to the pilot. - Reject an empty body at the fetch site as Errors.emptyResponse rather than letting it reach the decompressor, so "the server sent nothing" stays distinguishable from "the body arrived corrupt". A corrupt non-empty body is still reported: that would be a real contract break. - Add isUpstreamServerFailure (5xx and empty bodies) and fold it together with the existing cancellation and URLError sets into shouldReport, the single gate all three feeds and WeatherViewModel's conditions stream now consult. The conditions stream re-reports whatever the loader surfaces, so widening only the loader would have left the duplicate report that PR #9 fixed for URLErrors. - Collapse the three duplicated catch-block classifications into recordLoadFailure, preserving the existing Sentry tags and fingerprints so grouping is unaffected. The pilot's recourse needed no new code. The weather row stays a navigation link in the error state, and WeatherSource already titles its button "Try Again" whenever an error is set, calling load(force: true) to bypass the 15 minute reload interval. That makes the existing recoverySuggestion — re-download later or enter weather manually — true. shouldReport is left untested: it is internal and only observable through Sentry, so pinning it would mean reaching through @testable, the white-box shape rejected in review in ef1fe35. Verified: swift format --strict and swiftlint --strict clean on changed files; SF50 TOLD builds for iOS 26 iPhone 17 Pro; SF50 Shared Unit Tests 304/304 pass; the Try Again control confirmed on an iOS 26 simulator in the weather error state. Fixes SF50-TOLD-2F Fixes SF50-TOLD-2G Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
RISCfuture
force-pushed
the
fix/weather-transient-upstream-failures
branch
from
July 31, 2026 21:36
1e50d36 to
7649925
Compare
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.
Two Sentry issues were Aviation Weather failing on its own end, reported as though this app had a defect:
dataSize: 0onhttps://aviationweather.gov/data/cache/tafs.cache.xml.gz. The server serves an empty cache file while regenerating it, so the app tried to gunzip nothing.Neither is a defect here, yet both reached Sentry as warnings.
Design
An upstream 500 is not automatically retryable. That the AWC answers one at all is a flaw on their end, and working around a third party's flawed behavior with a backoff loop is not this app's job. So the app reports nothing to Sentry for third-party failures and leaves the retry decision to the pilot.
Changes
Errors.emptyResponserather than letting it reach the decompressor, so "the server sent nothing" stays distinguishable from "the body arrived corrupt". A corrupt non-empty body is still reported — that would be a real contract break.isUpstreamServerFailure(5xx and empty bodies) and fold it together with the existing cancellation andURLErrorsets intoshouldReport, the single gate all three feeds andWeatherViewModel's conditions stream now consult. The conditions stream re-reports whatever the loader surfaces, so widening only the loader would have left the duplicate report that Stop reporting transient weather-fetch network errors to Sentry #9 fixed forURLErrors.recordLoadFailure, preserving the existing Sentry tags and fingerprints so grouping is unaffected.The pilot's recourse needed no new code
The manual retry affordance already exists and is reachable in the error state, so no UI was added:
TakeoffAirportView/LandingAirportViewkeep the weather row as aNavigationLinkintoWeatherPickereven when weather failed (the row reads "Couldn't load weather — using ISA").WeatherPickerrendersWeatherSourcewhenever it is not actively loading, which is the case in the error state.WeatherSourcetitles its button "Try Again" whenevererror != nil, callingload(force: true), which bypasses the 15 minute reload interval and re-runs all three feeds.That makes the existing
recoverySuggestion— "Try re-downloading weather data later, or manually enter weather." — actually true.Verified live on an iOS 26 simulator via the
WEATHER-ERRORlaunch argument: the Weather picker'sSourcesection reads "Couldn't load weather — using ISA" with a Try Again button beside it, above the Customize Weather form.On testing
shouldReportis left untested. It is internal and only observable through Sentry, so pinning it would mean reaching through@testable— the white-box shape rejected in review in ef1fe35 ("unit tests should be black box"). The classification itself is aswitchover status-code ranges, which the repo's rule against trivial tests also covers. No UI test was added either: the retry control is pre-existing and a single button is not a major user flow.Verification
swift format lint --strictandswiftlint --strictclean on all changed files (only the pre-existingswitch_case_alignmentconfig warning).SF50 TOLDscheme builds for iOS 26 (iPhone 17 Pro), isolatedderivedDataPath.SF50 Shared Unit Teststest plan (the one CI runs): 304/304 pass, 0 failures, read viaxcresultparser.Fixes SF50-TOLD-2F
Fixes SF50-TOLD-2G
🤖 Generated with Claude Code