fix(portwatch): degrade to cold-path when MGET fails (PR #3299 review P1)

Greptile caught a regression: the new redisMgetJson preflight was
awaited without error handling. A transient Upstash outage at run-start
would abort the seed before any ArcGIS data was fetched — worse than
the pre-H behaviour where Redis was only required at the final write.

Fix: wrap redisMgetJson(prevKeys) in .catch that logs and returns an
array of nulls the same length as prevKeys. Every country then falls
through to the expensive fetch path (cache miss), which is correct
behaviour when we can't read the cache. The final write pipeline still
hits Redis and will fail loudly if Redis is genuinely down then — we
just no longer fail BEFORE the expensive work has even started.

Note: Greptile reviewed b79f22923 before the ddec47e5f anchor fix for
the earlier P1 (rolling-window cache staleness). Both fixes stand.

Tests: 61 portwatch (+1). Typecheck + lint clean.
This commit is contained in:
Elie Habib
2026-04-22 22:33:10 +04:00
parent ddec47e5f3
commit 0a4f092cc3
2 changed files with 20 additions and 1 deletions

View File

@@ -408,7 +408,16 @@ export async function fetchAll(progress, { signal } = {}) {
if (progress) progress.stage = 'cache-lookup';
const cacheT0 = Date.now();
const prevKeys = eligibleIso3.map((iso3) => `${KEY_PREFIX}${iso3ToIso2.get(iso3)}`);
const prevPayloads = await redisMgetJson(prevKeys);
// A transient Upstash outage at run-start must NOT abort the seed before
// any ArcGIS data is fetched — that's a regression from the previous
// behaviour where Redis was only required at the final write. On MGET
// failure, degrade to cold-path: treat every country as a cache miss
// and re-fetch. The write at run-end will retry its own Redis calls
// and fail loudly if Redis is genuinely down then too. PR #3299 review P1.
const prevPayloads = await redisMgetJson(prevKeys).catch((err) => {
console.warn(` [port-activity] cache MGET failed (${err?.message || err}) — treating all countries as cache miss`);
return new Array(prevKeys.length).fill(null);
});
console.log(` [port-activity] Loaded ${prevPayloads.filter(Boolean).length}/${prevKeys.length} cached payloads (${((Date.now() - cacheT0) / 1000).toFixed(1)}s)`);
// Preflight: maxDate check for every eligible country in parallel.

View File

@@ -147,6 +147,16 @@ describe('seed-portwatch-port-activity.mjs exports', () => {
assert.match(src, /cacheWrittenAt:\s*Date\.now\(\)/);
});
it('redisMgetJson failure degrades to cold-path (does not abort the seed)', () => {
// PR #3299 review P1: a transient Upstash outage at run-start used to
// abort the seed before any ArcGIS data was fetched — regression from
// the prior behaviour where Redis was only required at write-time.
// The MGET call is now wrapped in .catch that returns all-null so
// every country falls through to the expensive-fetch path.
assert.match(src, /redisMgetJson\(prevKeys\)\.catch\(/);
assert.match(src, /new Array\(prevKeys\.length\)\.fill\(null\)/);
});
it('registers SIGTERM + SIGINT + aborts shutdownController', () => {
assert.match(src, /process\.on\('SIGTERM'/);
assert.match(src, /process\.on\('SIGINT'/);