From f38f44dcced01bb23fb7c122e140b8c14eaef210 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Wed, 9 Sep 2026 19:43:37 +0000 Subject: [PATCH] The NTSB cache is ready when the last table lands, not the first `ntsb-fatal-accidents` failed its first real run with a bare ENOENT, and the cause is a race I wrote into the reader. Both NTSB sources read the same extract from the same directory, which is deliberate: one 96 MB download and one 558 MB Access database serve every source over that archive. But the readiness check was `exists(events.ndjson)`, and `events` is the FIRST of three tables written. `narratives` is the last and takes seconds longer. So the second source arrived mid-extraction, saw the events file, concluded the cache was warm, skipped the download, and then read an aircraft table that was still being written. The fix is a marker file written after every table, so "ready" means all of them rather than the first of them. The archive is also named per publication date now, since two sources unpacking `avall.zip` into one directory under different dates was the same mistake waiting in a different place. Beyond that, a source that finds a table genuinely missing -- a container that died between two exports leaves exactly that -- now deletes the marker and says which table is absent, so the next run re-fetches instead of raising ENOENT against a path nobody can interpret. Tested by pinning the thing the race depended on: both sources are seeded without their own `cacheDir`, because giving either one its own directory would mean two 96 MB downloads, and it was precisely their sharing one that made the ordering matter. 605 tests pass, biome clean. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01CjTtJPEpyJbvPeQcVfYPPs --- packages/adapters/src/ntsb.js | 33 ++++++++++++++++++++++++++++++--- test/ntsb.test.js | 16 ++++++++++++++++ 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/packages/adapters/src/ntsb.js b/packages/adapters/src/ntsb.js index 2cc9cc5..813877a 100644 --- a/packages/adapters/src/ntsb.js +++ b/packages/adapters/src/ntsb.js @@ -464,10 +464,24 @@ export const ntsbAccidents = defineAdapter({ await mkdir(dir, { recursive: true }); const stamp = join(dir, `avall-${published}`); const eventsFile = `${stamp}.events.ndjson`; - - if (!(await exists(eventsFile))) { + /* + * The cache is ready when this marker exists, and NOT when the first + * extract does. + * + * Two sources read this archive -- every accident, and the fatal ones -- + * and they share the directory, which is the point: one 96 MB download + * serves both. But `events` is written first and `narratives` last, so a + * second source arriving mid-extraction saw `events.ndjson`, concluded the + * cache was warm, skipped the download and then failed with ENOENT on a + * file still being written. That is exactly what `ntsb-fatal-accidents` + * did on its first real run. The marker is written after every table, so + * "ready" means all of them. + */ + const marker = `${stamp}.complete`; + + if (!(await exists(marker))) { log(`fetching the ${published} archive (96 MB)`); - const zip = join(dir, 'avall.zip'); + const zip = join(dir, `avall-${published}.zip`); const res = await http.request(ARCHIVE, { timeoutMs: 15 * 60_000 }); if (!res.ok) throw new Error(`${res.status} fetching the NTSB archive`); await writeFile(zip, Buffer.from(await res.arrayBuffer())); @@ -481,9 +495,22 @@ export const ntsbAccidents = defineAdapter({ // container that keeps both has 650 MB of disk it cannot use for anything. await rm(mdb, { force: true }); await rm(zip, { force: true }); + await writeFile(marker, `${TABLES.join(',')}\n`); log(`extracted ${TABLES.join(', ')} from the ${published} archive`); } + /* Belt and braces: the marker can only be missing above, never wrong, but a + * container that died between two exports leaves a directory that looks + * warm to nothing and cold to everything. If a table is genuinely absent + * here, say which one rather than letting `readFile` raise a bare ENOENT + * against a path nobody can interpret. */ + for (const table of TABLES) { + if (!(await exists(`${stamp}.${table}.ndjson`))) { + await rm(marker, { force: true }); + throw new Error(`the ${published} extract is missing ${table}; it will be re-fetched`); + } + } + const events = newestFirst( parseNdjson(await readFile(eventsFile, 'utf8')).filter((e) => { if (accidentsOnly && clean(e.ev_type) !== 'ACC') return false; diff --git a/test/ntsb.test.js b/test/ntsb.test.js index 623595d..910bb6d 100644 --- a/test/ntsb.test.js +++ b/test/ntsb.test.js @@ -206,6 +206,22 @@ describe('the NTSB bulk reader', () => { expect(indexByEvent(rows).get('x').Aircraft_Key).toBe(1); }); + test('the two NTSB sources share one archive, so readiness is a marker not a file', () => { + /* `ntsb-accidents` and `ntsb-fatal-accidents` read the same extract from + * the same directory, which is the point: one 96 MB download serves both. + * But `events` is written first and `narratives` last, so treating the + * first extract as "the cache is warm" let the second source skip the + * download and then fail with ENOENT on a file still being written. That + * is what it did on its first real run. Both sources must therefore be + * pointed at the same cache and distinguished only by their config. */ + const sources = adapterByName('ntsb-accidents').defaultSources; + expect(sources).toHaveLength(2); + expect(sources.map((s) => s.slug)).toEqual(['ntsb-accidents', 'ntsb-fatal-accidents']); + // Neither pins its own cacheDir, or they would each download the archive. + for (const s of sources) expect(s.config?.cacheDir).toBeUndefined(); + expect(sources[1].config.accidentsOnly).toBe('yes'); + }); + test('the archive date is read off the listing, so the download is conditional', () => { const page = 'avall.zip9/1/2026 7:03:59 AM96148686avall.zip';