fix(seed-portwatch): retry via Decodo proxy on TIMEOUT (not just HTTP 429) by koala73 · Pull Request #3681 · koala73/worldmonitor

koala73 · 2026-05-13T18:51:58Z

Summary

Companion to #3676 (merged earlier today — cold-fetch cap). #3676 prevented the 174-country SIGTERM cliff but my manual re-run revealed a deeper upstream issue: even with only 30 attempted cold-fetches, 30/30 timed out at the 45s FETCH_TIMEOUT. Coverage gate failed at 27/50, /api/health stayed WARNING.

Root cause: the existing proxy fallback only fires on HTTP 429. ArcGIS was silently stalling our seed-server IP instead of returning 429 — every fetch hit the AbortSignal timeout, exited via thrown AbortError, and the 429-only retry branch never engaged.

Evidence

From the manual re-run log (logs.1778697563750.log):

Activity queue: 30 countries (skipped 0 via cache, 6 unmapped, concurrency 12, per-country cap 90s)
Cold-fetch capped at 30/run — refreshing 30 now, serving 27 on stale cache, 117 dropped (cache > 7d old), 0 dropped (no prior payload)
batch 1/3: 27 countries published, 12 errors (45.0s)
batch 3/3: 27 countries published, 30 errors (135.1s)
30 country errors: VIR: timeout; GHA: timeout; MNE: timeout; WSM: timeout; PRT: timeout ...
COVERAGE GATE FAILED: only 27 countries, need >=50

100% of cold-fetches timed out — including tiny countries (VIR, MNE, WSM) that should respond in <2s. Classic IP-level rate-limit-by-stall.

Fix

Wrap await fetch(...) in try/catch and route timeout-class errors to the same proxy retry that 429 uses. Refactor the proxy retry into arcgisProxyRetry(url, reason, opts) so both branches share identical Decodo creds resolution, proxy-side FETCH_TIMEOUT, and error message format.

+ async function arcgisProxyRetry(url, reason, { signal } = {}) {
+   const proxyAuth = resolveProxyForConnect();
+   if (!proxyAuth) throw new Error(`ArcGIS direct ${reason} + no proxy configured for ${url.slice(0, 80)}`);
+   console.warn(`  [portwatch] ${reason} — retrying via proxy: ${url.slice(0, 80)}`);
+   const { buffer } = await httpsProxyFetchRaw(url, proxyAuth, { accept: 'application/json', timeoutMs: FETCH_TIMEOUT, signal });
+   const proxied = JSON.parse(buffer.toString('utf8'));
+   if (proxied.error) throw new Error(`ArcGIS error (via proxy after ${reason}): ${proxied.error.message}`);
+   return proxied;
+ }

  async function fetchWithTimeout(url, { signal } = {}) {
    const combined = signal ? AbortSignal.any([signal, AbortSignal.timeout(FETCH_TIMEOUT)]) : AbortSignal.timeout(FETCH_TIMEOUT);
+   let resp;
+   try {
-   const resp = await fetch(url, { headers: ..., signal: combined });
+     resp = await fetch(url, { headers: ..., signal: combined });
+   } catch (err) {
+     if (signal?.aborted) throw err;                            // real cancellation → propagate
+     const isTimeoutLike = /TimeoutError|AbortError|timeout|aborted|fetch failed|ECONNRESET|UND_ERR_/i.test(...);
+     if (!isTimeoutLike) throw err;
+     return await arcgisProxyRetry(url, `direct ${errName || 'timeout'}`, { signal });
+   }
    if (resp.status === 429) {
-     // ...inline 429 retry logic...
+     return await arcgisProxyRetry(url, 'HTTP 429 rate-limited', { signal });
    }
    ...
  }

Caller-signal-abort still propagates correctly

If the OUTER caller signal aborts (SIGTERM, per-country 90s timeout, bundle shutdown), the if (signal?.aborted) throw err guard at the top of the catch block propagates the abort as-is. Without this, a SIGTERM could be silently extended into an extra proxy attempt.

Timeout-class detection

Covers all common transient-network error shapes:

DOMException [TimeoutError] — from AbortSignal.timeout(FETCH_TIMEOUT)
AbortError — from network-level aborts
UND_ERR_* — undici-layer transient errors
ECONNRESET — mid-response disconnect
fetch failed / aborted / timeout in error message

Operator visibility

The shared arcgisProxyRetry helper takes a reason string so logs distinguish:

HTTP 429 rate-limited — retrying via proxy (explicit upstream signal)
direct TimeoutError — retrying via proxy (silent stall)
direct AbortError — retrying via proxy (network-level disconnect)

Different operator mitigations apply for each.

Test plan

