Skip to content

fix(etl): detect wrangler's SQLITE errors from stdout in safeD1 - #277

Merged
todorkolev merged 1 commit into
mainfrom
fix/safed1-stdout-detection
Aug 8, 2026
Merged

fix(etl): detect wrangler's SQLITE errors from stdout in safeD1#277
todorkolev merged 1 commit into
mainfrom
fix/safed1-stdout-detection

Conversation

@todorkolev

@todorkolev todorkolev commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Изваден от #188 на @StanislavBG. Кодът е негов, дословно, и коммитът пази авторството му, тъй че при squash заслугата остава при него.

Защо отделно

#188 („индекс на качеството на договорите") е голям feature PR - миграции, derive стъпки, две нови страници, 40+ коммита. Тази поправка е случайно попаднала вътре: два реда в scripts/import.mjs, които нямат нищо общо с индекса на качеството. Ако остане там, тръгва чак когато цялата функционалност мине ревю, а дотогава --plan-only стои счупен. Затова я разделяме - поправката да върви със своя скорост, а #188 да се преглежда по същество.

Няма нужда @StanislavBG да прави нещо. Когато това се слее, хънкът в #188 или ще се разреши сам при rebase, или ще е тривиален конфликт с еднакъв резултат.

Проблемът

safeD1 търсеше „no such table" в err.message. Но wrangler печата грешката от SQLite като JSON на stdout, а изключението остава голо Command failed: wrangler d1 execute .... Проверката така никога не хващаше, safeD1 пре-хвърляше вместо да върне [], и fallback-ът към data_freshness в latestLoadedDate беше недостижим.

Видимият ефект: pnpm run import --catchup --plan-only гърми, когато transient staging таблиците ги няма - а това е нормалното им състояние, понеже drop-transient-staging ги маха в finally, а --plan-only излиза преди основният поток да ги пресъздаде. Обикновените пускания не са засегнати: те първо създават празен raw_contracts и стигат до fallback-а.

Проверка

Срещу истинска локална база, след като transient staging таблиците са махнати (тоест в нормалното steady-state състояние):

Преди - --plan-only гърми:

Error: Command failed: wrangler d1 execute sigma --local --json --command ...
    "text": "no such table: raw_contracts: SQLITE_ERROR"

След - fallback-ът работи и планът се отпечатва, exit 0:

==> catchup plan maxLoadedDate=2026-07-27 from=2026-07-24 to=2026-07-28 gapDays=5 derive=slice

prettier --check е чист.

За теста

Първата редакция тук казваше, че тест няма как да се направи: import.mjs изпълнява код при внасяне, тъй че safeD1 не се тества модулно. Това вече не е вярно - #270 донесе scripts/import-guard.test.mjs, който кара истинския import.mjs като отделен процес с подставен wrangler най-отпред в PATH. Същата сглобка става и тук.

Тестът обаче нарочно не влиза в този PR: коммитът е един и е на @StanislavBG, тъй че при squash заслугата остава при него. Втори коммит от мен би я прехвърлил. Тестът идва с отделен PR.

Затова поправката е проверена с изпълнение, в двете посоки, със същата сглобка. Подставеният wrangler връща грешката така, както я връща истинският - JSON на stdout, голо съобщение на изключението:

Без поправката --catchup --plan-only гърми:

    "text": "no such table: raw_contracts: SQLITE_ERROR"
Node.js v22.22.3

С поправката fallback-ът към data_freshness се достига и планът се отпечатва:

==> catchup plan maxLoadedDate=2026-07-27 from=2026-07-24 to=2026-08-08 gapDays=16 derive=slice

Клонът е пребазиран върху main (а не слят), за да остане един коммит и авторството да се запази при squash.

@lyubomir-bozhinov

Copy link
Copy Markdown
Collaborator

Чист split — двата реда нямат нищо общо с индекса на качеството и логично вървят отделно (авторството на @StanislavBG е запазено в комита). Поправката е коректна: wrangler пише SQLITE грешката в stdout, не в err.message, тъй че старият String(err?.message ?? err) не хващаше „no such table" и safeD1 хвърляше вместо да върне [] — чупейки --plan-only. Новото конкатенира message + stdout + stderr преди match-а.

Ортогонално на #270 (не е зависимост по ред на merge): probe-ът върху contracts там се вика чак след wrangler d1 migrations apply, тъй че таблицата вече съществува; тази поправка е за пътищата, където таблицата още я няма (напр. --plan-only) — точно каквото safeD1 пази. Одобрявам.

safeD1 tested err.message for "no such table", but wrangler prints the
SQLite error as JSON on stdout and leaves the exception message as a bare
"Command failed: wrangler d1 execute ...". The guard therefore never
matched, safeD1 rethrew instead of returning [], and the data_freshness
fallback in latestLoadedDate was unreachable.

The visible effect is that `pnpm run import --catchup --plan-only` crashes
whenever the transient staging tables are absent, which is their normal
steady state: drop-transient-staging removes them in a finally, and
--plan-only exits before the main flow recreates them. Ordinary runs are
unaffected, since they create an empty raw_contracts first and so reach the
fallback.

Cherry-picked verbatim from e3fc6b3 in #188 by Bilko (StanislavBG), split
out so the fix can land without the contract health index feature.
@todorkolev
todorkolev force-pushed the fix/safed1-stdout-detection branch from fdc7169 to 872953a Compare August 8, 2026 00:26
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown

Test coverage

Workspace Lines Δ Branches Δ Functions Statements
apps/etl 75.43% +1.43pp 63.52% +5.32pp 70.00% 74.11%
apps/web 91.02% +1.32pp 82.50% +0.70pp 91.19% 89.73%
packages/config 92.85% +0.05pp 72.22% +0.02pp 92.85% 89.18%
packages/db 94.54% +0.34pp 79.38% +0.38pp 87.19% 91.55%
packages/ingest 88.17% +2.37pp 82.30% +2.30pp 80.17% 86.36%
packages/shared 95.50% +0.10pp 80.83% +0.83pp 92.30% 89.56%
Total (informational) 91.23% 80.72% 86.95% 89.08%

✅ No workspace dropped below its baseline (tolerance 0.5pp).

📈 Coverage rose by more than 1pp — run node scripts/check-coverage.mjs --update locally and commit coverage-baseline.json to ratchet the threshold up.

@todorkolev
todorkolev merged commit 8db751d into main Aug 8, 2026
5 checks passed
@todorkolev

Copy link
Copy Markdown
Collaborator Author

Кодът тук е на @StanislavBG - двата реда идват дословно от #188 и коммитът в клона беше негов. При squash обаче GitHub записа автор мен, не него: 8db751d е Todor Kolev <tkolev@obecto.com>.

Това е моя грешка в преценката. Смятах, че щом PR-ът е с един-единствен коммит, авторството оцелява при squash - не оцелява. Авторът на PR-а печели винаги, независимо от броя коммити. Единственото, което пази заслугата, е мърджване през PR, отворен от самия автор; трябваше да върна поправката в неговия PR, а не да я вадя в свой.

Заслугата за поправката е негова. Записвам го и тук, и в бележките към изданието, понеже пренаписване на main за това не си струва.

Отделно: обяснението в описанието, че тест няма как да се направи, вече не важи - #270 донесе scripts/import-guard.test.mjs, който кара истинския import.mjs като отделен процес с подставен wrangler. Поправката е проверена с изпълнение в двете посоки през тази сглобка, а тестът идва с отделен PR.

todorkolev added a commit that referenced this pull request Aug 9, 2026
Поправката от #277 нямаше нищо, което да я пази: `import.mjs` изпълнява код
при внасяне, тъй че `safeD1` не се тества модулно. Сглобката от #270 обаче
кара истинския скрипт като отделен процес с подставен `wrangler`, и това е
достатъчно.

Подставката е сверена срещу истинския двоичен файл, а не измислена - при
`d1 execute --json` върху липсваща таблица грешката от SQLite излиза на
stdout, известията на stderr, а съобщението на изключението е само
„Command failed". Точно това разминаване изпускаше старата проверка. Наблюдаваният
изход е записан в теста.

Мутациите: връщането на проверката само върху `err.message` вали два теста,
а превръщането на safeD1 в общ капан вали третия.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants