Skip to content

fix(safeFetch): exponential backoff, HTTP 429 awareness, Retry-After support - #121

Open
kawacukennedy wants to merge 1 commit into
calesthio:masterfrom
kawacukennedy:fix/safefetch-429-backoff
Open

fix(safeFetch): exponential backoff, HTTP 429 awareness, Retry-After support#121
kawacukennedy wants to merge 1 commit into
calesthio:masterfrom
kawacukennedy:fix/safefetch-429-backoff

Conversation

@kawacukennedy

Copy link
Copy Markdown
  • I have read CONTRIBUTING.md
  • My code follows the project's style (pure ESM, async/await, no new dependencies)
  • I have tested locally (63/63 unit tests pass, diag.mjs verifies all imports)
  • My change is backward compatible — no existing callers modified
  • I have added 18 new unit tests
  • No documentation changes needed (no new env vars, no behavior visible to users)

Problem

safeFetch() in apis/utils/fetch.mjs has five issues affecting all 27+ source modules:

  1. No HTTP 429 awareness — Rate-limited responses (e.g. OpenSky, GDELT) are treated as generic errors. The README explicitly documents this as a limitation: "OpenSky can also return HTTP 429 when its public hotspots are queried too aggressively. Crucix does not try to evade that limit."
  2. Fixed backoff (no exponential growth) — Retries sleep 2000 × (i+1) ms regardless of error type. If a server sends Retry-After: 30, the header is ignored.
  3. All errors retried equally — 400 Bad Request and 404 Not Found are retried just like 503, wasting time and potentially worsening server load.
  4. AbortController timer leakclearTimeout(timer) is only called on the success path. (Noted in issue fix: safeFetch timer leak, env quote stripping, source count #84.)
  5. No POST/body support — Sources that need HTTP POST (BLS, ReliefWeb) must implement their own fetch wrapper.

Root Cause

safeFetch at apis/utils/fetch.mjs:3-28 was a single flat retry loop with no status-code classification, no exponential backoff, no Retry-After parsing, and timer cleanup only on success.

Solution

apis/utils/fetch.mjs — Rewrite safeFetch

  • Status-code classification: Only retry known-retryable codes (408, 429, 500, 502, 503, 504). 4xx client errors bail immediately.
  • Exponential backoff with jitter: min(100ms × 2^i, 30s) + random(0, 1000ms). Total cumulative backoff capped at 30s.
  • Retry-After header support: When a server sends Retry-After: N, wait exactly N seconds (capped at 60s to prevent pathological waits).
  • Timer leak fix: clearTimeout(timer) in both success and catch paths.
  • POST/body support: New method and body options.
  • Backward compatible: Same function signature. Defaults (retries: 1, method: 'GET') unchanged.

apis/sources/opensky.mjs — Explicit retries

OpenSky is the most rate-limited source (4k credits/day unauthenticated, 10 parallel hotspot queries). Setting retries: 2 on getFlightsInArea gives exponential backoff two chances to succeed before a hotspot returns empty data.

test/safe-fetch.test.mjs — 18 new unit tests

Covers: success, non-JSON, 400/403/404 bail, 429/503/408 retry, Retry-After, network error, timeout abort, POST body, custom headers, max backoff cap.

Testing

  • node --test test/safe-fetch.test.mjs — 18/18 pass
  • node --test (all tests) — 63/63 pass, 0 fail, 1 skipped (needs API key)
  • node diag.mjs — All 12 imports OK, port available, server loads
  • node -e \"import('./apis/utils/fetch.mjs')\" — Module loads without error

Suggested labels

bug, performance, enhancement

…support

safeFetch() had five issues affecting all 27+ source modules:
1. No HTTP 429 awareness — rate-limited responses treated as generic errors
2. Fixed backoff — 2000*(i+1)ms regardless of error type
3. All errors retried equally — 400/404 retried same as 503
4. AbortController timer leak — clearTimeout only on success path
5. No POST/body support — BLS, ReliefWeb had to implement their own fetch

Rewrite with:
- Status-code classification (only retry 408/429/500/502/503/504)
- Exponential backoff with jitter (100ms*2^i + random(0,1000), capped 30s)
- Retry-After header parsing (capped at 60s)
- Timer cleanup in catch path
- POST/body method support

Update OpenSky getFlightsInArea to use retries: 2 for 429 resilience.
Add 18 unit tests covering all new behaviors.

Fixes the known limitation documented in README.md:498.
@kawacukennedy
kawacukennedy requested a review from calesthio as a code owner June 1, 2026 21:26
PetroczyP pushed a commit to PetroczyP/Crucix that referenced this pull request Aug 13, 2026
`briefing()` issued three full-body requests to the OFAC publication exports
on every sweep:

    SDN.XML            27.5 MB
    SDN_ADVANCED.XML  120.0 MB   (metadata)
    SDN_ADVANCED.XML  120.0 MB   (again, for sample entries)
    -------------------------
                      267.5 MB   every 15 minutes  = ~25.7 GB/day

Sizes confirmed from the origin's own Content-Range headers.

`safeFetch` reads the entire body with `res.text()` and only then truncates
to `rawText: text.slice(0, 500)`, so all 267 MB was pulled into memory and
discarded. The inline comment claiming it "will get the first 500 chars"
described an optimisation that does not exist.

The source therefore never completed: two 20s fetches in parallel followed by
a sequential 25s fetch cannot finish inside the 30s per-source budget in
apis/briefing.mjs, so every sweep logged

    Source OFAC timed out after 30s

Two of the three downloads were also pointless. SDN_ADVANCED.XML uses a
different schema — it has no <sdnEntry>, no <Publish_Date> and no
<Record_Count> — so `advancedList` was always all-null and `sampleEntries`
was always empty.

Fix:
- request only the first 64 KB via `Range: bytes=0-65535`; the S3 origin
  advertises `Accept-Ranges: bytes` and answers 206
- bound the read with a streaming reader too, so a proxy that ignores Range
  still cannot pull 120 MB into memory
- fetch each list once and reuse the buffer for metadata and sampling
- take sample entries from SDN.XML, which actually contains <sdnEntry>
- parse the advanced export's <DateOfIssue> block so its date populates

Measured before/after:

    requests   3        -> 2
    transfer   267.5 MB -> 128.0 KB      (~2,140x less)
    duration   timeout  -> 5.0 s
    sampleEntries  0    -> 10
    advancedList.publishDate  null -> 2026-08-07

This does not touch apis/utils/fetch.mjs, so it does not conflict with calesthio#121.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PetroczyP pushed a commit to PetroczyP/Crucix that referenced this pull request Aug 13, 2026
Cherry-picked from upstream PR calesthio#124 (fix2015), with two deliberate changes:

- server.mjs resolved against PR calesthio#128's renderBanner() refactor: calesthio#128
  replaced the inline banner template that calesthio#124 patched, so the structure
  is calesthio#128's and the corrected count (29) is calesthio#124's.
- The User-Agent bump (Crucix/1.0 -> Crucix/2.0) is dropped. It is
  unrelated to this fix, is the sole cause of a conflict with PR calesthio#121,
  and the rename is tracked separately as issue 007.

Source count verified against briefing.mjs: 29 runSource() call sites.
PetroczyP added a commit to PetroczyP/Crucix that referenced this pull request Aug 13, 2026
PR calesthio#121 computes waitMs = Math.min(desired, MAX_BACKOFF_MS - totalBackoff)
and guards it with `if (waitMs > 0)`. Once the accumulated backoff reaches
the 30s ceiling that expression is <= 0, the guard is false, and the retry
fires with no delay at all — against an endpoint that is already rate-limiting
us. calesthio#121's own 18 cases never reach that boundary, so they pass while the
defect stands.

Extracts computeRetryDelay(desired, totalBackoff), which clamps to the
remaining budget but never below MIN_RETRY_DELAY_MS (250ms). The floor
deliberately takes precedence over the ceiling: slightly exceeding the
backoff budget is better than hammering a rate-limited endpoint.

Adds 5 boundary cases covering at-ceiling, past-ceiling and sub-minimum
remaining budget. Project-authored — deliberately NOT folded into the
cherry-pick of calesthio#121, whose author is preserved on its own commit.
PetroczyP added a commit to PetroczyP/Crucix that referenced this pull request Aug 13, 2026
…an we can honour

Live regression found after the calesthio#121 harvest: the WORLD view showed zero
air activity across all 10 hotspots.

OpenSky returns HTTP 429 with 'x-rate-limit-retry-after-seconds: 20934'
(~5.8h) once the anonymous quota is spent. Two defects compounded:

1. Only the standard 'Retry-After' header was read, so OpenSky's vendor
   header was invisible and the request fell through to exponential
   backoff and retried anyway — burning more quota against a metered API.
2. Where a standard header did exist, it was silently capped 20934s -> 60s,
   so we waited the entire 30s backoff budget and retried a server that had
   said 'come back in six hours'.

calesthio#121 also raised OpenSky's retries from 1 to 2, tripling worst-case request
volume (10 hotspots x 3 attempts x 96 sweeps/day) against that quota.

Now: read the wait uncapped from either header; if it exceeds MAX_RETRY_AFTER_SEC,
return immediately with the requested delay attached instead of retrying.

Side effect: sweeps should drop from ~30s to near-instant when OpenSky is
cooling off, since that 30s WAS the backoff.

Project-authored. Does not restore air data while the quota is spent — that
needs last-known-good caching, filed as backlog 009.
PetroczyP added a commit to PetroczyP/Crucix that referenced this pull request Aug 13, 2026
…an we can honour

Live regression found after the calesthio#121 harvest: the WORLD view showed zero
air activity across all 10 hotspots.

OpenSky returns HTTP 429 with 'x-rate-limit-retry-after-seconds: 20934'
(~5.8h) once the anonymous quota is spent. Two defects compounded:

1. Only the standard 'Retry-After' header was read, so OpenSky's vendor
   header was invisible and the request fell through to exponential
   backoff and retried anyway — burning more quota against a metered API.
2. Where a standard header did exist, it was silently capped 20934s -> 60s,
   so we waited the entire 30s backoff budget and retried a server that had
   said 'come back in six hours'.

calesthio#121 also raised OpenSky's retries from 1 to 2, tripling worst-case request
volume (10 hotspots x 3 attempts x 96 sweeps/day) against that quota.

Now: read the wait uncapped from either header; if it exceeds MAX_RETRY_AFTER_SEC,
return immediately with the requested delay attached instead of retrying.

Side effect: sweeps should drop from ~30s to near-instant when OpenSky is
cooling off, since that 30s WAS the backoff.

Project-authored. Does not restore air data while the quota is spent — that
needs last-known-good caching, filed as backlog 009.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant