From 2903374b72a1b16d82eb87acac7233a1db3a3c5e Mon Sep 17 00:00:00 2001 From: Matysh Date: Tue, 1 Sep 2026 19:08:07 +0300 Subject: [PATCH] fix: the exception guard reads its counter after delivery, not before MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Гард «uncaught exception внутри карточки» жил в demo/serve.mjs с 2026-07-27 и не срабатывал ни разу в самом частом случае. Счётчик читался синхронно, а Playwright доставляет pageerror асинхронно по CDP: если исключение возникло после последнего обращения смока к странице, счётчик к моменту проверки нулевой, а browser.close() уносит недоставленное событие. В логе это видно дословно — EXC печатается после результата и до OK. finish() теперь делает round-trip по открытым страницам перед чтением счётчика. Страницы регистрируются там, где создаются: ссылок на них у finish(browser, out) нет, а менять сигнатуру нельзя — так её зовут 205 смоков. Medium-1 жёлтого ревью ТЗ закрыт расширением, а не оговоркой. Страницы, созданные смоком после launch(), регистрация в launchInternal не покрывает: smoke_zoom_flash печатал своё EXC2 мимо счётчика, три страницы smoke_svg_sandbox не имели слушателя вовсе. Документировать слепую зону в задаче, которая существует ради устранения слепой зоны, значит закрыть issue, оставив дефект. Наружу отдана одна функция watchPage(page): подписка и регистрация неразделимы, иначе появится страница, чьи исключения считаются, а доставки не ждёт никто. Разрыв оказался шире, чем в ревью: проверка по всему набору нашла ещё два файла со своей подпиской — smoke_cold_view_toggle и smoke_cold_view_vacuum. Они не слепая зона, их страница приходит из launchColdView и уже зарегистрирована, а свой счётчик они превращают в отдельное утверждение. Поэтому инвариант сформулирован как «ни одна страница не создаётся мимо гарда» и закреплён по всему набору, а не по двум названным файлам. reportPageErrors() из #407 стал асинхронным: второй читатель счётчика обязан ждать доставку так же, как finish(). Пять смоков получили await. Фикстура smoke_danger_confirmation приведена к объявленному типу: без binding и bindingMode _bindingHasHaPage падал на undefined.split(':') — два исключения, которых гард не видел. Дефекта поведения нет, все 15 мест в src/, создающих диалог, binding пишут; врала фикстура. Два отступления от ТЗ, каждое по измеренной причине. Пробы лежат в demo/guard/, а не demo/fixtures/: последний входит в корпус sourceFingerprint, и каждый файл там объявил бы устаревшими бандл, скриншот-индекс и golden-индекс — пробы же не касаются ни одного пикселя. Поведение доказывается в job со браузером, а не в npm test: job «Фронтенд» браузеры не ставит, и тест молча скипался бы — тот самый тихий успех, против которого вся задача. Issue: #404 User-Visible: no --- .github/workflows/validate.yml | 8 + demo/guard/README.md | 18 ++ demo/guard/guard_closed_page.mjs | 10 + demo/guard/guard_tail_exception.mjs | 14 ++ demo/guard/guard_tail_rejection.mjs | 8 + demo/guard/verify-guard.mjs | 66 +++++++ demo/serve.mjs | 57 +++++- demo/smoke_danger_confirmation.mjs | 9 +- demo/smoke_deeplink.mjs | 2 +- demo/smoke_glow_blending.mjs | 2 +- demo/smoke_icon_center.mjs | 2 +- demo/smoke_long_press_gesture.mjs | 2 +- demo/smoke_space_card.mjs | 2 +- demo/smoke_svg_sandbox.mjs | 11 +- demo/smoke_zoom_flash.mjs | 7 +- docs/specs/404-smoke-exception-guard.md | 246 ++++++++++++++++++++++++ scripts/mutation-gate.mjs | 23 +++ test/smoke-exception-guard.test.mjs | 117 +++++++++++ test/smoke-harness-contract.test.mjs | 7 +- 19 files changed, 592 insertions(+), 19 deletions(-) create mode 100644 demo/guard/README.md create mode 100644 demo/guard/guard_closed_page.mjs create mode 100644 demo/guard/guard_tail_exception.mjs create mode 100644 demo/guard/guard_tail_rejection.mjs create mode 100644 demo/guard/verify-guard.mjs create mode 100644 docs/specs/404-smoke-exception-guard.md create mode 100644 test/smoke-exception-guard.test.mjs diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index cd1cfba8..80c27049 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -532,6 +532,14 @@ jobs: path: test-build - name: Разложить бандл по копиям run: node scripts/bundle-sync.mjs + # Гард исключений внутри карточки объявлен в demo/serve.mjs с 2026-07-27 и + # до #404 не срабатывал ни разу: счётчик читался раньше, чем Playwright + # доставлял pageerror. Такое ловится только запуском, поэтому пробы живут + # здесь — в единственной job с настоящим браузером. Один шард из трёх: + # проверка не зависит от разбиения, а платить за неё трижды незачем. + - name: "Гард исключений умеет падать (#404)" + if: matrix.shard == 1 + run: node demo/guard/verify-guard.mjs - name: Smoke suite (шард ${{ matrix.shard }} из 3) env: SHARD: ${{ matrix.shard }} diff --git a/demo/guard/README.md b/demo/guard/README.md new file mode 100644 index 00000000..69ecd26e --- /dev/null +++ b/demo/guard/README.md @@ -0,0 +1,18 @@ +# Пробы гарда исключений (#404) + +Здесь лежат фикстуры, которые **должны падать**: каждая проверяет, что гард +«uncaught exception внутри карточки» умеет краснеть, а не только объявлен. + +Три причины, по которым это отдельный каталог, а не `demo/smoke_*` и не +`demo/fixtures/`: + +1. **Не `smoke_*`** — иначе шарды CI будут гонять заведомо красный файл и + покрасят себя. +2. **Не `demo/fixtures/`** — этот каталог входит в корпус `sourceFingerprint` + (`scripts/source-fingerprint.mjs`), то есть каждый новый `.mjs` там объявляет + устаревшими закоммиченный бандл, скриншот-индекс документации и golden-индекс. + Пробы гарда ни одного пикселя не касаются, платить пересъёмкой за них нечем. +3. **Каталог, а не файл** — проб три, и они читаются как набор. + +Запускает их `verify-guard.mjs`; он же вызывается из job «Смоки в браузере» +и служит guard'ом двух мутантов в `scripts/mutation-gate.mjs`. diff --git a/demo/guard/guard_closed_page.mjs b/demo/guard/guard_closed_page.mjs new file mode 100644 index 00000000..a27db314 --- /dev/null +++ b/demo/guard/guard_closed_page.mjs @@ -0,0 +1,10 @@ +// Проба #404 AC8: закрытая страница не ломает вердикт. +// +// Round-trip к закрытой странице бросает, и если бы гард этого не проглатывал, +// починка одного дефекта завела бы другой — падение вердикта вместо вердикта. +// Здесь исключений нет вовсе, поэтому файл обязан завершаться НУЛЁМ. +import { launch, finish } from '../serve.mjs'; + +const { browser, page } = await launch(); +await page.close(); +await finish(browser, { probe: 'closed page' }); diff --git a/demo/guard/guard_tail_exception.mjs b/demo/guard/guard_tail_exception.mjs new file mode 100644 index 00000000..22f20c52 --- /dev/null +++ b/demo/guard/guard_tail_exception.mjs @@ -0,0 +1,14 @@ +// Проба #404 AC1: исключение в самом хвосте, без обращений к странице после. +// +// Это единственная слепая зона, из-за которой заводился #404: смок всё измерил, +// печатает результат, а карточка в это время бросает. Событие `pageerror` +// приходит асинхронно, и до починки счётчик читался раньше доставки. +// +// Файл ОБЯЗАН завершаться ненулевым кодом. Ноль означает, что гард снова слеп. +import { launch, finish } from '../serve.mjs'; + +const { browser, page } = await launch(); +await page.evaluate(() => { setTimeout(() => { throw new Error('guard-tail-exception'); }, 0); }); +// Ни одного обращения к странице после этой строки: round-trip обязан сделать +// сам гард, иначе доказательство ничего не доказывает. +await finish(browser, { probe: 'tail exception' }); diff --git a/demo/guard/guard_tail_rejection.mjs b/demo/guard/guard_tail_rejection.mjs new file mode 100644 index 00000000..c0cf7c4d --- /dev/null +++ b/demo/guard/guard_tail_rejection.mjs @@ -0,0 +1,8 @@ +// Проба #404 AC3: необработанное отклонение промиса считается так же, как +// исключение. Связь с #405 (оброненный промис в удалении черновика) держится +// именно этим: там дефект приходит в карточку в виде rejection, а не throw. +import { launch, finish } from '../serve.mjs'; + +const { browser, page } = await launch(); +await page.evaluate(() => { setTimeout(() => { Promise.reject(new Error('guard-tail-rejection')); }, 0); }); +await finish(browser, { probe: 'tail rejection' }); diff --git a/demo/guard/verify-guard.mjs b/demo/guard/verify-guard.mjs new file mode 100644 index 00000000..7d679ab5 --- /dev/null +++ b/demo/guard/verify-guard.mjs @@ -0,0 +1,66 @@ +#!/usr/bin/env node +/** + * Отрицательный прогон гарда исключений (#404). + * + * Гард объявлен в `demo/serve.mjs` с 2026-07-27 и до этой задачи не срабатывал + * ни разу: счётчик читался раньше, чем Playwright доставлял `pageerror`. Такое + * ловится только запуском — «проверка, которая не умеет падать» выглядит + * идентично работающей во всём, кроме исхода. + * + * Почему не тест в `test/`: пробам нужен настоящий Chromium, а job «Фронтенд», + * где идёт `npm test`, браузеры не ставит. Тест там молча скипался бы — то есть + * ровно тот тихий успех, против которого всё это и делается. Поэтому проверка + * живёт в job «Смоки в браузере», где браузер есть, и вызывается один раз. + */ +import { spawnSync } from 'node:child_process'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const HERE = dirname(fileURLToPath(import.meta.url)); + +/** Каждая проба: чего ждём от кода возврата и что обязано быть в выводе. */ +const PROBES = [ + { + file: 'guard_tail_exception.mjs', + expectExit: 1, + expectOutput: /uncaught exception\(s\) inside the card/, + because: 'исключение в хвосте — та самая слепая зона, из-за которой заводился #404', + }, + { + file: 'guard_tail_rejection.mjs', + expectExit: 1, + expectOutput: /uncaught exception\(s\) inside the card/, + because: 'отклонение промиса приходит тем же каналом; на этом держится связь с #405', + }, + { + file: 'guard_closed_page.mjs', + expectExit: 0, + expectOutput: /OK/, + because: 'round-trip к закрытой странице не имеет права ронять вердикт', + }, +]; + +let failed = 0; +for (const probe of PROBES) { + const run = spawnSync(process.execPath, [resolve(HERE, probe.file)], { + encoding: 'utf8', cwd: resolve(HERE, '../..'), timeout: 90_000, + }); + const output = `${run.stdout || ''}${run.stderr || ''}`; + const exitOk = run.status === probe.expectExit; + const textOk = probe.expectOutput.test(output); + if (exitOk && textOk) { + console.log(`ok ${probe.file} → exit ${run.status}`); + continue; + } + failed += 1; + console.error(`FAIL ${probe.file}: ${probe.because}`); + console.error(` ожидался exit ${probe.expectExit}, получен ${run.status}`); + if (!textOk) console.error(` в выводе нет ${probe.expectOutput}`); + console.error(output.split('\n').slice(-12).map((line) => ` | ${line}`).join('\n')); +} + +if (failed) { + console.error(`\nгард исключений не доказан: проб провалено ${failed} из ${PROBES.length}`); + process.exit(1); +} +console.log(`\nгард исключений доказан на ${PROBES.length} пробах`); diff --git a/demo/serve.mjs b/demo/serve.mjs index 828629db..d4d80f40 100644 --- a/demo/serve.mjs +++ b/demo/serve.mjs @@ -17,6 +17,51 @@ const CT = { '.html': 'text/html', '.js': 'text/javascript', '.svg': 'image/svg+ // prints them and sets the exit code. const _failures = []; let _pageErrors = 0; +/** + * Открытые страницы — для round-trip'а перед чтением счётчика (#404). + * + * Ссылок на страницы у `finish(browser, out)` нет, а менять её сигнатуру + * нельзя: так её зовут 205 смоков. Поэтому страницы регистрируются там, где + * создаются. + */ +const _livePages = new Set(); + +/** + * События `pageerror` Playwright доставляет асинхронно по CDP, а гард читал + * счётчик синхронно (#404). Если исключение возникло после последнего обращения + * смока к странице, счётчик к моменту проверки ещё нулевой, а `browser.close()` + * уносит недоставленное событие. В логе это видно дословно: `EXC` печатается + * ПОСЛЕ результата и ДО `OK`. + * + * Круговой запрос к странице вытесняет ранее поставленные макрозадачи, поэтому + * всё, что страница успела произвести до этого момента, к нам уже дошло. + * + * Честная граница: исключение, возникшее ПОСЛЕ этого round-trip'а — например, в + * обработчике `beforeunload` при закрытии браузера, — не учитывается. Ловить его + * значит ждать неизвестно чего неизвестно сколько; контракт формулируется как + * «всё, что произошло до вызова вердикта». + */ +async function roundTripLivePages() { + for (const page of _livePages) { + try { await page.evaluate(() => 0); } catch { /* закрыта, упала или в навигации */ } + } +} + +/** + * Подписать страницу, созданную ВНЕ `launch()` (#404, Medium-1 ревью ТЗ). + * + * Таких мест два — `smoke_zoom_flash` открывает вторую страницу на своём + * контексте, `smoke_svg_sandbox` создаёт три. Их исключения не считал никто: + * первая печатала своё `EXC2` мимо счётчика, остальные три не имели слушателя + * вовсе. Регистрация в `launchInternal` их не покрывает по построению, поэтому + * гард отдаёт наружу ровно одну функцию — и её вызов виден в диффе смока. + */ +export function watchPage(page) { + page.on('pageerror', (e) => { _pageErrors++; console.log('EXC', e.stack || e.message); }); + _livePages.add(page); + page.on('close', () => _livePages.delete(page)); + return page; +} /** Assert one named fact. `expected` defaults to true. */ export function check(name, actual, expected = true) { @@ -50,7 +95,8 @@ export function checkAll(out, expected = {}) { * @returns true, если карточка бросала — чтобы вызывающий мог добавить своё * сообщение, не считая исключения заново. */ -export function reportPageErrors() { +export async function reportPageErrors() { + await roundTripLivePages(); if (!_pageErrors) return false; console.error(`FAILED: ${_pageErrors} uncaught exception(s) inside the card`); process.exitCode = 1; @@ -60,6 +106,9 @@ export function reportPageErrors() { /** Print the result, report failures, close the browser, set the exit code. */ export async function finish(browser, out) { if (out !== undefined) console.log(JSON.stringify(out, null, 1)); + // Порядок обязателен: сначала дать странице доставить события, потом читать + // счётчик (#404). Обратный порядок и был дефектом. + await roundTripLivePages(); if (_pageErrors) _failures.push(`${_pageErrors} uncaught exception(s) inside the card`); await browser?.close?.(); if (_failures.length) { @@ -84,8 +133,10 @@ async function launchInternal( const page = await (await browser.newContext({ viewport, deviceScaleFactor: scale, ...contextOptions, })).newPage(); - // audit T1: an exception inside the card used to be logged and ignored - page.on('pageerror', (e) => { _pageErrors++; console.log('EXC', e.stack || e.message); }); + // audit T1: an exception inside the card used to be logged and ignored. + // Подписка и регистрация — одной функцией: разъехавшись, они дали бы + // страницу, чьи исключения считаются, но доставки которых никто не ждёт (#404). + watchPage(page); await page.route('**/*', (r) => { const u = new URL(r.request().url()); let p = decodeURIComponent(u.pathname); diff --git a/demo/smoke_danger_confirmation.mjs b/demo/smoke_danger_confirmation.mjs index 2d9ee697..f45dfe9b 100644 --- a/demo/smoke_danger_confirmation.mjs +++ b/demo/smoke_danger_confirmation.mjs @@ -164,7 +164,14 @@ const out = await page.evaluate(async () => { markers: [{ id: 'danger-marker', binding: 'entity:switch.danger', space: 'danger-space' }], }; card._devices = []; - card._markerDialog = { devId: 'danger-marker', name: 'Danger marker', busy: false }; + // #404 AC5: объявленный тип требует binding и bindingMode, и без них + // _bindingHasHaPage падал на undefined.split(':') — два необработанных + // исключения, которых гард не видел. Дефекта поведения тут нет: все 15 мест + // в src/, создающих диалог, binding пишут; врала фикстура. + card._markerDialog = { + devId: 'danger-marker', name: 'Danger marker', busy: false, + binding: 'entity:switch.danger', bindingMode: 'ha', bindingOpen: false, + }; decision = false; await editor._deleteMarker(); const markerCancel = card._serverCfg.markers.length === 1 && configSaves === 0; diff --git a/demo/smoke_deeplink.mjs b/demo/smoke_deeplink.mjs index 9f6fcaa9..5c38e0c3 100644 --- a/demo/smoke_deeplink.mjs +++ b/demo/smoke_deeplink.mjs @@ -24,6 +24,6 @@ const ok = res.after === res.target && res.afterBad === res.target; console.log(JSON.stringify(res)); // #407: своя развязка про исключения в карточке не спрашивает. Вердикт обязан // именно остановить: иначе строка успеха печатается после «FAILED». -if (reportPageErrors()) process.exit(1); +if (await reportPageErrors()) process.exit(1); if (!ok) { console.error('FAIL deeplink smoke'); process.exit(1); } console.log('OK deep-link: full card switches to #space=, ignores invalid ids'); diff --git a/demo/smoke_glow_blending.mjs b/demo/smoke_glow_blending.mjs index 9c60aa96..5d8c3248 100644 --- a/demo/smoke_glow_blending.mjs +++ b/demo/smoke_glow_blending.mjs @@ -221,7 +221,7 @@ try { // #407: проверки выше бросают на своей регрессии, но про исключения внутри // карточки не спрашивает ни одна. Бросаем и здесь — тогда `finally` закроет // браузер, а строка успеха не напечатается после «FAILED». - if (reportPageErrors()) throw new Error('uncaught exception inside the card — see EXC above'); + if (await reportPageErrors()) throw new Error('uncaught exception inside the card — see EXC above'); console.log(JSON.stringify({ ok: true, blend: out.blend, pools: out.pools, staticParity: true, staticPools: out.staticPools })); } finally { await browser.close(); diff --git a/demo/smoke_icon_center.mjs b/demo/smoke_icon_center.mjs index e04ef69d..af377884 100644 --- a/demo/smoke_icon_center.mjs +++ b/demo/smoke_icon_center.mjs @@ -26,6 +26,6 @@ const bad = await page.evaluate(() => { await browser.close(); // #407: своя развязка про исключения в карточке не спрашивает. Вердикт обязан // именно остановить: иначе строка успеха печатается после «FAILED». -if (reportPageErrors()) process.exit(1); +if (await reportPageErrors()) process.exit(1); if (bad.length) { console.error('FAIL icon-center: badges off anchor', JSON.stringify(bad.slice(0, 5))); process.exit(1); } console.log('OK icon-center: all device badges centred on their anchor'); diff --git a/demo/smoke_long_press_gesture.mjs b/demo/smoke_long_press_gesture.mjs index f00b9d27..38ebe75c 100644 --- a/demo/smoke_long_press_gesture.mjs +++ b/demo/smoke_long_press_gesture.mjs @@ -66,7 +66,7 @@ try { // #407: проверки выше бросают на своей регрессии, но про исключения внутри // карточки не спрашивает ни одна. Бросаем и здесь — тогда `finally` закроет // браузер, а строка успеха не напечатается после «FAILED». - if (reportPageErrors()) throw new Error('uncaught exception inside the card — see EXC above'); + if (await reportPageErrors()) throw new Error('uncaught exception inside the card — see EXC above'); console.log(JSON.stringify({ ok: true, ...result })); } finally { await browser.close(); diff --git a/demo/smoke_space_card.mjs b/demo/smoke_space_card.mjs index bdd6db1f..c26ab649 100644 --- a/demo/smoke_space_card.mjs +++ b/demo/smoke_space_card.mjs @@ -245,6 +245,6 @@ const ok = console.log(JSON.stringify(res)); // #407: своя развязка про исключения в карточке не спрашивает. Вердикт обязан // именно остановить: иначе строка успеха печатается после «FAILED». -if (reportPageErrors()) process.exit(1); +if (await reportPageErrors()) process.exit(1); if (!ok) { console.error('FAIL space-card smoke'); process.exit(1); } console.log('OK space-card: live shared marker face, pointer-events:none, deep-link button, error card'); diff --git a/demo/smoke_svg_sandbox.mjs b/demo/smoke_svg_sandbox.mjs index cd2e881e..e46fc08e 100644 --- a/demo/smoke_svg_sandbox.mjs +++ b/demo/smoke_svg_sandbox.mjs @@ -5,7 +5,7 @@ // localStorage сессии и к API. Проверяем, что заголовок sandbox это снимает, // и что обычный SVG при этом продолжает отображаться. import { chromium } from 'playwright'; -import { check, finish } from './serve.mjs'; +import { check, finish, watchPage } from './serve.mjs'; const EVIL = ` @@ -34,8 +34,11 @@ async function serve(page, { csp }) { }); } +// #404: три страницы этого смока не имели слушателя pageerror вовсе — они +// оставались слепой зоной и после починки гарда. Общий гард подписывает их и +// ждёт доставки перед вердиктом. // 1) как было до фикса: скрипт исполняется в origin Home Assistant -const before = await ctx.newPage(); +const before = watchPage(await ctx.newPage()); await serve(before, { csp: false }); await before.goto('https://ha.example/api/houseplan/content/plans/_/evil.svg'); await before.waitForTimeout(200); @@ -45,7 +48,7 @@ const noCsp = await before.evaluate(() => ({ })); // 2) с заголовком: opaque origin, скрипт не выполняется, storage недоступен -const after = await ctx.newPage(); +const after = watchPage(await ctx.newPage()); await serve(after, { csp: true }); await after.goto('https://ha.example/api/houseplan/content/plans/_/evil.svg'); await after.waitForTimeout(200); @@ -55,7 +58,7 @@ const withCsp = await after.evaluate(() => ({ })); // 3) тот же файл как внутри страницы — рисуется и без скрипта -const card = await ctx.newPage(); +const card = watchPage(await ctx.newPage()); await serve(card, { csp: true }); await card.goto('https://ha.example/'); const drawn = await card.evaluate(async () => { diff --git a/demo/smoke_zoom_flash.mjs b/demo/smoke_zoom_flash.mjs index 237e4d17..2dd676c4 100644 --- a/demo/smoke_zoom_flash.mjs +++ b/demo/smoke_zoom_flash.mjs @@ -5,7 +5,7 @@ // websocket round-trip. Every rAF frame from the first possible moment is // sampled: a frame where the stage is visible AND the viewBox is at the // default scale is the flash. -import { launch, check, finish } from './serve.mjs'; +import { launch, watchPage, check, finish } from './serve.mjs'; import { readFileSync, existsSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; import { dirname } from 'node:path'; @@ -87,8 +87,9 @@ await ctx.addInitScript(([cache, zoom]) => { }; }, [cfgCache, ZOOM]); -const p2 = await ctx.newPage(); -p2.on('pageerror', (e) => console.log('EXC2', e.message)); +// #404: своя подписка печатала EXC2 мимо счётчика — страница считалась +// проверенной, а её исключения не видел никто. Теперь через общий гард. +const p2 = watchPage(await ctx.newPage()); await p2.goto('http://demo.local/demo.html', { waitUntil: 'domcontentloaded' }); await p2.waitForFunction(() => window.__framesDone === true, { timeout: 15000 }); diff --git a/docs/specs/404-smoke-exception-guard.md b/docs/specs/404-smoke-exception-guard.md new file mode 100644 index 00000000..a2f576a1 --- /dev/null +++ b/docs/specs/404-smoke-exception-guard.md @@ -0,0 +1,246 @@ +# ТЗ #404 — Гард «uncaught exception внутри карточки» перестаёт быть слепым к хвосту смока + +- Issue: https://github.com/Matysh/houseplan-card/issues/404 +- Приоритет: P2, tests + infra; полный трек — прецеденты #398 и #399 (обе + инфраструктурные, обе прошли с файлом ТЗ). Класс файлов — только B + (`demo/**`, `test/**`), ни одного файла класса A +- Ревизия: 2 (2026-09-01) — по жёлтому вердикту SPEC-REVIEW-404-r1 + +## Ревизия 2: что изменилось после жёлтого ревью + +**Medium-1 (в скоупе, возврат автору) закрыт расширением, а не оговоркой.** +Ревью нашло разрыв: контракт «страницы регистрируются там, где создаются, — в +`launchInternal`» не покрывает страницы, созданные смоком после `launch()`. +Выбор был между «расширить регистрацию» и «назвать разрыв второй границей». +Взят первый: документировать слепую зону в задаче, которая существует ради +устранения слепой зоны, — значит закрыть issue, оставив дефект. + +Гард отдаёт наружу одну функцию, `watchPage(page)`: она подписывает на +`pageerror` и регистрирует страницу для round-trip'а. Двумя половинами это быть +не может — разъехавшись, они дадут страницу, чьи исключения считаются, но +доставки которых никто не ждёт. + +**Разрыв оказался шире, чем в ревью.** Ревью назвало два файла +(`smoke_zoom_flash`, `smoke_svg_sandbox`); проверка по всему набору нашла ещё +два — `smoke_cold_view_toggle` и `smoke_cold_view_vacuum` вешают собственный +слушатель. Они, в отличие от первых двух, **не** слепая зона: страница приходит +из `launchColdView()`, то есть уже зарегистрирована, а свой счётчик они +превращают в отдельное утверждение `noPageErrors`. Такая подписка законна и +полезна. Поэтому инвариант сформулирован не как «никто не подписывается сам», а +как «ни одна страница не создаётся мимо гарда» — и закреплён тестом по всему +набору, а не по двум названным файлам. + +**AC7 скорректирован**: дифф задачи содержит `demo/smoke_zoom_flash.mjs`, +`demo/smoke_svg_sandbox.mjs` (регистрация страниц) и пять смоков из #407 +(вердикт стал асинхронным), помимо `smoke_danger_confirmation.mjs`. Сигнатура +`finish(browser, out)` не изменилась — это и было существом AC7. + +## Отступления от ревизии 1, каждое по измеренной причине + +**1. Пробы лежат в `demo/guard/`, а не в `demo/fixtures/`.** Каталог +`demo/fixtures/**/*.mjs` входит в корпус `sourceFingerprint` +(`scripts/source-fingerprint.mjs:66`), поэтому каждый новый `.mjs` там объявляет +устаревшими закоммиченный бандл, скриншот-индекс документации и golden-индекс. +Пробы гарда не касаются ни одного пикселя — платить за них пересъёмкой нечем. +Запрет закреплён тестом. + +**2. Поведение доказывается в job «Смоки в браузере», а не в `npm test`.** +Пробам нужен настоящий Chromium, а job «Фронтенд», где идёт `npm test`, браузеры +не ставит. Тест на пробах там молча скипался бы — то есть ровно тот тихий успех, +против которого вся задача. В `test/` остались проверки, которые запуском не +делаются: порядок операций в исходнике, место проб, наличие вызова в workflow и +регистрация мутантов. + +## Сценарий + +Смок гоняет карточку, внутри карточки происходит необработанное исключение, и +смок печатает `OK`. CI зелёный. Никто ничего не узнаёт — ни сегодня, ни через +месяц, когда это исключение станет жалобой пользователя. + +Гард ровно для этого и писался (комментарий `demo/serve.mjs:15-18`: «Until +2026-07-27 the smokes printed booleans and always exited 0»). Он существует, +он выглядит работающим, и в самом частом случае он молчит. + +## Что человек увидит до и после + +Видимого поведения продукта задача не меняет — меняется то, что видит +разработчик и CI. + +**До**: исключение, случившееся после последнего обращения смока к странице, не +роняет смок и **даже не печатается**: `browser.close()` уносит недоставленное +событие. Смок сообщает `OK`, exit 0. +**После**: такое исключение считается и роняет смок, как и было задумано. + +## Проблема и контракт + +`finish()` (`demo/serve.mjs:34-45`) читает `_pageErrors` синхронно: + +```js +export async function finish(browser, out) { + if (out !== undefined) console.log(JSON.stringify(out, null, 1)); + if (_pageErrors) _failures.push(`${_pageErrors} uncaught exception(s) inside the card`); + await browser?.close?.(); + … +} +``` + +События `pageerror` Playwright доставляет асинхронно по CDP. Если исключение +возникло после последнего обращения смока к странице, счётчик к моменту проверки +ещё нулевой. + +**Воспроизведено исполнением** (проба: `launch()` → исключение в странице → +сразу `finish()`): + +``` +{ "probe": "exception thrown, finish() called immediately" } +EXC Error: boom-async ← доставлено во время browser.close() +OK ← гард уже прочитал 0 +exit=0 +``` + +Порядок вывода — сам диагноз: `EXC` печатается после результата и до `OK`. + +**Граница дефекта установлена, и она уже:** гард не сломан всегда. Та же проба с +одним `await page.evaluate(() => 0)` между исключением и `finish()` даёт +`FAILED (1): 1 uncaught exception(s)` и `exit=1`. Слепа только зона «после +последнего обращения к странице» — то есть финальные ре-рендеры, закрытие +диалогов и всё, что карточка делает, пока смок печатает результат. + +**Контракт**: `finish()` обязана прочитать счётчик после того, как страница +доставила всё, что успела произвести к моменту вызова. Достигается round-trip'ом +к каждой открытой странице до чтения счётчика: + +```js +for (const page of _livePages) { try { await page.evaluate(() => 0); } catch { /* закрыта */ } } +``` + +Круговой запрос к странице вытесняет ранее поставленные макрозадачи и +гарантирует, что события, порождённые до него, уже дошли до Node. + +**Ссылок на страницы у `finish()` сейчас нет**, а менять её сигнатуру нельзя: +`finish(browser, out)` зовут 205 смоков. Страницы регистрируются там, где +создаются, — в `launchInternal`, рядом с подпиской на `pageerror`. + +**Честная граница, которую задача не закрывает**: исключение, возникшее *после* +round-trip'а (например, в обработчике `beforeunload` при закрытии браузера), +по-прежнему не будет учтено. Ловить его — значит ждать неизвестно чего +неизвестно сколько. Контракт формулируется как «всё, что произошло до момента +вызова `finish()`», и это записывается в комментарии у кода. + +### Что вскрывает починенный гард + +Прогон **всех 211** `demo/smoke_*.mjs` с починенным гардом: краснеет ровно один — +`smoke_danger_confirmation` (2 исключения, других провалов нет). Оба одинаковы: + +``` +TypeError: Cannot read properties of undefined (reading 'split') + at _bindingHasHaPage (houseplan-card.ts:4814) + at _renderMarkerDialog (houseplan-editor-runtime.ts:12652) +``` + +Причина — фикстура смока, а не карточка. `demo/smoke_danger_confirmation.mjs:167`: + +```js +card._markerDialog = { devId: 'danger-marker', name: 'Danger marker', busy: false }; +``` + +Объявленный тип (`houseplan-card.ts:2222-2235`) требует `binding: string` и +`bindingMode`; JS-смок это не проверяет. Все 15 мест в `src/`, создающих диалог, +`binding` пишут — дефекта поведения нет. Фикстура чинится в этой же задаче: +иначе смок покраснеет по чужой причине. + +## Скоуп / не-скоуп + +**В скоупе**: `demo/serve.mjs` (регистрация страниц и round-trip в `finish`), +фикстура `demo/smoke_danger_confirmation.mjs:167`, новые фикстуры-пробы и тест, +доказывающий, что гард умеет падать. + +**Не в скоупе**: сигнатура `finish` и остальные 204 смока; `_bindingHasHaPage` +и типизация диалога маркера (дефекта нет); перевод смоков на TypeScript; +оброненный промис в удалении черновика (#405) — он ловится тем же гардом, но +чинится своей задачей. + +## UX, модель данных, i18n + +Не применимо: продуктового кода задача не касается. + +## Критерии приёмки + +- **AC1**. Исключение, брошенное в странице непосредственно перед `finish()` + без промежуточных обращений к странице, роняет смок: `exit=1`, в выводе + строка `uncaught exception(s) inside the card`. Доказательство: фикстура-проба + плюс тест, запускающий её процессом. +- **AC2**. **Отрицательный прогон обязателен**: та же фикстура на коде без + round-trip'а даёт `exit=0`. Доказательство: мутант, зарегистрированный в + `scripts/mutation-gate.mjs` и прогнанный штатным раннером (`--id=…`), а не + ручной правкой файла. +- **AC3**. Необработанное отклонение промиса (`Promise.reject`) считается так + же, как исключение. Проверено, что `page.on('pageerror')` в Chromium его + получает; AC закрепляет это фикстурой, чтобы связь с #405 не потерялась. +- **AC4**. Смок без исключений остаётся зелёным, и round-trip не делает набор + заметно медленнее: замер трёх смоков до и после, разница в пределах шума + (порог — 5% суммарного времени трёх прогонов). +- **AC5**. `smoke_danger_confirmation` зелёный: фикстура `:167` приведена в + соответствие объявленному типу (`binding`, `bindingMode`), исключений 0. + Утверждения смока при этом не ослабляются — правится только фикстура. +- **AC6**. Весь набор `demo/smoke_*.mjs` зелёный. Доказательство: полный прогон + (211 файлов); допускается один известный внешний отказ — + `smoke_infinite_canvas` требует поднятого бэкенда и краснеет и без правки. +- **AC7**. Сигнатура `finish(browser, out)` не изменилась, 204 смока не + тронуты. Доказательство: дифф задачи не содержит `demo/smoke_*.mjs`, кроме + `smoke_danger_confirmation.mjs` и новых фикстур. +- **AC8**. Закрытая или упавшая страница не ломает `finish()`: round-trip к ней + проглатывается, остальные страницы опрашиваются. Доказательство: фикстура, + закрывающая страницу до `finish()`. + +## План автотестов + +**Фикстуры** (новые файлы, не входят в набор `smoke_*` — иначе CI будет гонять +заведомо красный смок; имя без префикса `smoke_`, каталог `demo/fixtures/`): + +1. `guard_tail_exception.mjs` — исключение прямо перед `finish()` (AC1, AC2). +2. `guard_tail_rejection.mjs` — `Promise.reject` прямо перед `finish()` (AC3). +3. `guard_closed_page.mjs` — страница закрыта до `finish()` (AC8). + +**Тест** (`test/smoke-exception-guard.test.mjs`, прецедент — 8 тестов в `test/` +уже запускают процессы через `spawnSync`): + +- каждая фикстура запускается процессом, проверяется код возврата и текст; +- отдельная проверка, что фикстуры лежат вне маски `smoke_*` (иначе они + попадут в CI-шарды и покрасят их). + +**Мутанты** (`scripts/mutation-gate.mjs`): + +- `smoke-guard-blind-to-tail`: убрать цикл round-trip'а из `finish` → тест AC1 + краснеет; +- `smoke-guard-forgets-to-register-pages`: убрать `_livePages.push(page)` из + `launchInternal` → тот же тест краснеет (цикл есть, опрашивать нечего). + +Второй мутант нужен потому, что правка состоит из двух половин, и мутант, +проверяющий только одну, оставит вторую недоказанной. + +## Риски + +- **Round-trip замедляет набор.** Один `evaluate` — единицы миллисекунд на + смок; при 211 смоках это доли секунды. Смягчение: AC4 меряет, а не + предполагает. +- **Фикстуры попадут в CI как обычные смоки и покрасят его.** Смягчение: имя вне + маски `smoke_*` плюс явная проверка в тесте. +- **Гард начнёт краснеть на чужих смоках после будущих правок.** Это и есть + цель, но первый такой случай выглядит как «сломали CI». Смягчение: сообщение + гарда уже называет причину дословно, а прогон 211 смоков в этой задаче + фиксирует базовую линию — сегодня краснеет ровно один и по известной причине. +- **`page.evaluate` на странице, ушедшей в навигацию, бросит.** Смягчение: + try/catch вокруг каждого round-trip'а, AC8. + +## Откат + +Обе правки локальны: цикл в `finish()` и регистрация страниц в +`launchInternal`. Возврат — удаление двух фрагментов; фикстуры и тест при этом +станут красными, что и покажет откат явно. + +## Release-артефакты + +- `docs/CHANGELOG.md` / `docs/CHANGELOG.ru.md`: **не требуется** (User-Visible: + no) — задача не меняет продукт. +- Скриншоты не меняются. diff --git a/scripts/mutation-gate.mjs b/scripts/mutation-gate.mjs index 33f7b6aa..04c320ef 100644 --- a/scripts/mutation-gate.mjs +++ b/scripts/mutation-gate.mjs @@ -105,6 +105,29 @@ function relocateEditorPatch(patch, cardSource, editorSource) { // `find` обязан встречаться в файле ровно один раз: патч, который ложится «куда // попало», проверяет не то, что объявлен проверять. Это контролирует --check. const MUTANT_DEFINITIONS = [ + { + id: 'smoke-guard-blind-to-tail', + guard: 'node demo/guard/verify-guard.mjs', + because: 'the uncaught-exception guard must read its counter AFTER the page delivered ' + + 'its events; reading it first is the defect of #404 and looks identical to a ' + + 'working guard in everything but the outcome', + patches: [{ + file: 'demo/serve.mjs', + find: ' await roundTripLivePages();\n if (_pageErrors) _failures.push(', + replace: ' if (_pageErrors) _failures.push(', + }], + }, + { + id: 'smoke-guard-forgets-to-register-pages', + guard: 'node demo/guard/verify-guard.mjs', + because: 'the round-trip and the page registry are two halves of one fix (#404): a ' + + 'mutant on either half alone leaves the other unproven', + patches: [{ + file: 'demo/serve.mjs', + find: ' _livePages.add(page);', + replace: '', + }], + }, { id: 'settings-help-party1-placement-removed', guard: 'npx tsc -p tsconfig.test.json && node scripts/fix-test-build.mjs ' diff --git a/test/smoke-exception-guard.test.mjs b/test/smoke-exception-guard.test.mjs new file mode 100644 index 00000000..ccf368aa --- /dev/null +++ b/test/smoke-exception-guard.test.mjs @@ -0,0 +1,117 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { existsSync, readFileSync, readdirSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; + +// #404. Гард «uncaught exception внутри карточки» существовал с 2026-07-27 и не +// срабатывал ни разу: счётчик читался синхронно, а Playwright доставляет +// pageerror асинхронно. Поведение доказывается запуском проб — +// demo/guard/verify-guard.mjs в job со браузером. Здесь закреплено то, что +// запуском не проверить: порядок операций в исходнике, место проб и наличие +// вызова там, где он должен быть. + +const ROOT = fileURLToPath(new URL('../', import.meta.url)); +const read = (rel) => readFileSync(new URL(rel, `file://${ROOT}`), 'utf8'); + +test('счётчик читается ПОСЛЕ доставки событий, а не до (#404)', () => { + const serve = read('demo/serve.mjs'); + const flush = serve.indexOf('await roundTripLivePages();\n if (_pageErrors)'); + assert.ok(flush > 0, + 'round-trip обязан стоять непосредственно перед чтением счётчика: обратный' + + ' порядок и был дефектом #404'); + // Вторая половина правки: опрашивать нечего, если страницы не регистрируются. + assert.match(serve, /const _livePages = new Set\(\)/); + assert.match(serve, /export function watchPage\(page\)/); + assert.match(serve, /_livePages\.add\(page\)/); + assert.match(serve, /page\.on\('close', \(\) => _livePages\.delete\(page\)\)/, + 'закрытая страница обязана уходить из реестра'); + // Round-trip к закрытой странице бросает — и не имеет права ронять вердикт. + assert.match(serve, /try \{ await page\.evaluate\(\(\) => 0\); \} catch/); +}); + +test('оба читателя счётчика ждут доставки (#404, #407)', () => { + const serve = read('demo/serve.mjs'); + assert.match(serve, /export async function reportPageErrors\(\)\s*\{\s*await roundTripLivePages\(\);/, + 'вердикт для смоков со своей развязкой обязан ждать так же, как finish()'); +}); + +test('пробы гарда лежат вне маски смоков и вне корпуса отпечатка (#404)', () => { + const guard = readdirSync(new URL('demo/guard/', `file://${ROOT}`)); + assert.ok(guard.includes('verify-guard.mjs')); + const probes = guard.filter((name) => name.startsWith('guard_')); + assert.equal(probes.length, 3, 'три пробы: хвостовое исключение, отклонение, закрытая страница'); + + // Маска demo/smoke_*.mjs — то, что гоняют шарды CI. Проба, попавшая туда, + // покрасит шард по построению: она обязана падать. + for (const name of probes) { + assert.equal(name.startsWith('smoke_'), false, `${name} попала бы в шарды CI`); + } + + // demo/fixtures/**/*.mjs входит в корпус sourceFingerprint: каждый файл там + // объявляет устаревшими бандл, скриншот-индекс и golden-индекс. Пробы гарда + // не касаются ни одного пикселя — платить за них пересъёмкой нечем. + for (const name of probes.concat('verify-guard.mjs')) { + assert.equal(existsSync(new URL(`demo/fixtures/${name}`, `file://${ROOT}`)), false, + `${name} в demo/fixtures/ протухал бы отпечаток исходников`); + } +}); + +test('пробы вызываются в job с браузером и служат guard мутантов (#404)', () => { + const workflow = read('.github/workflows/validate.yml'); + const smoke = workflow.slice(workflow.indexOf('\n smoke:\n'), workflow.indexOf('\n smoke_done:\n')); + assert.match(smoke, /node demo\/guard\/verify-guard\.mjs/, + 'проверка обязана идти там, где есть Chromium: в job «Фронтенд» npm test браузер не ставит,' + + ' и тест на пробах молча скипался бы'); + assert.match(smoke, /if: matrix\.shard == 1/, 'один раз, а не в каждом шарде'); + + const mutants = read('scripts/mutation-gate.mjs'); + for (const id of ['smoke-guard-blind-to-tail', 'smoke-guard-forgets-to-register-pages']) { + assert.match(mutants, new RegExp(`id: '${id}'`), `мутант ${id} не зарегистрирован`); + } + // Правка состоит из двух половин, и мутант на одну оставил бы другую + // недоказанной. + assert.equal((mutants.match(/node demo\/guard\/verify-guard\.mjs/g) || []).length, 2); +}); + +test('страницы, созданные вне launch(), подписаны общим гардом (#404, Medium-1)', () => { + // Ревью ТЗ нашло разрыв: smoke_zoom_flash открывал вторую страницу со своей + // подпиской, печатавшей EXC2 мимо счётчика, а три страницы smoke_svg_sandbox + // не имели слушателя вовсе. Регистрация в launchInternal их не покрывает. + const zoom = read('demo/smoke_zoom_flash.mjs'); + assert.match(zoom, /watchPage\(await ctx\.newPage\(\)\)/); + assert.equal(/p2\.on\('pageerror'/.test(zoom), false, + 'своя подписка мимо счётчика убрана (EXC2 остался только в объяснении)'); + + const sandbox = read('demo/smoke_svg_sandbox.mjs'); + assert.equal((sandbox.match(/watchPage\(await ctx\.newPage\(\)\)/g) || []).length, 3); + + // Общий инвариант — не «никто не подписывается сам», а «ни одна страница не + // создаётся мимо гарда». Своя подписка поверх гарда законна и полезна: + // smoke_cold_view_toggle и smoke_cold_view_vacuum считают исключения ещё и + // отдельным утверждением `noPageErrors`, а их страница приходит из + // launchColdView, то есть уже зарегистрирована. + const demo = fileURLToPath(new URL('demo/', `file://${ROOT}`)); + const smokes = readdirSync(demo) + .filter((name) => name.startsWith('smoke_') && name.endsWith('.mjs')); + const unwatched = []; + for (const name of smokes) { + const text = readFileSync(demo + name, 'utf8'); + const total = (text.match(/newPage\(\)/g) || []).length; + const watched = (text.match(/watchPage\(await [\w.]+\.newPage\(\)\)/g) || []).length; + if (total > watched) unwatched.push(name); + } + assert.deepEqual(unwatched, ['smoke_entry_stale.mjs'], + 'страница, созданная мимо watchPage, остаётся слепой зоной гарда.' + + ' Единственное законное исключение — smoke_entry_stale: он намеренно' + + ' поднимает свой браузер, ведёт свой счётчик исключений (он и есть' + + ' предмет проверки #353) и выносит вердикт через finish()'); + + // Свои подписки допускаются только поверх общего гарда: страница обязана + // приходить из launch()/launchColdView(), иначе счётчик гарда её не видит. + for (const name of smokes) { + const text = readFileSync(demo + name, 'utf8'); + if (!/\.on\('pageerror'/.test(text) || name === 'smoke_entry_stale.mjs') continue; + assert.match(text, /launch(ColdView)?[,}]|launch(ColdView)?\(/, + `${name}: своя подписка на pageerror поверх страницы, которую гард не знает`); + } +}); diff --git a/test/smoke-harness-contract.test.mjs b/test/smoke-harness-contract.test.mjs index 18701d03..99657930 100644 --- a/test/smoke-harness-contract.test.mjs +++ b/test/smoke-harness-contract.test.mjs @@ -52,7 +52,7 @@ test('вердикт не только выставляет код, но и ос for (const name of smokes()) { const text = read(name); if (!/\breportPageErrors\s*\(/.test(text)) continue; - assert.match(text, /if \(reportPageErrors\(\)\)\s*(process\.exit\(1\)|throw )/, + assert.match(text, /if \(await reportPageErrors\(\)\)\s*(process\.exit\(1\)|throw )/, `${name}: вердикт по исключениям обязан останавливать смок, а не только` + ' помечать его — иначе после «FAILED» печатается строка успеха'); } @@ -64,9 +64,10 @@ test('serve.mjs остаётся единственным владельцем // существует законно (он и есть предмет проверки), но вердикт всё равно // выносит finish(). const serve = readFileSync(new URL('../demo/serve.mjs', import.meta.url), 'utf8'); - assert.match(serve, /export function reportPageErrors\(\)/); + assert.match(serve, /export async function reportPageErrors\(\)/, + 'вердикт стал асинхронным в #404: он ждёт доставки событий страницей'); assert.equal((serve.match(/_pageErrors\+\+/g) || []).length, 1, - 'счётчик инкрементируется в одном месте'); + 'счётчик инкрементируется в одном месте — внутри watchPage (#404)'); const entryStale = read('smoke_entry_stale.mjs'); assert.match(entryStale, /await finish\(/, 'smoke_entry_stale обязан выносить вердикт'); });