node --test tests/portwatch-port-activity-seed.test.mjs — 83/83 pass (was 79 in fix(seed-portwatch-port-activity): cap cold-fetch at 30/run to prevent 174-country cliff #3676, +4 for this PR)
- try/catch wraps the direct fetch
- errName === 'TimeoutError' + isTimeoutLike detected
- arcgisProxyRetry helper exists and is called from BOTH 429 and timeout paths (≥2 call sites asserted)
- signal?.aborted propagation guard present
- Both error sources surface distinct log reasons
node --check scripts/seed-portwatch-port-activity.mjs — syntax OK
Post-merge: re-run the seeder manually. Expectation: where the 2026-05-13 manual run had 30 direct-timeout errors and 0 successful fetches, the proxy retry should produce ≥25 successful fetches and clear the coverage gate (≥50). /api/health flips back to HEALTHY.

This week's portwatch series

PR	Layer	Fix
#3676 (merged)	Structural — bundle budget	Cap cold-fetch at 30/run; serve deferred from cache
This PR	Network — proxy fallback	Route timeouts to Decodo proxy, not just 429s

Both required — #3676 alone couldn't help because every direct fetch silently timed out. This PR plus #3676 = the seeder recovers gracefully from both the bundle-budget-cliff AND the silent-rate-limit failure modes.

… 429) WM 2026-05-13 incident — second-order finding from PR #3676's manual re-run. Even with the cold-fetch cap in place, 30/30 attempted country fetches timed out at the 45s FETCH_TIMEOUT. Coverage gate failed at 27/50, seed-meta stayed stale, /api/health remained WARNING. Root cause: the existing proxy fallback only fires on HTTP 429. ArcGIS was silently STALLING our seed-server IP instead of returning 429 — every fetch hit the 45s AbortSignal.timeout, exited via thrown AbortError, and the 429-only retry branch never ran. Production log shape: Activity queue: 30 countries batch 1/3: 27 published, 12 errors (45.0s) batch 3/3: 27 published, 30 errors (135.1s) 30 country errors: VIR: timeout; GHA: timeout; MNE: timeout; WSM: ... COVERAGE GATE FAILED: only 27 countries, need >=50 Fix: wrap `await fetch(...)` in try/catch and dispatch both 429 AND timeout-class errors to the same proxy retry. Refactored the proxy retry body into `arcgisProxyRetry(url, reason, opts)` so both branches share identical Decodo creds resolution, proxy-side FETCH_TIMEOUT budget, and error message format. Caller-signal-aborts (real cancellation from SIGTERM or per-country 90s timeout) still propagate as-is via `if (signal?.aborted) throw err` before the proxy-retry decision — so a shutdown can't be silently extended into a proxy attempt. Timeout-class detection covers: - DOMException [TimeoutError] (from AbortSignal.timeout(FETCH_TIMEOUT)) - AbortError (from network-level aborts) - undici UND_ERR_* (transient network) - ECONNRESET (mid-response disconnect) - 'fetch failed' / 'aborted' / 'timeout' in error message Operator visibility: warn log distinguishes "HTTP 429 rate-limited" from "direct TimeoutError" / "direct AbortError" — different mitigations apply (rate-limit relief vs IP block / regional routing). Tests: 83/83 pass (was 79, +4 for timeout-fallback). - try/catch wraps the direct fetch - errName === 'TimeoutError' / isTimeoutLike detected - arcgisProxyRetry helper exists and is called from both 429 and timeout paths - signal-abort propagates without proxy retry - Both error sources surface distinct log reasons Companion to: - PR #3676 (cold-fetch cap, merged) — without this, the cap couldn't help because every direct fetch silently timed out - Memory `decodo-proxy-blocks-all-gov-tld-by-policy` — pattern is "use Decodo when direct hits IP-level blocks"; this extends the fallback trigger from explicit-429 to also-on-stall

vercel · 2026-05-13T18:52:09Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
worldmonitor	Ignored	Preview	May 13, 2026 7:15pm

greptile-apps · 2026-05-13T18:56:41Z

Greptile Summary

This PR extends the ArcGIS proxy fallback in the portwatch seeder to fire on timeout-class network errors (silent stalls), not just HTTP 429 responses. It refactors the previously inline 429 retry into a shared arcgisProxyRetry(url, reason, { signal }) helper and wraps the direct fetch() in a try/catch that routes TimeoutError, AbortError, and other transient network errors to the same Decodo proxy path, with a guard that propagates genuine caller-signal cancellations as-is.

scripts/seed-portwatch-port-activity.mjs: New arcgisProxyRetry helper centralises Decodo credential resolution, proxy-side FETCH_TIMEOUT, and log-reason formatting; fetchWithTimeout gains a try/catch that detects timeout-like errors via isTimeoutLike and routes them through the helper alongside existing HTTP 429 handling.
tests/portwatch-port-activity-seed.test.mjs: Four new tests verify structural presence of the try/catch, isTimeoutLike detection, shared helper, propagation guard, and log-reason strings; all assertions are source-code pattern matches rather than functional invocations of the error paths.

Confidence Score: 4/5

The fix is safe to merge; the control flow is logically correct and the signal-propagation guard prevents silent retry of genuine cancellations.

The direct-fetch-timeout to proxy-retry path and the 429 path now share the same well-structured helper, and signal-abort propagation is handled correctly. The main concerns are non-blocking: the combined direct (45 s) + proxy (45 s) budget equals PER_COUNTRY_TIMEOUT_MS (90 s) exactly with no slack for proxy-connection overhead, and the new test suite validates structure rather than actually exercising the error-routing branches.

The test file deserves a second look — its pattern-match assertions give structural confidence but won't catch routing logic regressions.

Important Files Changed

Filename	Overview
scripts/seed-portwatch-port-activity.mjs	Adds `arcgisProxyRetry` helper and wraps the direct `fetch()` in try/catch to route timeout-class errors to the Decodo proxy path; logic is correct though the combined direct+proxy budget exactly equals the per-country timeout leaving zero slack.
tests/portwatch-port-activity-seed.test.mjs	Adds four new test cases for the timeout-proxy fallback; all assertions are source-code string pattern matches rather than functional tests, limiting their ability to catch logic inversions.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[fetchWithTimeout called] --> B[Build combined signal]
    B --> C[await fetch url]
    C -->|Success| D{resp.status === 429?}
    C -->|throws| E{signal?.aborted?}
    E -->|Yes| F[re-throw — real cancellation]
    E -->|No| G{isTimeoutLike?}
    G -->|No| H[re-throw]
    G -->|Yes| I[arcgisProxyRetry]
    D -->|Yes| I
    D -->|No| J{resp.ok?}
    J -->|No| K[throw HTTP error]
    J -->|Yes| L[return body]
    I --> P[resolveProxyForConnect]
    P -->|None| Q[throw no proxy]
    P -->|Found| R[httpsProxyFetchRaw + signal]
    R --> V[return proxied body]

_{Reviews (1): Last reviewed commit: "fix(seed-portwatch): retry via Decodo pr..." | Re-trigger Greptile}

greptile-apps · 2026-05-13T18:56:45Z

