[object Object]

← back to Costa Rica

costa-rica: ingest layer resilience — fetch timeout + per-record isolation (cycle 25) — TK-10346

f10bf96729c1efdd23262677123aeb2c83f869c4 · 2026-09-24 04:40:10 -0700 · Steve

Cold Cody audit (cycle 24) of the ingest layer found two reliability holes
(no live exploit path, unlike the XSS also found that cycle):
  - no fetch timeout anywhere in scripts/ingest/_lib.js: a hung gov site
    (CKAN, ICT WordPress) could stall the sequential run-all.js FOREVER.
  - one poison record aborted the WHOLE run in meic-pymes.js + ict-cst.js
    (no per-row try/catch) -> a bad row at #500 drops the other ~14,500.

Fix:
- _lib.js: fetchText/fetchJson/fetchBuffer now default to an AbortSignal.timeout
  (30s HTML/JSON via SCRAPER_TIMEOUT_MS, 60s file-download via a SEPARATE
  SCRAPER_BUFFER_TIMEOUT_MS knob); a caller-supplied opts.signal is honored as-is.
- meic-pymes.js + ict-cst.js: wrapped the per-record loop body in its own
  try/catch (errors++, warn capped at 20, continue) mirroring the existing
  google-places.js/local-portals.js pattern; finishRun downgrades to
  status='partial' (not 'error'/exit 1) when errors>0 -- already a live
  status value, no schema/consumer impact (verified: no CHECK constraint,
  the only 2 consumers just display the string).

Cody gate caught a real bug in my own first pass: fetchBuffer's timeout fell
through `Number(opts.timeoutMs) || fallbackMs || DEFAULT_TIMEOUT_MS` where an
absent opts.timeoutMs -> NaN (falsy) -> the hardcoded 60000 fallbackMs literal
always won, silently making SCRAPER_TIMEOUT_MS a no-op for the one fetch (the
multi-MB MEIC XLSX) that most needs a real override on a slow link. Gave
fetchBuffer its own dedicated SCRAPER_BUFFER_TIMEOUT_MS env var instead of a
fallback chain; verified with a live timing test that swaps env vars and reads
actual elapsed ms (not just code inspection). test/ingest-resilience.test.js:
behavioral timeout tests (fake never-resolving fetch) + structural isolation
guards. Suite 209 -> 215, serial green.

Deprioritized (Cody, correctly): osm-fetch.js's raw fetch has no client timeout
but isn't in run-all.js's ORDER array, so it can't stall the pipeline -- low
priority, manual-run-only script. cr-osm-match.js has the same per-record-no-
isolation pattern on a DB-only loop (no fetch) -- queued for a later cycle.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TouFkmUGKHtwqpZwgReVic

Files touched

Diff

commit f10bf96729c1efdd23262677123aeb2c83f869c4
Author: Steve <steve@designerwallcoverings.com>
Date:   Thu Sep 24 04:40:10 2026 -0700

    costa-rica: ingest layer resilience — fetch timeout + per-record isolation (cycle 25) — TK-10346
    
    Cold Cody audit (cycle 24) of the ingest layer found two reliability holes
    (no live exploit path, unlike the XSS also found that cycle):
      - no fetch timeout anywhere in scripts/ingest/_lib.js: a hung gov site
        (CKAN, ICT WordPress) could stall the sequential run-all.js FOREVER.
      - one poison record aborted the WHOLE run in meic-pymes.js + ict-cst.js
        (no per-row try/catch) -> a bad row at #500 drops the other ~14,500.
    
    Fix:
    - _lib.js: fetchText/fetchJson/fetchBuffer now default to an AbortSignal.timeout
      (30s HTML/JSON via SCRAPER_TIMEOUT_MS, 60s file-download via a SEPARATE
      SCRAPER_BUFFER_TIMEOUT_MS knob); a caller-supplied opts.signal is honored as-is.
    - meic-pymes.js + ict-cst.js: wrapped the per-record loop body in its own
      try/catch (errors++, warn capped at 20, continue) mirroring the existing
      google-places.js/local-portals.js pattern; finishRun downgrades to
      status='partial' (not 'error'/exit 1) when errors>0 -- already a live
      status value, no schema/consumer impact (verified: no CHECK constraint,
      the only 2 consumers just display the string).
    
    Cody gate caught a real bug in my own first pass: fetchBuffer's timeout fell
    through `Number(opts.timeoutMs) || fallbackMs || DEFAULT_TIMEOUT_MS` where an
    absent opts.timeoutMs -> NaN (falsy) -> the hardcoded 60000 fallbackMs literal
    always won, silently making SCRAPER_TIMEOUT_MS a no-op for the one fetch (the
    multi-MB MEIC XLSX) that most needs a real override on a slow link. Gave
    fetchBuffer its own dedicated SCRAPER_BUFFER_TIMEOUT_MS env var instead of a
    fallback chain; verified with a live timing test that swaps env vars and reads
    actual elapsed ms (not just code inspection). test/ingest-resilience.test.js:
    behavioral timeout tests (fake never-resolving fetch) + structural isolation
    guards. Suite 209 -> 215, serial green.
    
    Deprioritized (Cody, correctly): osm-fetch.js's raw fetch has no client timeout
    but isn't in run-all.js's ORDER array, so it can't stall the pipeline -- low
    priority, manual-run-only script. cr-osm-match.js has the same per-record-no-
    isolation pattern on a DB-only loop (no fetch) -- queued for a later cycle.
    
    Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
    Claude-Session: https://claude.ai/code/session_01TouFkmUGKHtwqpZwgReVic
