Repository navigation
fix(photos): stop caching a failed lookup as a confirmed absence - #67
Merged
Merged
Conversation
photo_for() cached whatever _fetch() returned under cache_ttl_days, and _fetch()
returned None for two very different things: upstream answering "no photo for
this hex", and upstream never answering at all. The first is good for weeks.
The second says nothing about the airframe.
So a single transient error blanked that aircraft's photo for the full success
TTL — 30 days by default. Observed in the field: Cloudflare returned 525 (SSL
handshake failed to origin) for api.planespotters.net on three hexes within
twenty minutes. Those aircraft would have had no photo until late September,
for an outage that lasted minutes.
Failures now carry their own five-minute TTL. Short enough that a blip costs
one alert's photo rather than a month's, long enough that a sustained outage
costs one request per aircraft per five minutes instead of one per alert. The
cache entry carries its own ttl rather than the reader branching on result-is-
None, so a confirmed absence and a failure can never share an expiry by
accident. _fetch returns (result, answered) to make the distinction explicit at
the only place that can actually tell them apart.
Also logs error_class and retry_in_s on the failure path. httpx's timeout,
connect, and protocol errors all stringify to "" when raised without a message,
which produced log lines reading {"hex": "aa9e4a", "error": ""} — a failure
report carrying no indication of what failed. Same field shape
receiver_fetch_failed already uses.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FiVqZDdasMeshTd7CqdxEp
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.
What you saw
The 525s are upstream — Cloudflare failing its SSL handshake to Planespotters'
origin. Nothing to fix on our side. But the log surfaced two real bugs.
Bug 1: a blip blanks the photo for a month
photo_for()cached whatever_fetch()returned undercache_ttl_days, and_fetch()returnedNonefor two very different things:Same
None, same 30-day TTL. So those three hexes would have had no photountil late September, for an outage that lasted minutes.
Failures now carry their own 5-minute TTL. Short enough that a blip costs one
alert's photo rather than a month's; long enough that a sustained outage costs
one request per aircraft per five minutes instead of one per alert.
The cache entry carries its own
ttlrather than the reader branching onresult is None, so a confirmed absence and a failure can never share anexpiry by accident.
_fetchreturns(result, answered)to make thedistinction explicit at the only place that can actually tell them apart.
Bug 2:
"error": ""That second log line reports a failure and says nothing about it. httpx's
timeout, connect, and protocol errors all stringify to
""when raised withouta message:
Now logs
error_classandretry_in_s, matching the shapereceiver_fetch_failedalready uses.Tests
Five new, covering: failure retries after the short TTL; a failure does not get
the success TTL; a confirmed absence still keeps the long one (the regression
risk in fixing this); a 200 with no usable thumbnail counts as answered; and the
empty-
str()logging case.813 pass, ruff + mypy clean.
🤖 Generated with Claude Code