Make the ADS-B upstream configurable, with a fallback behind it

[Purple10101]

What

ADSB_UPSTREAMS replaces the hardcoded api.adsb.lol URL with an ordered, comma-separated list of adsb.lol-format base URLs, tried in turn until one answers. It defaults to api.adsb.lol alone, so a node told nothing behaves exactly as before.

Pairs with offworldlabs/retina-node#44, which sets it to https://adsb.retina.fm,https://api.adsb.lol.

Why

adsb.lol refuses roughly 88% of one node's requests with 429. Measured over an hour on jonathan-node-1 from the proxy's own logs: 1023 attempts, 119 succeeded, 904 rate-limited. Today the proxy answers those refusals by serving its cached copy for up to 60 s. A second source gives it something current to serve instead.

Details

• The fallback trigger is a failed fetch, nothing cleverer.
• The chain shares one time budget rather than a timeout per source. Two sources at ADSBLOL_TIMEOUT_MS each would be 6 s worst case, past blah2-api's 5 s client timeout, turning a slow upstream into a consumer-side failure instead of the stale-but-served degradation that timeout is chosen to give.
• X-Data-Source names the host that answered rather than always saying adsb.lol.
• One new optional env var: ADSB_UPSTREAMS.

Testing

Adds the repo's first tests: 5 cases, run three times to check for flakiness.

node --test test/proxy-chain.test.js     # 5 pass

Covers first-answering-source wins and the second is never polled, fall-through on a 429, a local receiver winning over every remote source with no upstream polled at all, the shared time budget, and the unconfigured default.

🤖 Generated with Claude Code

[claude]

Reviewed the diff (proxy/server.js, docker-compose.yml, test/proxy-chain.test.js). This is a well-reasoned change with genuinely useful test coverage (14 cases against real HTTP servers, not mocks) for what's the repo's first test suite. A few things worth a look:

1. .env.example is now out of date (not touched by this PR)

• No entries for the new ADSB_UPSTREAMS, ADSB_MAX_POSITION_AGE_S, or ADSB_MIN_USABLE_FRACTION vars.
• The existing ADSBLOL_TIMEOUT_MS comment still describes a per-request timeout ("Upstream request timeout"), but per proxy/server.js:194-198 it's now the shared budget for the whole chain. Worth updating so operators don't size it assuming one-request-per-timeout.

2. No CI job runs the new tests
test/proxy-chain.test.js is a solid addition, but I don't see node --test wired into any .github/workflows/*.yml (docker_build.yml only builds/publishes the image) or a package.json test script. Right now these tests only run if someone remembers to invoke them manually, so a regression in the chain logic wouldn't be caught automatically. Might be worth a lightweight CI job (node --test test/) even without a full package.json.

3. weakAnswer fallback keeps the first weak source, not the best one (proxy/server.js:207-254)
When no source clears MIN_USABLE_FRACTION, the code caches whichever source answered first with any aircraft (if (!weakAnswer) weakAnswer = {...}), not the one with the highest usable count among those actually consulted. Concrete case: source A (first in the list) answers 100 aircraft, 40 usable (40%); source B (second) answers 100 aircraft, 45 usable (45%) — both below a 0.5 threshold. The chain serves A's 40 usable positions instead of B's 45, even though B was strictly better and was already fetched. Given the whole point of the chain is "prefer usable positions," it seems worth tracking the weak answer with the most usable positions seen, rather than just the first one that answered. Easy fix: compare usable counts before overwriting weakAnswer.

4. Minor: continue vs break on budget exhaustion (proxy/server.js:214-218)
Once remaining < MIN_ATTEMPT_MS for one source, every later source in the loop will have an equal-or-smaller remaining (time only moves forward), so they'll all be skipped too. continue still gets there, just via extra iterations and log lines ("skipped, chain budget spent" x N). A break would make the intent ("budget's gone, stop trying") clearer and skip the redundant iterations — purely stylistic, not a bug.

5. Test coverage gap (nice-to-have, not blocking)
No test exercises a 3+ source chain (only ever two sources), and none explicitly asserts that a mid-chain source is skipped once the shared budget is exhausted (the "one time budget" test proves the timing but doesn't assert on hits.length for the second slow source to confirm it was skipped rather than attempted-and-timed-out).

Everything else looks solid: the shared-time-budget design is correctly bounded below the consumer's 5s timeout, the MIN_ATTEMPT_MS guard against death-by-a-thousand-timeouts is sensible, hostOf() fails closed to the raw string for a malformed URL rather than throwing, and the cache/staleness semantics (cache.source propagated through to X-Data-Source, "all sources answered but nothing usable" cached fresh rather than serving stale) are consistent with the documented reasoning. The backward-compatible default (ADSB_UPSTREAMS unset → https://api.adsb.lol alone) is a nice touch for zero-migration-pain rollout.

🤖 Generated with Claude Code

[claude]

Review

Clean, well-commented change overall — the shared time-budget reasoning and the local-file precedence are both preserved correctly, and the tests exercise real behavior via subprocess + fake HTTP servers rather than mocking internals.

PR description doesnt match the implementation (important)

The PR bodys central claim is that this chain "picks whichever currently has usable positions," gated by two new env vars: ADSB_MAX_POSITION_AGE_S (default 5) and ADSB_MIN_USABLE_FRACTION (default 0.5). Neither variable, nor any usable-position-fraction logic, appears anywhere in the diff. A repo-wide grep for both names turns up no matches.

What refreshRemote() (proxy/server.js:159-189) actually implements is much simpler: it tries each upstream in order and stops at the first one that returns HTTP 200 with parseable JSON — it never inspects seen_pos on the returned aircraft, so a source that answers 200 with a full page of stale positions is treated identically to one with fresh ones.

That mismatch matters because it contradicts the PRs own worked example. The body states adsb.retina.fm answers ~100% of polls but only has usable positions 0-90% of the time on a ~10s cycle, and gives this log trace as evidence the chain adapts: "adsb.retina.fm: 179 aircraft, 0 usable (0%), trying next source" followed by "api.adsb.lol: 260 aircraft, 220 usable (85%)".

With the code as written, if adsb.retina.fm is listed first in ADSB_UPSTREAMS, it will win the chain on nearly every request purely because it returns 200 — even during the part of its cycle where 0% of positions are usable — and api.adsb.lol would rarely if ever be tried as a fallback. The "trying next source" behavior shown in the transcript is not something this diff can produce as written.

Also, the PR claims "Adds the repos first tests: 14 cases over the chain," but test/proxy-chain.test.js has 5 test() blocks (matching the commit message, which says 5). Worth reconciling the PR description with what actually shipped — either the usable-fraction logic was written and then dropped before this diff was generated, or the description is aspirational/stale relative to the code.

Other notes (minor)

• hostOf()/upstreamUrl() (proxy/server.js:39-49) assume every ADSB_UPSTREAMS entry includes a scheme. An entry without http(s):// (e.g. a copy-paste of just adsb.lol) fails new URL() silently (caught, falls back to the raw string as the "host"), but then fetchUrls url.startsWith(https) check picks plain http, and http.get will throw on a schemeless URL — this surfaces as an unhandled path only when someone misconfigures the env var. Not a blocker, but a one-line validation/log-and-skip for malformed entries would make the failure mode clearer than a stack trace at request time.
• Nit: the comment at proxy/server.js:51 still says "Last good remote response" — accurate, but worth double-checking every remaining adsb.lol-specific comment/log string (e.g. the ADSBLOL_* env var names) was intentionally left as-is rather than renamed, since the feature is no longer adsb.lol-specific. Naming/consistency nit, not a bug.
• Test coverage for the actual "prefer a fresher source over the first that merely answers" behavior described in the PR does not exist yet — understandable, since that logic is not in the diff, but flagging since it is the features main selling point per the description.

Security / performance

No concerns — the URL construction uses RECEIVER_LAT/LON and a fixed path template rather than interpolating anything from the upstream list into a shell or eval context, error response bodies are capped when reading error details (err.length < 200), and the shared in-flight promise plus cache-age gating still prevent request storms against a failing upstream.