---
 scripts/ingest/_lib.js         | 29 ++++++++++++--
 scripts/ingest/ict-cst.js      | 45 ++++++++++++---------
 scripts/ingest/meic-pymes.js   | 14 +++++--
 test/ingest-resilience.test.js | 88 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 151 insertions(+), 25 deletions(-)

diff --git a/scripts/ingest/_lib.js b/scripts/ingest/_lib.js
index 1e075ed..a0b62e5 100644
--- a/scripts/ingest/_lib.js
+++ b/scripts/ingest/_lib.js
@@ -7,24 +7,47 @@ const pool = new Pool({ connectionString: process.env.DATABASE_URL });
 
 const UA = process.env.SCRAPER_UA || 'Mozilla/5.0 (Macintosh; Intel Mac OS X 14_5) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.5 Safari/605.1.15 (CR-Directory bot; contact info@agentabrams.com)';
 
+// Every ingest fetch gets a bounded timeout (connect + full body read). Without
+// one, a hung/slow gov site (ICT WordPress, CKAN, datos.go.cr) stalls the process
+// FOREVER — and run-all.js runs modules sequentially, so one hang silently blocks
+// every later module. A caller that passes its own opts.signal keeps full control;
+// otherwise AbortSignal.timeout fires a TimeoutError that surfaces as a normal
+// rejection the caller's try/catch already handles. (Cody ingest audit, cycle 24.)
+const DEFAULT_TIMEOUT_MS = Number(process.env.SCRAPER_TIMEOUT_MS) || 30000;
+// A SEPARATE env knob for the buffer (file-download) path — NOT a fallback chain
+// through DEFAULT_TIMEOUT_MS. `fetchBuffer(url)` (no opts.timeoutMs) previously
+// always resolved to the hardcoded fallbackMs=60000 literal, because in
+// `Number(opts.timeoutMs) || fallbackMs || DEFAULT_TIMEOUT_MS` an undefined
+// opts.timeoutMs -> NaN (falsy) -> fallbackMs (a truthy literal) always won,
+// silently making SCRAPER_TIMEOUT_MS a no-op for the one fetch (the multi-MB MEIC
+// XLSX) that most needs a real override on a slow link. (Cody gate, cycle 25.)
+const DEFAULT_BUFFER_TIMEOUT_MS = Number(process.env.SCRAPER_BUFFER_TIMEOUT_MS) || 60000;
+function timeoutSignal(opts, fallbackMs) {
+  if (opts.signal) return opts.signal; // caller manages its own abort/timeout
+  if (opts.timeoutMs) return AbortSignal.timeout(Number(opts.timeoutMs));
+  return AbortSignal.timeout(fallbackMs);
+}
+
 async function fetchText(url, opts = {}) {
   const res = await fetch(url, {
     headers: { 'User-Agent': UA, 'Accept': 'text/html,application/xhtml+xml,application/json;q=0.9,*/*;q=0.8', 'Accept-Language': 'es-CR,es;q=0.9,en;q=0.7', ...(opts.headers||{}) },
     redirect: 'follow',
-    signal: opts.signal,
+    signal: timeoutSignal(opts, DEFAULT_TIMEOUT_MS),
   });
   if (!res.ok) throw new Error(`HTTP ${res.status} on ${url}`);
   return await res.text();
 }
 
 async function fetchBuffer(url, opts = {}) {
-  const res = await fetch(url, { headers: { 'User-Agent': UA, ...(opts.headers||{}) }, redirect: 'follow', signal: opts.signal });
+  // Larger default: this pulls multi-MB files (the MEIC XLSX) that legitimately
+  // take longer than an HTML page.
+  const res = await fetch(url, { headers: { 'User-Agent': UA, ...(opts.headers||{}) }, redirect: 'follow', signal: timeoutSignal(opts, DEFAULT_BUFFER_TIMEOUT_MS) });
   if (!res.ok) throw new Error(`HTTP ${res.status} on ${url}`);
   return Buffer.from(await res.arrayBuffer());
 }
 
 async function fetchJson(url, opts = {}) {
-  const res = await fetch(url, { headers: { 'User-Agent': UA, 'Accept': 'application/json', ...(opts.headers||{}) }, redirect: 'follow', signal: opts.signal });
+  const res = await fetch(url, { headers: { 'User-Agent': UA, 'Accept': 'application/json', ...(opts.headers||{}) }, redirect: 'follow', signal: timeoutSignal(opts, DEFAULT_TIMEOUT_MS) });
   if (!res.ok) throw new Error(`HTTP ${res.status} on ${url}`);
   return await res.json();
 }
