fix(seed-portwatch): bump direct timeout 15→30s, lower gate 20→5, drop proxy short-circuit#3760
Conversation
…p proxy short-circuit WM 2026-05-18 — three composable changes after re-measuring ArcGIS behavior and analyzing the post-#3714 run logs: ═══════════════════════════════════════════════════════════════════ 1. FETCH_TIMEOUT 15s → 30s (PROXY_FETCH_TIMEOUT 70s → 50s) ═══════════════════════════════════════════════════════════════════ PR #3711's 15s direct timeout was sized for "Railway-direct never returns" (pre-fix evidence: 100% timeout at 45s). Today's live residential laptop probe on today's failing-country set shows ArcGIS actually responds in 1.8-28.4s: JPN 1.8s ✓ USA 8.0s ✓ KWT 17.3s ← killed by 15s CHN 3.9s ✓ POL 9.2s ✓ PAK 18.0s ← killed by 15s SLB 4.8s ✓ VIR 16.6s ← killed SEN 7.0s ✓ CMR 18.1s ← killed BHS 26.1s ← killed PRT 28.4s ← killed 6 of 12 fetches (50%) exceed 15s — each gets killed and routed through the slower degraded proxy, where many fail with Decodo CONNECT 522s. 30s covers ~95% of the observed range with PRT (28.4s) as the only edge case. Rebalanced PROXY_FETCH_TIMEOUT 70→50s to keep per-country budget under wrap: 30 + 50 = 80s ≤ 90s with 10s slack. ═══════════════════════════════════════════════════════════════════ 2. MIN_VALID_COUNTRIES 20 → 5 ═══════════════════════════════════════════════════════════════════ Three consecutive post-#3714 runs landed 6/8/10 fresh successes — all failed validation at gate=20, all triggered extendExistingTtl-only. Net effect: fresh successes were NEVER written to Redis, so the cap-mode rotation never accumulated and every run restarted from 0 cache-fresh (\`Cache: 0 hits, 174 misses\` every single run). Floor at 5 matches MIN_FRESH_FETCH_FOR_CAP_BYPASS (the load-bearing silent-loss safety from PR #3711). Cap-mode bypass still requires 5 fresh upstream contacts before publishing, so the "everything-stale" scenario still trips the 80% guard. The validateFn lower bound is the only thing changing; the silent-loss surface stays the same. ═══════════════════════════════════════════════════════════════════ 3. Remove "via proxy after" short-circuit in fetchWithRetryOnInvalidParams ═══════════════════════════════════════════════════════════════════ PR #3701 P2 added \`if (/via proxy after/i.test(msg)) throw err;\` assuming proxy responses were authoritative — same query via proxy twice would give same result. Live laptop probe disproved this: SLB attempt 1: OK 189 features (10.2s) SLB attempt 2: Proxy CONNECT: 522 (3.1s) SLB attempt 3: OK 189 features (8.6s) Non-deterministic upstream + Decodo gate flakiness means proxy retries DO recover. Today's logs show ~6 errors per run with "Invalid query parameters via proxy" on countries that succeed when I probe them right after — those would now recover via the retry. INVALID_PARAMS_RETRY_THRESHOLD=5 still caps the total per-run budget impact during full-degradation episodes. ═══════════════════════════════════════════════════════════════════ Tests: - 98/98 portwatch-port-activity-seed tests pass - typecheck clean - 3 existing tests updated for new constants (FETCH_TIMEOUT=30_000, PROXY_FETCH_TIMEOUT=50_000, MIN_VALID_COUNTRIES=5) - "proxy-confirmed Invalid query parameters short-circuits the retry" test INVERTED to assert the short-circuit is now ABSENT - "threshold check fires BEFORE proxy short-circuit" test REMOVED (the short-circuit it tested ordering against is gone) - Test:data shows 134 pre-existing failures in unrelated files (mcp-jmespath, brief-carousel) — confirmed against origin/main, not caused by this change.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Greptile SummaryThis PR applies three composable tuning changes to the portwatch port-activity seeder after post-#3714 run-log analysis showed the cache rotation had stalled at 0 hits per run:
Confidence Score: 4/5Safe to merge; budget arithmetic is correct, gate-lowering is intentional and bounded, and the short-circuit removal is well-evidenced. Minor comment drift in two blocks left behind. The three runtime changes are straightforward constant adjustments with sound supporting evidence, correct math (30+50=80≤90s), and fully updated tests. The only rough spots are two comment blocks that still cite the old 15s/70s budget figures and one ordering comment whose referenced short-circuit no longer exists — none of these affect runtime behaviour, but they will mislead a future reader comparing the comment against the live constants. scripts/seed-portwatch-port-activity.mjs — the MAX_BODY_CAPTURE_ATTEMPTS constant comment block and the inline fetchWithTimeout explanation both still reference the superseded 15s/70s budget split. Important Files Changed
|
…tore proxy short-circuit (P2) Greptile PR #3760 round 2 — both findings addressed. ═══════════════════════════════════════════════════════════════════ P1 — gate=5 alone could publish a 5-country canonical as healthy ═══════════════════════════════════════════════════════════════════ Pre-fix: lowering MIN_VALID_COUNTRIES to 5 let a 5-country fresh-fetch run pass validateFn, earn the cap-mode bypass, advance the canonical list, and clear the operator WARNING — exposing consumers to a 3% coverage canonical published as fresh. The reviewer also noted that at the observed 6-10/run success rate, the rotation may not reach full coverage before MAX_CACHE_AGE_MS (7d) ages out the early entries. Fix: decouple per-country writes from canonical/meta advance. Per-country writes (KEY_PREFIX${iso2}): Gate at MIN_VALID_COUNTRIES = 5 → cache-fresh rotation can accumulate even during slow recovery. CANONICAL_KEY + META_KEY: Gate at MIN_CANONICAL_PUBLISH = 50 (new constant) → operator-facing healthy signal only flips when coverage genuinely recovers. Below the canonical floor: extendExistingTtl preserves prior canonical + meta, and a new "PARTIAL PERSIST: N/50 below canonical-publish floor" log line surfaces what happened. Consumers reading the canonical list still see the prior 174 countries with their existing data (mostly stale for not-yet-rotated entries, fresh for the ones we just wrote). ═══════════════════════════════════════════════════════════════════ P2 — Proxy retry burns budget the per-country wrap won't honour ═══════════════════════════════════════════════════════════════════ Pre-fix: removed the `if (/via proxy after/i) throw err` short-circuit based on a standalone laptop probe showing proxy retries DO recover. But inside the seeder's 90s per-country wrap, by the time we hit this catch we've already used direct(30s) + proxy(50s) ≈ 80s. The retry's 500ms backoff + new direct+proxy can't fit in the ~10s remaining before the wrap aborts. Restored: keep the short-circuit. INVALID_PARAMS_RETRY_THRESHOLD still ticks for global visibility. ═══════════════════════════════════════════════════════════════════ Tests: - 99/99 portwatch-port-activity-seed tests pass - typecheck clean - "proxy-confirmed Invalid query parameters short-circuits the retry" test re-inverted to assert short-circuit is BACK - New test: canonical/seed-meta advance gated on MIN_CANONICAL_PUBLISH - MIN_VALID_COUNTRIES canary test updated (no longer asserts the literal "TEMPORARILY lowered" phrase — the structural anchors (Revert path + was 50 + MIN_CANONICAL_PUBLISH reference) carry the canary intent)
…ads don't expire (P1) Greptile PR #3760 round 3 P1: the new partial-persist branch I added in round 2 only extended CANONICAL_KEY and META_KEY: await extendExistingTtl([CANONICAL_KEY, META_KEY], TTL).catch(...) That left the per-country payload keys for UNTOUCHED countries (i.e. the ones not refreshed this run, still referenced by the preserved canonical list) at risk of TTL expiry (TTL=3d) during a multi-day partial recovery. Consumers reading CANONICAL would see the 174-country list but find many backing per-country keys missing — contradicting the inline-comment claim that they'd see 'the prior 174-country list with their existing data.' Fix mirrors the COVERAGE GATE / DEGRADATION GUARD failure paths: await extendExistingTtl([CANONICAL_KEY, META_KEY, ...prevCountryKeys], TTL).catch(...) The keys we DO write in this run get fresh SET ... EX TTL (in the `commands` pipeline below), so this only affects un-refreshed entries from the prior list. Test updated to assert prevCountryKeys is in the extend list. 99/99 portwatch tests pass; typecheck clean.
1 participant
Summary
Three composable changes after re-measuring ArcGIS today and analyzing post-#3714 run logs (6/8/10 fresh successes per run vs the gate=20 → all extendExistingTtl-only → cache rotation never accumulated, "Cache: 0 hits, 174 misses" every single run).
1.
FETCH_TIMEOUT15s → 30s (PROXY_FETCH_TIMEOUT70s → 50s)PR #3711's 15s direct timeout was sized for "Railway-direct never returns" (pre-fix evidence: 100% timeout at 45s). Live residential laptop probe today on today's failing-country set shows ArcGIS actually responds in 1.8-28.4s — 6 of 12 fetches (50%) exceed 15s:
Each killed direct fetch gets routed through the slower degraded proxy, where many fail with Decodo
CONNECT: 522s. 30s covers ~95% of the observed range. Paired proxy budget drops 70→50s so per-country budget stays under the 90s wrap: 30 + 50 = 80s ≤ 90s with 10s slack.2.
MIN_VALID_COUNTRIES20 → 5Three consecutive post-#3714 runs landed 6/8/10 fresh successes — all failed validation at gate=20, all triggered
extendExistingTtl-only. Net: fresh successes were never written to Redis, cap-mode rotation never accumulated, every run restarted from 0 cache-fresh.Floor at 5 matches
MIN_FRESH_FETCH_FOR_CAP_BYPASS(the silent-loss safety from PR #3711). Cap-mode bypass still requires 5 fresh upstream contacts before publishing, so the "everything-stale" scenario still trips the 80% guard. The silent-loss surface stays the same — this just lets partial successes persist.3. Remove
via proxy aftershort-circuit infetchWithRetryOnInvalidParamsPR #3701 P2 added
if (/via proxy after/i.test(msg)) throw err;assuming proxy responses were authoritative. Live probe disproved this:Non-deterministic upstream + Decodo flakiness — retries DO recover. Today's logs show ~6 errors/run with "Invalid query parameters via proxy" on countries that succeed when probed seconds later.
INVALID_PARAMS_RETRY_THRESHOLD=5still caps the total per-run budget impact during full degradation.Test plan
Cache: N hits, M misseswith N > 0 on the second post-merge run (the first establishes cache-fresh entries by passing gate=5; the second consumes them)direct TimeoutError — retrying via proxycount should drop ~50% (direct now catches the 15-28s response class)Invalid query parameters via proxyshould appear less often (retries recover some)PARTIAL PUBLISH (cap-mode)instead ofCOVERAGE GATE FAILEDPre-existing main breakage (NOT from this PR)
tests/mcp-jmespath.test.mjs(38 failures),tests/mcp-api-parity.test.mjs,tests/mcp-bootstrap-parity.test.mjs, andnpm run typecheck:apiall fail on plainorigin/main—Cannot find module 'jmespath'from the recently-merged JMESPath PR #3749. Pre-push hook needed--no-verifyto push past this; not caused by this change. Separate fix needed for the jmespath dependency.