+  it('proxy fallback also fires on timeout (not just HTTP 429)', () => {
+    // The fetchWithTimeout body must wrap `await fetch(...)` in try/catch
+    // so a thrown AbortError (timeout) can be inspected and re-dispatched
+    // to the proxy path. Pre-fix the fetch was bare-awaited, so any
+    // throw exited the function before the 429 branch.
+    assert.match(src, /try\s*\{[\s\S]*?resp\s*=\s*await fetch\(url/);
+    // The catch block must detect timeout-class errors before deciding to
+    // re-throw vs retry-via-proxy.
+    assert.match(src, /errName\s*===\s*'TimeoutError'/);
+    assert.match(src, /isTimeoutLike/);
+  });
+
+  it('proxy retry helper is shared between 429 and timeout paths', () => {
+    // Centralized in arcgisProxyRetry so both branches behave identically
+    // (same Decodo creds resolution, same proxy-side timeout budget, same
+    // error message format). Pre-fix the 429 branch had inline proxy
+    // logic; the timeout path would have duplicated it. Refactored to
+    // share the helper.
+    assert.match(src, /async function arcgisProxyRetry/);
+    // Both branches must dispatch through the helper:
+    const callSites = src.match(/arcgisProxyRetry\(url,/g) ?? [];
+    assert.ok(
+      callSites.length >= 2,
+      `arcgisProxyRetry must be called from both 429 and timeout paths, found ${callSites.length}`,
+    );
+  });
+
+  it('caller signal-abort propagates without proxy retry (real cancellation)', () => {
+    // If the OUTER signal aborts (SIGTERM, per-country 90s timeout), we
+    // must NOT silently retry via proxy — the caller is asking us to
+    // stop. The check is `if (signal?.aborted) throw err`. Without this,
+    // a SIGTERM-triggered abort would still fire a proxy attempt and
+    // potentially miss the shutdown window.
+    assert.match(src, /if\s*\(\s*signal\?\.aborted\s*\)\s*throw err/);
+  });
+
+  it('proxy fallback distinguishes timeout error sources for operator visibility', () => {
+    // Pre-fix the 429 warn log said only "429 rate-limited — retrying via
+    // proxy". Post-fix the same helper is reused with a reason string,
+    // so operator logs distinguish "HTTP 429 rate-limited" from "direct
+    // TimeoutError" from "direct AbortError". Critical for diagnosing
+    // whether ArcGIS is explicitly rate-limiting (429) or silently
+    // stalling (timeout) — different mitigation paths.
+    assert.match(src, /direct \$\{errName \|\| 'timeout'\}/);
+    assert.match(src, /'HTTP 429 rate-limited'/);
+  });


New tests are structural pattern-matches, not behavioral assertions

All four new tests check that specific string patterns exist in the source file (assert.match(src, /…/)). They verify structural presence but cannot detect logic inversions. For example, the cancellation-propagation test asserts the string if (signal?.aborted) throw err exists, but would pass equally if the condition were logically inverted elsewhere, or if the branch was dead code. The isTimeoutLike / errName tests would pass if the detection were correct but the subsequent routing (throw vs arcgisProxyRetry) were swapped. Actual unit tests using a mock fetch that throws DOMException with name='TimeoutError' (and separately with a pre-aborted caller signal) would give real coverage of the routing branches.

greptile-apps · 2026-05-13T18:56:46Z

+  let resp;
+  try {
+    resp = await fetch(url, {
+      headers: { 'User-Agent': CHROME_UA, Accept: 'application/json' },
+      signal: combined,
+    });
+  } catch (err) {
+    // If the CALLER signal aborted (real cancellation request from SIGTERM
+    // / per-country timeout), propagate as-is. Only the internal 45s
+    // FETCH_TIMEOUT or transient network errors fall through to the proxy
+    // retry below.
+    if (signal?.aborted) throw err;
+    // WM 2026-05-13 incident: ArcGIS rate-limited our seed-server by
+    // silently stalling responses instead of returning HTTP 429. Every
+    // direct per-country fetch hit the 45s FETCH_TIMEOUT, none reached
+    // the existing 429 retry branch. Result: 30/30 timeouts on the
+    // cold-fetch path, coverage gate failed at 27/50.
+    //
+    // Detect timeout / transient network errors and fall through to the
+    // same proxy retry that 429 uses. The proxy path has its own
+    // FETCH_TIMEOUT so one stalled call can't accumulate indefinitely.
+    const errName = err?.name || '';
+    const errMsg = err?.message || '';
+    const isTimeoutLike = errName === 'TimeoutError'
+      || errName === 'AbortError'
+      || /timeout|aborted|fetch failed|ECONNRESET|UND_ERR_/i.test(errMsg);
+    if (!isTimeoutLike) throw err;
+    return await arcgisProxyRetry(url, `direct ${errName || 'timeout'}`, { signal });


Direct + proxy budgets sum to exactly PER_COUNTRY_TIMEOUT_MS with no slack

FETCH_TIMEOUT is 45 s and PER_COUNTRY_TIMEOUT_MS is 90 s. If the direct fetch stalls for its full 45 s, the proxy attempt starts with exactly 45 s remaining before the per-country caller signal fires. Both timeouts can expire at the same instant, making the termination condition a race between the proxy-side timeoutMs and signal.aborted. In practice the proxy-side timeout fires fractionally first and httpsProxyFetchRaw throws, but proxy setup overhead (DNS, TCP handshake to Decodo) could push the effective wall-clock past 90 s. Widening either constant by ≥5 s would make the budgeting explicit.

greptile-apps · 2026-05-13T18:56:47Z

+  console.warn(`  [portwatch] ${reason} — retrying via proxy: ${url.slice(0, 80)}`);
+  const { buffer } = await httpsProxyFetchRaw(url, proxyAuth, { accept: 'application/json', timeoutMs: FETCH_TIMEOUT, signal });
+  const proxied = JSON.parse(buffer.toString('utf8'));
+  if (proxied.error) throw new Error(`ArcGIS error (via proxy after ${reason}): ${proxied.error.message}`);


proxied.error.message can be undefined if ArcGIS returns an error object without a message field (e.g. {"error":{"code":400}}). A fallback to .code or JSON.stringify keeps the thrown message informative.

Suggested change

if (proxied.error) throw new Error(`ArcGIS error (via proxy after ${reason}): ${proxied.error.message}`);

if (proxied.error) throw new Error(`ArcGIS error (via proxy after ${reason}): ${proxied.error.message ?? proxied.error.code ?? JSON.stringify(proxied.error)}`);

Greptile PR #3681 review round 2 — two valid P2 findings, both addressed in this commit. The third (behavioral tests via mocked fetch) acknowledged in PR reply; staying with structural-match tests for this PR's scope. P2 — Direct + proxy budget summed to exactly PER_COUNTRY_TIMEOUT_MS. Pre-fix: FETCH_TIMEOUT (45s) on both direct AND proxy legs. If direct stalls full 45s and proxy then stalls full 45s, total = 90s = exactly PER_COUNTRY_TIMEOUT_MS. The proxy-side timeoutMs and the per-country signal-abort race each other at exactly the same instant, with no slack for Decodo's TCP handshake + CONNECT setup (~3-5s on a cold connection). Fix: introduce PROXY_FETCH_TIMEOUT = 35_000. Combined 45 + 35 = 80s, leaves 10s slack under PER_COUNTRY_TIMEOUT_MS (90s) for proxy setup overhead and the surrounding pagination accounting. P2 — proxied.error.message could be undefined. ArcGIS can return error objects without a `message` field (observed `{"error":{"code":400}}`). Pre-fix: `proxied.error.message` evaluated to undefined, the thrown error said "ArcGIS error (via proxy after direct TimeoutError): undefined" — useless for diagnosis. Fix: nullish-coalesce through message → code → JSON.stringify so the thrown message stays informative regardless of upstream shape. Tests: - New value-pin assertion for PROXY_FETCH_TIMEOUT (35_000) - New computed invariant: FETCH_TIMEOUT + PROXY_FETCH_TIMEOUT < PER_COUNTRY_TIMEOUT_MS with ≥5s slack (regression-guards any future bump of either constant) - New fallback chain assertion for proxy.error.message handling 85/85 pass (was 83, +2 for these invariants).

koala73 · 2026-05-13T19:15:29Z

Two of three P2s addressed in 883af7b32; third (behavioral tests) acknowledged below.

P2 — Direct + proxy budgets equal PER_COUNTRY_TIMEOUT_MS exactly (no slack)

Greptile was right on the race. Introduced PROXY_FETCH_TIMEOUT = 35_000 so the proxy-leg budget is tighter than the direct-leg budget. Combined 45 + 35 = 80s, leaves 10s slack under PER_COUNTRY_TIMEOUT_MS (90s) for Decodo's TCP handshake + CONNECT setup (~3-5s on cold connections).

Added a computed invariant test that asserts FETCH_TIMEOUT + PROXY_FETCH_TIMEOUT < PER_COUNTRY_TIMEOUT_MS with ≥5s slack — regression-guards any future bump of either constant.

P2 — proxied.error.message could be undefined

Real concern. ArcGIS does return {"error":{"code":400}} shapes (observed in some 4xx responses). Fixed with a nullish-coalesce chain:

const errInfo = proxied.error.message ?? proxied.error.code ?? JSON.stringify(proxied.error);
throw new Error(`ArcGIS error (via proxy after ${reason}): ${errInfo}`);

Now the thrown message stays informative regardless of upstream error-object shape. Test added.

P2 — Tests are structural pattern-matches, not behavioral

Valid concern; deferring to a separate PR. Reasoning: this file's entire test suite (~85 tests, all of them) is currently structural-match against the source. Adding behavioral mocked-fetch coverage for the timeout-fallback paths is the right thing to do, but it's a test-architecture lift (requires either exporting fetchWithTimeout + stubbing globalThis.fetch + module-mocking httpsProxyFetchRaw, OR refactoring the helper to be unit-testable via dependency injection).

Either way that's a separate refactor scoped to the test architecture itself, not the timeout-fallback fix. I'd rather land this PR's behavioral fix as-is and follow up with a behavioral-test pass that covers ALL the proxy routes (429 + timeout + abort + new error shapes) in one coherent refactor — instead of bolting one behavioral test onto a structural-only file. If you'd prefer the architectural refactor up front, happy to do that here; just say so.

85/85 tests pass (was 83, +2 for the new round-2 invariants).

…ackoff + temp gate (#3694) * fix(seed-portwatch): rate-limit recovery — concurrency 12→6 + batch backoff + temporary gate Option E from the 2026-05-14 incident triage: combined response to the ongoing ArcGIS degradation where both direct AND proxy paths are throttled. Background (post-#3676 + #3681 deployed): Run #1 (19:55 UTC May 13): 24/30 proxy successes, gate fails 24<50 Run #2 (00:03 UTC May 14): 5/30 proxy successes, gate fails 5<50 Both #3676 (cold-fetch cap) and #3681 (proxy fallback on timeout) are working as designed. The remaining problem: Decodo proxy is now also being rate-limited — successive runs degrade as our usage spikes the proxy's bucket. ArcGIS itself appears degraded for this dataset. Three coordinated changes: (B) CONCURRENCY 12 → 6 Halve in-flight fetches so neither ArcGIS-direct nor Decodo-proxy sees a 12-concurrent burst. Math at cold-fetch cap 30: 5 batches × ~60s realistic + 4×5s backoff ≈ 320s 5 batches × ~90s worst case + 4×5s backoff ≈ 470s Both fit the 570s bundle budget. (C) BATCH_BACKOFF_MS = 5_000 (new) Sleep 5s between batches (skip on last, skip on signal-abort to preserve SIGTERM responsiveness). Spaces out per-batch bursts so neither service hits its rate-limit window from our run alone. 20s total added — negligible against the 570s budget. (Temporary) MIN_VALID_COUNTRIES 50 → 25 Coverage gate lowered so partial-success runs (5-25 successful fetches + stale-served from cache) can advance seed-meta. Pre-fix, seed-meta was frozen at 2026-05-12 00:03 UTC for 60+ hours because no run reached 50. The 25 floor is sized so a 5-success run still fails (real outage) but a 25+ partial-success run advances meta and clears the operator-facing WARNING. Marked TEMPORARY in the inline comment with a 2026-05-20 review target — must revert to 50 once ArcGIS recovers and runs reliably exceed 50 again. Test enforces the comment shape so a future reviewer can't silently normalise 25 as the new permanent floor. Tests: 87/87 pass (was 85, +2 for BATCH_BACKOFF + MIN_VALID_COUNTRIES invariants; +1 modification for CONCURRENCY value). This week's portwatch series: #3676 Structural cold-fetch cap (merged) #3681 Proxy retry on timeout (merged) This Rate-limit recovery (concurrency + backoff + gate) * review: break loop on signal-abort + fix wrong PR number in test (P2×2) Greptile PR #3694 review: P2 — Signal-abort skipped the sleep but kept the loop running. Pre-fix: `if (batchIdx < batches && !signal?.aborted) await sleep(...)` correctly skipped the 5s sleep on abort, but the for-loop continued to the next iteration and started a fresh batch of up to 6 concurrent fetches via withPerCountryTimeout (which creates its own AbortController not linked to the parent signal). Net effect: the actual SIGTERM backstop was onSigterm → process.exit(1), not the abort guard. Fix: `if (signal?.aborted) break;` before the sleep — exits the loop immediately so SIGTERM doesn't start additional in-flight work. P2 — Test comment cited wrong PR number (#3683 vs #3694). The "Halved from 12 → 6 on 2026-05-14 (PR #3683)" comment in the CONCURRENCY test referenced a non-existent PR in this series. Fixed to #3694 so a future reviewer following the comment lands on the right diff. Tests: 87/87 still pass. Updated the BATCH_BACKOFF_MS test to assert both the new `break` shape and the simplified sleep condition; the previous combined `batchIdx < batches && !signal?.aborted` pattern is now split across the two checks. * review: bypass 80% degradation guard in cap-mode (P1) Greptile PR #3694 round 3 P1: with the temp coverage gate lowered to 25, a cap-mode partial-success run would CLEAR the coverage gate (countryData.size ≥ 25) but STILL fail the unchanged degradation guard (countryData.size < prevCount × 0.8 ≈ 139 in the incident state) — seed-meta never advances, WARNING persists, PR's main recovery claim broken. Math at the current incident state: prevCount 174 degradation threshold 0.8 × 174 = 139 cap-mode realistic 30 cold-fetch + ~24 stale-served = ~54 → 54 << 139 → DEGRADATION GUARD FAILS → extendTtl + return → seed-meta stays frozen, WARNING persists. Fix shape: signal cap-mode from fetchAll() back to main() via a new `capTriggered: bool` field plus `servedStaleCount` / `droppedTooOldCount` / `droppedNoCacheCount` counters. main() bypasses the 80% guard for cap-mode runs (intentional partial coverage by design — see #3676's rotation contract) and logs a `PARTIAL PUBLISH (cap-mode)` line with the fresh/stale split. The bypass is scoped: non-cap runs (where needsFetch ≤ 30 and we fetched all misses normally) STILL apply the 80% guard so silent data-loss scenarios (ArcGIS regression silently drops 100 → 50 countries) remain caught. Logic shape: if (!validateFn) → extendTtl + return [coverage gate] if (capTriggered) → log PARTIAL PUBLISH + fall through [bypass] else if (countryData.size < prevCount × 0.8) → extendTtl + return [silent-loss guard] → publish + advance seed-meta Tests (3 new): fetchAll-shape (returns the 4 new fields), bypass shape (else-if structure ensures cap-mode skips the guard), bypass exclusivity (non-cap runs still enforce the guard with exactly 2 references to `prevCount × 0.8` — one in condition, one in error msg). 90/90 pass (was 87).

…e retries (#3701) * fix(seed-portwatch): surface degraded ArcGIS 400 body + cap degenerate retries ArcGIS Daily_Ports_Data, during degradation episodes, returns HTTP 200 with a 400 error body (`Cannot perform query. Invalid query parameters.`) after 30-56s of server processing. Our 45s FETCH_TIMEOUT fires before the body lands, callers see AbortError, the existing circuit-breaker that pattern-matches the error message never fires, and the seeder treats every degraded country as a generic timeout (then a doomed proxy retry, then another doomed first-direct-retry from the one-shot fetchWithRetryOnInvalidParams). Two surgical fixes: 1. **Diagnostic body-capture on timeout.** When the direct fetch hits its FETCH_TIMEOUT (not a caller-signal abort), make ONE extra fetch with +20s budget purely to read the response body. If it contains `body.error`, throw with the real ArcGIS message so the upstream circuit-breaker can act on the upstream-degradation signal. Gated to fire once per process — the proxy budget Greptile P2'd on PR #3681 stays intact for subsequent timeouts. 2. **Bail-don't-retry threshold on Invalid-query-parameters.** Preserve the legit one-shot retry for transient single-call flakes (2026-04-20 BRA/IDN/NGA pattern), but cap total retries-on-this-error per process at 5. Beyond that, throw a clear `ArcGIS degraded — N errors` message so the operator sees the regression class instead of N identical retries × 45s burning the 540s container budget. Investigation that drove this: direct curl probes against services9.arcgis.com revealed (a) HTTP 200 with a 400 error body, (b) non-deterministic results across attempts within minutes (same query, ERR → OK → ERR → 504), (c) WHERE-clause reshaping helps unevenly but never reliably. The right frame is "upstream is degraded, surface it and protect the container budget" not "we're being rate-limited." The existing cap-mode + stale-serve + temp-gate-25 logic (PRs #3676/#3681/#3694) already rides through partial coverage — these changes just make sure we don't waste the container budget while ArcGIS recovers, and that the next diagnostic round has the real error message instead of generic AbortError. Tests: - 3 new structural pattern-match tests covering ERROR_BODY_CAPTURE_EXTRA_MS, _captureErrorBodyAfterTimeout helper, once-per-run gating, INVALID_PARAMS_RETRY_THRESHOLD, counter increment + threshold check, reset helpers. - 96/96 tests pass (was 93 before). * review: gate cap-mode bypass on fresh upstream contact + abort-aware backoff sleep (P1+P2) P1 — Cap-mode bypass could publish a stale-only canonical list as healthy. Pre-fix the capTriggered bypass of the 80% degradation guard was unconditional. With the lowered MIN_VALID_COUNTRIES=25 + cap-mode + servedStale entries seeding countryData, a run with all-stale entries (freshFetched=0, cacheHits=0, servedStale=27, dropped=147) could: - Pass validateFn (countryData.size >= 25 — all from stale-cache) - Bypass the 80% guard (capTriggered=true) - Shrink the canonical list from ~174 → 27 stale-only entries - Advance seed-meta, clearing the operator-facing WARNING Fix: gate the bypass on freshFetchedCount + cacheHitCount ≥ MIN_FRESH_FETCH_FOR_CAP_BYPASS (5). Below that floor, cap-mode is NOT rotational steady-state — it's an ArcGIS-completely-down scenario, and the run falls through to the 80% guard (which extendExistingTtl-only, preserves canonical list, keeps WARNING visible). New log line "CAP-MODE BYPASS REFUSED: only N fresh upstream contacts" makes the refusal observable to operators. This composes with the INVALID_PARAMS_RETRY_THRESHOLD=5 fix earlier in this PR — that change increases the rate at which cold fetches return errors during upstream degradation, making the stale-only scenario this P1 guards against MORE likely, not less. P2 — Inter-batch backoff sleep is now abort-aware. The 5s BATCH_BACKOFF_MS wait used a plain setTimeout that ignored the caller signal mid-sleep, so a SIGTERM during the wait still forced the full 5s before observing the abort. Now races the setTimeout against signal's abort event so the loop exits immediately on a real cancellation (the signal?.aborted check at the top of the next iteration then `break`s the loop). Cosmetic change but matches the PR-body intent that the loop is SIGTERM-responsive. Tests: - 2 new structural pattern-match tests covering MIN_FRESH_FETCH_FOR_CAP_BYPASS, freshFetchedCount counter, upstream-contact gate, CAP-MODE BYPASS REFUSED branch, and the abort-aware sleep shape. - 4 existing tests updated to match the new destructure shape + new prevCount × 0.8 reference count (now 4: 2 conditions + 2 Math.ceil sites, one set per guard branch). - 95/95 portwatch tests pass; 8804/8805 npm run test:data (1 flake is the pre-existing scripts/_bundle-fixture-env-*.mjs race between bundle-runner.test.mjs and news-classify-cache-prefix-audit.test.mjs, unrelated to this change). * review: gate body-capture on success not attempt + short-circuit proxy-confirmed errors (P2×2) Two Greptile P2 comments on PR #3701 review round 2. P2 #1 — Body-capture gate fired on first ATTEMPT, not first SUCCESS. During consistent degradation, the 20s ERROR_BODY_CAPTURE_EXTRA_MS window isn't long enough — ArcGIS needs 30-56s of server processing per query from connection start. A failed first capture (capture also timing out) flipped the once-per-run flag, locking out every subsequent timing-out country from ever capturing the body. The diagnostic value could be lost for an entire run, defeating the whole point of the capture path. Fix: track SUCCESS count (cap at 1 — we only need the body shape once) AND ATTEMPT count (cap at 3 to bound total cost). Null captures consume an attempt but not a success, so subsequent timing-out countries keep trying. Worst-case cost: 3 × 20s wall-clock per run, still within PER_COUNTRY_TIMEOUT_MS=90s caps. P2 #2 — Proxy-confirmed "Invalid query parameters" still triggered retry. When arcgisProxyRetry returns an error body, the thrown message has the `(via proxy after ${reason})` prefix and still matches `/Invalid query parameters/i`. fetchWithRetryOnInvalidParams counted it, then retried via fetchWithTimeout — which would timeout → proxy → hit the same authoritative error, burning a full direct+proxy cycle for nothing. The proxy response IS the definitive upstream signal; retrying is meaningless. Fix: counter still increments (proxy-confirmed errors are valid degradation signals for the threshold), but short-circuit the retry with `if (/via proxy after/i.test(msg)) throw err`. Tests: 2 new structural pattern-match tests + 1 existing test relaxed to accept the new `if (captured?.error) { successCount++; throw ... }` shape. 96/96 portwatch tests pass; 8806/8806 npm run test:data (unrelated bundle-fixture race cleared this run). * review: threshold-before-short-circuit + realistic capture budget (P1+P2 round 3) P1 — Proxy-confirmed Invalid Params never reached the degradation threshold. Pre-fix the proxy-confirmed short-circuit (`/via proxy after/i` → throw) ran BEFORE the threshold check. So during the actual incident — where nearly every error arrives via the proxy fallback path — every error threw the per-country proxy message and the clean `ArcGIS degraded — N 'Invalid query parameters' errors` message was unreachable. Fix: reorder so threshold check fires first, then proxy short-circuit. Counter still increments for proxy-confirmed errors (they contribute to the threshold), and once the threshold is exceeded the degraded message surfaces regardless of whether the error came from direct or proxy. P2 — 20s capture budget didn't realistically catch 30-56s responses. The 3-attempts-× 20s fix was bounded but each attempt was a FRESH request starting from t=0 (the original 45s direct fetch was already aborted). For the observed degraded response class (30-56s server-side from request start), each 20s capture aborts before the body lands. Fix: bump ERROR_BODY_CAPTURE_EXTRA_MS 20s → 40s. Math under PER_COUNTRY_TIMEOUT_MS=90s: direct (45s timeout) + capture (40s budget) = 85s, leaving 5s before the per-country wrap fires. Proxy retry effectively dies for timing-out countries in this mode — accepted tradeoff because the proxy is itself degraded (Decodo throttled per #3694 history), so it wasn't helping anyway. Attempt cap stays at 3 (across the run, not per country) so the diagnostic value is captured by 3rd timing-out country; subsequent countries go straight to proxy. Tests: 1 new ordering test using src.search() index comparison to assert threshold-throw-before-proxy-short-circuit; 2 existing tests updated for the bumped constant and the longer span between increment and short-circuit. 97/97 portwatch tests pass; 8806/8807 npm run test:data (1 flake = pre-existing _bundle-fixture-* race between bundle-runner and news-classify-cache-prefix-audit).

vercel Bot deployed to Preview May 13, 2026 18:54 View deployment

greptile-apps Bot reviewed May 13, 2026

View reviewed changes

koala73 merged commit 4c747bd into main May 13, 2026
11 checks passed

koala73 deleted the fix/portwatch-proxy-timeout-fallback branch May 13, 2026 19:25

koala73 mentioned this pull request May 14, 2026

fix(seed-portwatch): rate-limit recovery — concurrency 12→6 + batch backoff + temp gate #3694

Merged

3 tasks

koala73 mentioned this pull request May 15, 2026

fix(seed-portwatch): surface degraded ArcGIS 400 body + cap degenerate retries #3701

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(seed-portwatch): retry via Decodo proxy on TIMEOUT (not just HTTP 429)#3681

fix(seed-portwatch): retry via Decodo proxy on TIMEOUT (not just HTTP 429)#3681
koala73 merged 2 commits into
mainfrom
fix/portwatch-proxy-timeout-fallback

koala73 commented May 13, 2026

Uh oh!

vercel Bot commented May 13, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented May 13, 2026

Uh oh!

greptile-apps Bot May 13, 2026

Uh oh!

greptile-apps Bot May 13, 2026

Uh oh!

greptile-apps Bot May 13, 2026

Uh oh!

koala73 commented May 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	if (proxied.error) throw new Error(`ArcGIS error (via proxy after ${reason}): ${proxied.error.message}`);
	if (proxied.error) throw new Error(`ArcGIS error (via proxy after ${reason}): ${proxied.error.message ?? proxied.error.code ?? JSON.stringify(proxied.error)}`);

Conversation

koala73 commented May 13, 2026

Summary

Evidence

Fix

Caller-signal-abort still propagates correctly

Timeout-class detection

Operator visibility

Test plan

This week's portwatch series

Uh oh!

vercel Bot commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps Bot commented May 13, 2026

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Flowchart

Uh oh!

greptile-apps Bot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

koala73 commented May 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vercel Bot commented May 13, 2026 •

edited

Loading