diff --git a/scripts/ingest/ict-cst.js b/scripts/ingest/ict-cst.js
index d2f70b0..5cc2f99 100644
--- a/scripts/ingest/ict-cst.js
+++ b/scripts/ingest/ict-cst.js
@@ -68,7 +68,7 @@ function extractListings(html) {
 (async () => {
   const runId = await startRun('ict-cst', 'Scrape ICT CST sustainable-tourism directory');
   const rmap = await regionMap();
-  let html, listings = [], inserted = 0, updated = 0;
+  let html, listings = [], inserted = 0, updated = 0, errors = 0;
 
   try {
     html = await fetchText(ROOT);
@@ -80,21 +80,28 @@ function extractListings(html) {
     }
 
     for (const l of listings) {
-      const region = resolveRegion(rmap, l.canton || l.distrito || l.province);
-      const placeSlug = slug(`ict-${l.name}-${region.slug}`);
-      const r = await upsertPlace({
-        slug: placeSlug,
-        name: l.name,
-        category: 'tourism',
-        vertical: l.vertical,
-        region_id: region.id,
-        description: `ICT CST-certified ${l.vertical.replace('tourism_','')} in ${l.canton||l.province||'Costa Rica'}.`,
-        address: l.address,
-        image_url: l.image_url,
-        source: l.source,
-        source_url: l.source_url,
-      });
-      if (r.inserted) inserted++; else updated++;
+      // Per-record isolation: one bad listing must skip + count, not abort the run
+      // via the outer catch. (Cody ingest audit, cycle 24 — google-places pattern.)
+      try {
+        const region = resolveRegion(rmap, l.canton || l.distrito || l.province);
+        const placeSlug = slug(`ict-${l.name}-${region.slug}`);
+        const r = await upsertPlace({
+          slug: placeSlug,
+          name: l.name,
+          category: 'tourism',
+          vertical: l.vertical,
+          region_id: region.id,
+          description: `ICT CST-certified ${l.vertical.replace('tourism_','')} in ${l.canton||l.province||'Costa Rica'}.`,
+          address: l.address,
+          image_url: l.image_url,
+          source: l.source,
+          source_url: l.source_url,
+        });
+        if (r.inserted) inserted++; else updated++;
+      } catch (e) {
+        errors++;
+        if (errors <= 20) console.warn(`[ict-cst] listing skipped (${String(l.name||'').slice(0,40)}): ${e.message}`);
+      }
       await sleep(50);
     }
 
@@ -102,11 +109,11 @@ function extractListings(html) {
       rows_in: listings.length,
       rows_added: inserted,
       rows_updated: updated,
-      status: listings.length ? 'ok' : 'empty',
-      notes: listings.length ? '' : 'parser found no <article class=entidad> blocks; ICT page likely AJAX-rendered, switch to Browserbase'
+      status: !listings.length ? 'empty' : (errors ? 'partial' : 'ok'),
+      notes: !listings.length ? 'parser found no <article class=entidad> blocks; ICT page likely AJAX-rendered, switch to Browserbase' : (errors ? `listing errors=${errors}` : '')
     });
 
-    console.log(`[ict-cst] in=${listings.length} added=${inserted} updated=${updated}`);
+    console.log(`[ict-cst] in=${listings.length} added=${inserted} updated=${updated} errors=${errors}`);
   } catch (e) {
     await finishRun(runId, { status: 'error', notes: e.message });
     console.error('[ict-cst] FAIL', e.message);
diff --git a/scripts/ingest/meic-pymes.js b/scripts/ingest/meic-pymes.js
index 4665a8b..c23a156 100644
--- a/scripts/ingest/meic-pymes.js
+++ b/scripts/ingest/meic-pymes.js
@@ -36,7 +36,7 @@ function findCol(headers, ...needles) {
 
 (async () => {
   const runId = await startRun('meic-pymes', 'Fetch MEIC PYMEs Activas via CKAN + parse XLSX');
-  let added = 0, updated = 0, total = 0;
+  let added = 0, updated = 0, total = 0, errors = 0;
   try {
     fs.mkdirSync(CACHE_DIR, { recursive: true });
 
@@ -101,6 +101,10 @@ function findCol(headers, ...needles) {
     const dataRows = rows.slice(headerIdx + 1).filter(r => Array.isArray(r) && r[iName]);
 
     for (const r of dataRows.slice(0, ROW_CAP)) {
+     // Per-row isolation: a single bad row (DB CHECK violation, weird cell) must
+     // skip + count, not abort the remaining ~14,500 rows via the outer catch.
+     // (Cody ingest audit, cycle 24 — mirrors google-places.js/local-portals.js.)
+     try {
       const name = String(r[iName] || '').trim(); if (!name) continue;
       const canton   = String(r[iCanton] || '').trim();
       const province = String(r[iProv] || '').trim();
@@ -138,10 +142,14 @@ function findCol(headers, ...needles) {
       if (upsertResult.inserted) added++; else updated++;
       total++;
       if (total % 1000 === 0) console.log(`[meic-pymes] progress: ${total}`);
+     } catch (e) {
+      errors++;
+      if (errors <= 20) console.warn(`[meic-pymes] row skipped (${String(r[iName]||'').slice(0,40)}): ${e.message}`);
+     }
     }
 
-    await finishRun(runId, { rows_in: total, rows_added: added, rows_updated: updated, status: 'ok' });
-    console.log(`[meic-pymes] DONE in=${total} added=${added} updated=${updated}`);
+    await finishRun(runId, { rows_in: total, rows_added: added, rows_updated: updated, status: errors ? 'partial' : 'ok', notes: errors ? `row errors=${errors}` : '' });
+    console.log(`[meic-pymes] DONE in=${total} added=${added} updated=${updated} errors=${errors}`);
   } catch (e) {
     await finishRun(runId, { status: 'error', notes: e.message });
     console.error('[meic-pymes] FAIL', e.message);
diff --git a/test/ingest-resilience.test.js b/test/ingest-resilience.test.js
new file mode 100644
index 0000000..9621cbb
--- /dev/null
+++ b/test/ingest-resilience.test.js
@@ -0,0 +1,88 @@
+'use strict';
+// Ingest-layer resilience (Cody ingest audit, cycle 24):
+//   (A) fetchText/fetchJson/fetchBuffer must time out instead of hanging forever
+//       on a stalled gov site — behavioral test with a fake, never-resolving fetch.
+//   (B) meic-pymes.js + ict-cst.js must isolate each record so one poison row
+//       doesn't abort the whole run — structural regression guard on the source.
+
+const { test, after } = require('node:test');
+const assert = require('node:assert');
+const fs = require('node:fs');
+const path = require('node:path');
+
+const lib = require('../scripts/ingest/_lib');
+const realFetch = global.fetch;
+after(async () => { global.fetch = realFetch; try { await lib.pool.end(); } catch {} });
+
+// (A) ---- fetch timeout behavior -------------------------------------------------
+
+test('fetchText rejects (does not hang) when the server never responds — default-signal timeout', async () => {
+  // A fetch that resolves ONLY when its abort signal fires => simulates a hung host.
+  global.fetch = (_url, o) => new Promise((_res, rej) => {
+    if (o.signal.aborted) return rej(o.signal.reason);
+    o.signal.addEventListener('abort', () => rej(o.signal.reason));
+  });
+  const t0 = Date.now();
+  await assert.rejects(
+    () => lib.fetchText('http://stalled.example/x', { timeoutMs: 40 }),
+    (e) => { assert.equal(e.name, 'TimeoutError', 'a hung fetch must abort with a TimeoutError'); return true; });
+  assert.ok(Date.now() - t0 < 2000, 'it must give up in ~40ms, not hang');
+});
+
+test('fetchJson honors a CALLER-supplied signal instead of imposing its own timeout', async () => {
+  global.fetch = (_url, o) => new Promise((_res, rej) => {
+    o.signal.addEventListener('abort', () => rej(o.signal.reason || new Error('aborted')));
+  });
+  const ctrl = new AbortController();
+  const p = lib.fetchJson('http://x.example/j', { signal: ctrl.signal });
+  ctrl.abort(new Error('caller-aborted'));
+  await assert.rejects(() => p, /caller-aborted/, 'the caller-owned signal must be the one in effect');
+});
+
+test('fetchText still returns the body on a fast, healthy response', async () => {
+  global.fetch = async () => ({ ok: true, status: 200, text: async () => 'hola' });
+  assert.equal(await lib.fetchText('http://ok.example/'), 'hola');
+});
+
+// Regression: fetchBuffer(url) with NO opts previously always resolved its timeout
+// to a hardcoded 60000 literal via `Number(opts.timeoutMs) || fallbackMs || DEFAULT`
+// (undefined -> NaN -> falsy -> the truthy fallbackMs literal always won), so
+// SCRAPER_TIMEOUT_MS was silently a no-op for the multi-MB MEIC XLSX download —
+// the one fetch that most needs an env-tunable timeout. (Cody gate, cycle 25.)
+test('fetchBuffer honors its OWN env knob (SCRAPER_BUFFER_TIMEOUT_MS), independent of SCRAPER_TIMEOUT_MS', async () => {
+  process.env.SCRAPER_TIMEOUT_MS = '5000';        // deliberately large/irrelevant
+  process.env.SCRAPER_BUFFER_TIMEOUT_MS = '35';    // must be the one that governs
+  delete require.cache[require.resolve('../scripts/ingest/_lib')];
+  const freshLib = require('../scripts/ingest/_lib');
+  global.fetch = (_url, o) => new Promise((_res, rej) => {
+    o.signal.addEventListener('abort', () => rej(o.signal.reason));
+  });
+  const t0 = Date.now();
+  await assert.rejects(() => freshLib.fetchBuffer('http://big.example/x.xlsx'), /./);
+  const elapsed = Date.now() - t0;
+  assert.ok(elapsed < 2000, `fetchBuffer must respect SCRAPER_BUFFER_TIMEOUT_MS=35, not the 60000 default or the unrelated 5000 (took ${elapsed}ms)`);
+  delete process.env.SCRAPER_TIMEOUT_MS;
+  delete process.env.SCRAPER_BUFFER_TIMEOUT_MS;
+  await freshLib.pool.end(); // re-requiring _lib created a SECOND pg Pool; close it too
+  delete require.cache[require.resolve('../scripts/ingest/_lib')];
+});
+
+// (B) ---- per-record isolation (structural regression guard) ---------------------
+
+function src(rel) { return fs.readFileSync(path.join(__dirname, '..', rel), 'utf8'); }
+
+test('meic-pymes.js isolates each row so one poison record cannot abort the run', () => {
+  const s = src('scripts/ingest/meic-pymes.js');
+  // The per-row catch's warn string only exists if the row body is wrapped; it +
+  // errors++ + the partial status together prove a skipped row is counted, not fatal.
+  assert.match(s, /\[meic-pymes\] row skipped/, 'per-row catch (with its skip log) must exist');
+  assert.match(s, /errors\+\+/, 'a skipped row must be counted, not fatal');
+  assert.match(s, /status: errors \? 'partial' : 'ok'/, 'errors must downgrade to partial, not fail the whole run');
+});
+
+test('ict-cst.js isolates each listing so one bad card cannot abort the run', () => {
+  const s = src('scripts/ingest/ict-cst.js');
+  assert.match(s, /\[ict-cst\] listing skipped/, 'per-listing catch (with its skip log) must exist');
+  assert.match(s, /errors\+\+/, 'a skipped listing must be counted, not fatal');
+  assert.match(s, /errors \? 'partial' : 'ok'/, 'errors must downgrade to partial');
+});

← 6564e86 cycle 24 docs: YOLO_NOTES ledger — stored-XSS fix on place.h  ·  back to Costa Rica  ·  cycle 25 docs: YOLO_NOTES ledger — ingest fetch timeout + pe 86389af →