diff --git a/app/app/Console/Commands/ReapStuckCreativeJobs.php b/app/app/Console/Commands/ReapStuckCreativeJobs.php new file mode 100644 index 00000000..642a1bbb --- /dev/null +++ b/app/app/Console/Commands/ReapStuckCreativeJobs.php @@ -0,0 +1,82 @@ +subMinutes(self::STUCK_MINUTES); + + // taken_at без значения тоже считаем зависшим: такое задание иначе не разобрать + // ничем, а в работе оно висит и очередь держит. + $stuck = AdCreativeJob::on('pgsql_admin') + ->where('status', AdCreativeJob::STATUS_TAKEN) + ->where(fn (Builder $q) => $q->whereNull('taken_at')->orWhere('taken_at', '<', $deadline)) + ->get(); + + foreach ($stuck as $job) { + if ($job->attempts >= self::MAX_ATTEMPTS) { + $job->update([ + 'status' => AdCreativeJob::STATUS_FAILED, + 'failure_reason' => sprintf( + 'Робот не отчитался за %d минут, и попытки исчерпаны (%d). Задание закрыто сторожем очереди.', + self::STUCK_MINUTES, + $job->attempts, + ), + 'finished_at' => now(), + ]); + + Log::warning('Задание робота закрыто сторожем: попытки исчерпаны', [ + 'job_id' => $job->id, 'campaign_id' => $job->campaign_id, 'attempts' => $job->attempts, + ]); + $this->warn("Задание #{$job->id}: попытки исчерпаны, закрыто сбоем."); + + continue; + } + + $job->update(['status' => AdCreativeJob::STATUS_QUEUED, 'taken_at' => null]); + + Log::warning('Задание робота возвращено в очередь: робот не отчитался', [ + 'job_id' => $job->id, 'campaign_id' => $job->campaign_id, 'attempts' => $job->attempts, + ]); + $this->info("Задание #{$job->id}: возвращено в очередь."); + } + + if ($stuck->isEmpty()) { + $this->info('Зависших заданий нет.'); + } + + return self::SUCCESS; + } +} diff --git a/app/app/Http/Controllers/Api/CreativeRobotController.php b/app/app/Http/Controllers/Api/CreativeRobotController.php index 3d585223..ef0c27ff 100644 --- a/app/app/Http/Controllers/Api/CreativeRobotController.php +++ b/app/app/Http/Controllers/Api/CreativeRobotController.php @@ -11,8 +11,10 @@ use App\Models\AdCreativeJob; use App\Services\Advertising\CreativeJobService; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; +use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Storage; use Symfony\Component\HttpFoundation\StreamedResponse; +use Throwable; /** * Служебный канал робота-грузчика креативов. Три действия: взять задание, скачать файл @@ -117,6 +119,25 @@ class CreativeRobotController extends Controller } catch (CreativeMatchFailedException $e) { // Задание уже помечено сбойным внутри complete(). Роботу отвечаем 200: свою // работу он сделал, разошёлся слепок креативов — это наша сторона, не его. + return response()->json(['status' => AdCreativeJob::STATUS_FAILED, 'message' => $e->getMessage()]); + } catch (Throwable $e) { + // Приём отчёта ходит в живой Яндекс за слепком креативов. Любая другая беда + // (API недоступен, лимит, оборвалась сеть) раньше улетала наружу: робот получал + // 500, а задание НАВСЕГДА оставалось «в работе». А пока хоть одно задание в + // работе, выдача отвечает «работы нет» ВСЕМ — очередь встаёт колом для всех + // клиентов сразу. Поэтому закрываем задание сбойным: возобновляемый запуск + // поставит новое, и работа продолжится. + Log::error('Не смогли принять отчёт робота о креативах', [ + 'job_id' => $job->id, + 'campaign_id' => $job->campaign_id, + 'error' => $e->getMessage(), + ]); + + $fresh = $job->fresh(); + if ($fresh !== null && $fresh->status === AdCreativeJob::STATUS_TAKEN) { + $this->jobs->fail($fresh, 'Портал не смог принять отчёт: '.$e->getMessage()); + } + return response()->json(['status' => AdCreativeJob::STATUS_FAILED, 'message' => $e->getMessage()]); } diff --git a/app/app/Services/Advertising/CreativeIdMatcher.php b/app/app/Services/Advertising/CreativeIdMatcher.php index 6b776aab..410d4678 100644 --- a/app/app/Services/Advertising/CreativeIdMatcher.php +++ b/app/app/Services/Advertising/CreativeIdMatcher.php @@ -18,14 +18,26 @@ final class CreativeIdMatcher { /** * @param array $before слепок до загрузки: номер → [ш, в] - * @param array $after слепок после загрузки + * @param array $after слепок после загрузки * @param list $expectedSizes размеры, которые робот должен был залить - * @return array «300x250» → номер креатива + * @return array «300x250» → номер креатива * * @throws CreativeMatchFailedException */ public function match(array $before, array $after, array $expectedSizes): array { + // Пустой список ожидаемых размеров — не «всё сошлось», а тихий ноль: цикл ниже + // просто не выполнится и вернёт пустоту как успех. Задание пометится «готово» + // с нулём проставленных номеров, запуск снова увидит баннеры без креативов и + // поставит новое задание — робот будет заливать те же файлы по кругу, оставляя + // каждый раз пачку мусорных креативов в живом кабинете. Ловим это громко. + if ($expectedSizes === []) { + throw new CreativeMatchFailedException( + 'Опознавать нечего: список ожидаемых размеров пуст. Так бывает, если баннеры кампании не видны ' + .'служебной роли — проверьте, что после выката перезапущен db/03_service_bypass_policies.sql.' + ); + } + $newIds = array_diff_key($after, $before); $bySize = []; diff --git a/app/app/Services/Advertising/CreativeJobService.php b/app/app/Services/Advertising/CreativeJobService.php index ed1ee43f..33852f75 100644 --- a/app/app/Services/Advertising/CreativeJobService.php +++ b/app/app/Services/Advertising/CreativeJobService.php @@ -97,6 +97,19 @@ final class CreativeJobService ->orderBy('width')->orderBy('height') ->get(); + // Проверяем ДО похода в Яндекс. Ноль баннеров — это не «нечего делать, всё хорошо», + // а признак, что мы их не видим: например, служебной роли не выдан кросс-тенантный + // доступ (`db/03_service_bypass_policies.sql` не перезапущен после выката). Пойди мы + // дальше — задание закрылось бы «готово» с нулём проставленных номеров, а запуск + // поставил бы новое задание, и робот заливал бы те же файлы по кругу. + if ($banners->isEmpty()) { + $this->markFailed($job, $e = new CreativeMatchFailedException( + "У кампании #{$job->campaign_id} не видно ни одного включённого баннера — опознавать нечего. " + .'Проверьте, что после выката перезапущен db/03_service_bypass_policies.sql.' + )); + throw $e; + } + $expected = $banners->map(fn ($b) => [(int) $b->width, (int) $b->height])->all(); $before = $this->normalizeSnapshot($job->snapshot_before ?? []); $after = $this->client()->listImageCreativeIds(); @@ -104,11 +117,7 @@ final class CreativeJobService try { $matched = $this->matcher->match($before, $after, $expected); } catch (CreativeMatchFailedException $e) { - $job->update([ - 'status' => AdCreativeJob::STATUS_FAILED, - 'failure_reason' => mb_substr($e->getMessage(), 0, 1024), - 'finished_at' => now(), - ]); + $this->markFailed($job, $e); throw $e; } @@ -132,6 +141,16 @@ final class CreativeJobService ]); } + /** Закрыть задание сбойным с причиной несостоявшегося опознания. */ + private function markFailed(AdCreativeJob $job, CreativeMatchFailedException $e): void + { + $job->update([ + 'status' => AdCreativeJob::STATUS_FAILED, + 'failure_reason' => mb_substr($e->getMessage(), 0, 1024), + 'finished_at' => now(), + ]); + } + /** * Отчитаться можно только по заданию в работе. Проверка продублирована здесь, а не * оставлена одному контроллеру: сервис зовут не только из него, а закрытое задание, diff --git a/app/routes/console.php b/app/routes/console.php index 9e8c9efc..ab7d73e0 100644 --- a/app/routes/console.php +++ b/app/routes/console.php @@ -337,3 +337,13 @@ Schedule::job(new SyncCampaignModerationJob) ->timezone('Europe/Moscow') ->onSuccess(fn () => $hb->recordRunResult('App\Jobs\SyncCampaignModerationJob', true, null, null)) ->onFailure(fn () => $hb->recordRunResult('App\Jobs\SyncCampaignModerationJob', false, 'Job failed', null)); + +// Сторож очереди робота-грузчика креативов. Задание, брошенное «в работе», держит очередь +// для ВСЕХ клиентов: пока хоть одно в работе, выдача отвечает «работы нет» и ни одна +// кампания не стартует. Робот может умереть молча — разбирать пробку руками некому. +// Каждые 10 минут при пороге в 30 минут: свежее задание сторож не отбирает. +Schedule::command('creative-jobs:reap') + ->everyTenMinutes() + ->timezone('Europe/Moscow') + ->onSuccess(fn () => $hb->recordRunResult('creative-jobs:reap', true, null, null)) + ->onFailure(fn () => $hb->recordRunResult('creative-jobs:reap', false, 'Command failed', null)); diff --git a/app/tests/Feature/Advertising/AdvertisingScheduleTest.php b/app/tests/Feature/Advertising/AdvertisingScheduleTest.php index ced35214..9a6a2cd4 100644 --- a/app/tests/Feature/Advertising/AdvertisingScheduleTest.php +++ b/app/tests/Feature/Advertising/AdvertisingScheduleTest.php @@ -13,3 +13,16 @@ it('registers advertising Direct jobs in the scheduler', function () { ->and($joined)->toContain('SyncCampaignAudienceJob') ->and($joined)->toContain('SyncCampaignModerationJob'); }); + +/** + * Сторож очереди робота обязан быть в расписании. + * + * Сама команда без расписания бесполезна: зависшее «в работе» задание держит очередь для + * ВСЕХ клиентов, а разбирать пробку руками некому — робот работает по ночам без людей. + */ +it('registers the creative job reaper in the scheduler', function () { + $events = app(Schedule::class)->events(); + $joined = implode("\n", array_map(fn ($e) => $e->getSummaryForDisplay(), $events)); + + expect($joined)->toContain('creative-jobs:reap'); +}); diff --git a/app/tests/Feature/Advertising/CreativeJobReaperTest.php b/app/tests/Feature/Advertising/CreativeJobReaperTest.php new file mode 100644 index 00000000..0f0ca442 --- /dev/null +++ b/app/tests/Feature/Advertising/CreativeJobReaperTest.php @@ -0,0 +1,74 @@ +create(); + $campaign = AdCampaign::create([ + 'tenant_id' => $tenant->id, 'name' => 'C', 'mode' => AdCampaign::MODE_MANUAL, + 'audience_days' => 10, 'client_cpm_rub' => '120.00', + ]); + + return AdCreativeJob::create(array_merge([ + 'tenant_id' => $tenant->id, + 'campaign_id' => $campaign->id, + 'status' => AdCreativeJob::STATUS_TAKEN, + 'attempts' => 1, + 'snapshot_before' => [], + 'taken_at' => now()->subMinutes(40), + ], $attributes)); +} + +it('returns a job abandoned in flight back to the queue', function () { + $job = makeStuckJob(); + + $this->artisan('creative-jobs:reap')->assertExitCode(0); + + expect($job->fresh()->status)->toBe(AdCreativeJob::STATUS_QUEUED) + ->and($job->fresh()->taken_at)->toBeNull(); +}); + +it('leaves a job that was taken just now alone', function () { + // Робот работает минутами: качает файлы, поднимает браузер, грузит 15 креативов. + // Отобрать у него задание на середине — значит получить два робота на одной кампании. + $job = makeStuckJob(['taken_at' => now()->subMinutes(3)]); + + $this->artisan('creative-jobs:reap')->assertExitCode(0); + + expect($job->fresh()->status)->toBe(AdCreativeJob::STATUS_TAKEN); +}); + +it('closes the job as failed once the attempts are spent instead of looping forever', function () { + $job = makeStuckJob(['attempts' => 3]); + + $this->artisan('creative-jobs:reap')->assertExitCode(0); + + expect($job->fresh()->status)->toBe(AdCreativeJob::STATUS_FAILED) + ->and($job->fresh()->failure_reason)->toContain('не отчитался') + ->and($job->fresh()->finished_at)->not->toBeNull(); +}); + +it('does not touch jobs that are not in flight', function () { + $job = makeStuckJob(['status' => AdCreativeJob::STATUS_DONE, 'taken_at' => now()->subDays(3)]); + + $this->artisan('creative-jobs:reap')->assertExitCode(0); + + expect($job->fresh()->status)->toBe(AdCreativeJob::STATUS_DONE); +}); diff --git a/app/tests/Feature/Advertising/CreativeJobServiceTest.php b/app/tests/Feature/Advertising/CreativeJobServiceTest.php index 32ecb23d..d1e8c002 100644 --- a/app/tests/Feature/Advertising/CreativeJobServiceTest.php +++ b/app/tests/Feature/Advertising/CreativeJobServiceTest.php @@ -7,6 +7,7 @@ use App\Models\AdCampaign; use App\Models\AdCampaignBanner; use App\Models\AdCreativeJob; use App\Models\Tenant; +use App\Services\Advertising\CreativeIdMatcher; use App\Services\Advertising\CreativeJobService; use Illuminate\Database\QueryException; use Illuminate\Foundation\Testing\DatabaseTransactions; @@ -138,6 +139,44 @@ it('fails the job and touches no banner when the snapshot does not add up', func ->and($job->fresh()->failure_reason)->toContain('728x90'); }); +/** + * Пустой список ожидаемых размеров — это НЕ «всё сошлось», это тихий ноль. + * + * Раньше `match()` с пустым списком просто не входил в цикл и молча возвращал пустоту. + * А пустой список получается сам собой: если после выката не перезапустить + * `db/03_service_bypass_policies.sql`, служебная роль не увидит ни одного баннера. Дальше + * задание помечается «готово» с нулём проставленных номеров, запуск снова видит баннеры + * без креативов, ставит новое задание — и робот заливает те же файлы по кругу, оставляя + * каждый раз пачку мусорных креативов в живом кабинете. В журнале при этом всё зелёное. + */ +it('refuses to call an empty match a success', function () { + expect(fn () => app(CreativeIdMatcher::class)->match([], [], [])) + ->toThrow(CreativeMatchFailedException::class); +}); + +it('fails a done report for a campaign without banners without asking Yandex at all', function () { + config(['services.yandex_direct.enabled' => true]); + config(['services.yandex_direct.token' => 'T']); + config(['services.yandex_direct.base_url' => 'https://api.direct.yandex.com']); + Http::fake(); + + // Мимо enqueue(): он сам сходил бы в Яндекс за слепком и испортил проверку «не ходили». + $campaign = makeCampaignWithBanners([]); + $job = AdCreativeJob::create([ + 'tenant_id' => $campaign->tenant_id, + 'campaign_id' => $campaign->id, + 'status' => AdCreativeJob::STATUS_TAKEN, + 'snapshot_before' => [], + 'taken_at' => now(), + ]); + + expect(fn () => app(CreativeJobService::class)->complete($job)) + ->toThrow(CreativeMatchFailedException::class); + + Http::assertNothingSent(); + expect($job->fresh()->status)->toBe(AdCreativeJob::STATUS_FAILED); +}); + /** * «Задание в работе всегда не больше одного» — инвариант, на котором держится ВСЁ * опознание креативов: слепки `creatives.get` до/после снимаются по аккаунту целиком, diff --git a/app/tests/Feature/Advertising/CreativeRobotEndpointTest.php b/app/tests/Feature/Advertising/CreativeRobotEndpointTest.php index 1994630f..8a8a51c3 100644 --- a/app/tests/Feature/Advertising/CreativeRobotEndpointTest.php +++ b/app/tests/Feature/Advertising/CreativeRobotEndpointTest.php @@ -206,6 +206,34 @@ it('accepts a failure report from the robot', function () { ->and($job->fresh()->failure_reason)->toBe('вход слетел'); }); +/** + * Своя беда портала не должна вешать очередь. + * + * Приём отчёта «готово» ходит в живой Яндекс за слепком креативов. Любая ошибка API + * (недоступен, лимит, отвалилась сеть) вылетала наружу необработанной: робот получал 500, + * задание навсегда оставалось «в работе», а выдача заданий при живом «в работе» отвечает + * «работы нет» ВСЕМ — ни одна кампания больше не стартовала бы. + */ +it('does not leave the job in flight when the portal itself fails on the done report', function () { + Http::fake(['*/json/v5/creatives' => Http::sequence() + ->push(['result' => ['Creatives' => []]]) + ->push(['error' => ['error_string' => 'Сервис временно недоступен']], 500), + ]); + + [$campaign, $banners] = makeRobotCampaign([[300, 250]]); + $job = app(CreativeJobService::class)->enqueue($campaign); + app(CreativeJobService::class)->takeNext(); + + $this->withHeader('X-Creative-Robot-Token', 'ROBOTSECRET') + ->postJson("/api/creative-robot/jobs/{$job->id}/done", ['ok' => true]) + ->assertOk() + ->assertJsonPath('status', AdCreativeJob::STATUS_FAILED); + + expect($job->fresh()->status)->toBe(AdCreativeJob::STATUS_FAILED) + ->and($job->fresh()->failure_reason)->toContain('Сервис временно недоступен') + ->and($banners[0]->fresh()->yandex_creative_id)->toBeNull(); +}); + /** * Отчёт принимается ТОЛЬКО по заданию, которое сейчас в работе. * diff --git a/bots/yandex-creatives/.env.example b/bots/yandex-creatives/.env.example index cfe573fb..5892a415 100644 --- a/bots/yandex-creatives/.env.example +++ b/bots/yandex-creatives/.env.example @@ -15,3 +15,5 @@ SMTP_PASS= ALARM_FROM=robot@liderra.ru ALARM_TO=ops@liderra.ru HUMAN_DELAY_MS=800 +# Пауза между повторами доклада порталу «готово» после удачной загрузки. +REPORT_RETRY_DELAY_MS=3000 diff --git a/bots/yandex-creatives/src/cabinet.js b/bots/yandex-creatives/src/cabinet.js index 90bd85da..b9477984 100644 --- a/bots/yandex-creatives/src/cabinet.js +++ b/bots/yandex-creatives/src/cabinet.js @@ -108,7 +108,11 @@ export async function uploadCreatives(page, config, files, options = {}) { // Шаг 7 — уходим со страницы. Кнопку «Сохранить изменения» (SaveBannerButton) не // трогаем НИКОГДА: она единственная на этом пути меняет кабинет. Черновик объявления // никуда не сохраняется. - await page.goto(overviewUrl(config), { waitUntil: 'domcontentloaded' }); + // + // Осечку тут глотаем намеренно: работа уже сделана — файлы приняты, «Создать» нажата. + // Сетевая икота на уборке за собой не должна выдаваться за «креативы не загрузились», + // иначе портал пометит задание сбойным, а креативы останутся лежать в кабинете. + await page.goto(overviewUrl(config), { waitUntil: 'domcontentloaded' }).catch(() => {}); return { modalClosed, cabinetSaid }; } diff --git a/bots/yandex-creatives/src/config.js b/bots/yandex-creatives/src/config.js index b731b192..69e3a718 100644 --- a/bots/yandex-creatives/src/config.js +++ b/bots/yandex-creatives/src/config.js @@ -30,5 +30,8 @@ export function loadConfig(env = process.env) { alarmTo: required(env, 'ALARM_TO'), // Человекоподобный темп: кабинет не должен видеть машинную скорость. humanDelayMs: Number(env.HUMAN_DELAY_MS ?? '800'), + // Пауза между повторами доклада порталу «готово». Доклад повторяется, потому что + // обрыв на нём стоит дорого: креативы уже в кабинете, а портал об этом не знает. + reportRetryDelayMs: Number(env.REPORT_RETRY_DELAY_MS ?? '3000'), }; } diff --git a/bots/yandex-creatives/src/portal.js b/bots/yandex-creatives/src/portal.js index 30defac4..fb4d1d4d 100644 --- a/bots/yandex-creatives/src/portal.js +++ b/bots/yandex-creatives/src/portal.js @@ -7,6 +7,20 @@ import { pipeline } from 'node:stream/promises'; /** * Разговор робота с порталом. fetch передаётся снаружи — так его можно подменить в тестах. */ +/** + * Портал принимает причину сбоя не длиннее 1024 знаков. Берём с запасом: сообщения + * Playwright при таймауте штатно тянут за собой «Call log:» на десятки строк, и отчёт + * о САМОМ частом виде сбоя портал отверг бы целиком. Отчёт не принят → задание навсегда + * «в работе» → очередь встаёт колом для всех клиентов. + */ +const REASON_LIMIT = 900; + +function shortenReason(reason) { + const text = String(reason ?? ''); + + return text.length <= REASON_LIMIT ? text : `${text.slice(0, REASON_LIMIT - 1)}…`; +} + export function createPortal(config, fetchImpl = fetch) { const headers = { 'X-Creative-Robot-Token': config.robotToken, Accept: 'application/json' }; @@ -44,7 +58,7 @@ export function createPortal(config, fetchImpl = fetch) { return call(`/api/creative-robot/jobs/${jobId}/done`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ ok: false, reason }), + body: JSON.stringify({ ok: false, reason: shortenReason(reason) }), }); }, }; diff --git a/bots/yandex-creatives/src/runner.js b/bots/yandex-creatives/src/runner.js index 5d86726a..1cd15af8 100644 --- a/bots/yandex-creatives/src/runner.js +++ b/bots/yandex-creatives/src/runner.js @@ -27,13 +27,14 @@ export async function runOnce(config, portal, browser, mailer, { timestamp, work let context; let page; let step = 'начало'; + let files = []; + let said = null; try { mkdirSync(shotsDir, { recursive: true }); mkdirSync(dir, { recursive: true }); step = 'скачивание файлов'; - const files = []; for (const banner of job.banners) { files.push(await portal.downloadBanner(banner, join(dir, `${banner.width}x${banner.height}.jpg`))); } @@ -48,41 +49,36 @@ export async function runOnce(config, portal, browser, mailer, { timestamp, work step = 'загрузка креативов'; // В кабинет уходят ПУТИ к файлам: их кладут прямо в поле файлов на странице. - const said = await browser.uploadCreatives(page, config, files); - - // Отчитываемся «готово» даже если кабинет повёл себя непривычно: правду об успехе - // знает портал по слепку креативов «до/после», а не робот по экрану. Соврать «сбой» - // при удачной загрузке хуже всего: задание умрёт, а креативы в кабинете останутся. - await portal.reportDone(job.id); - try { - await mailer.report({ campaignId: job.campaign_id, count: files.length }); - } catch { /* письмо — не повод считать работу проваленной */ } - - if (said && said.modalClosed === false) { - // Человека всё равно зовём: окно осталось открытым, и мы хотим увидеть, что - // именно написал кабинет — этого мы ещё ни разу не видели живьём. - try { - await mailer.alarm({ - step, - reason: `Окно загрузки не закрылось. Кабинет сказал: ${said.cabinetSaid || 'ничего не написал'}`, - campaignId: job.campaign_id, - screenshotPath: null, - }); - } catch { /* см. выше */ } - } - - return { ok: true, jobId: job.id, count: files.length }; + // + // 🔴 На этом работа робота заканчивается. Доклад «готово» вынесен НИЖЕ, за пределы + // этого try: его обработчик шлёт порталу «сбой», и обрыв на самом докладе объявлял + // бы провалом СВОЮ УДАЧНУЮ работу — задание умирало бы, а креативы оставались бы + // в кабинете. А если портал успел принять «готово» и потерялся только ответ, «сбой» + // затирал бы правильный результат с уже проставленными номерами креативов. + said = await browser.uploadCreatives(page, config, files); } catch (e) { try { await page?.screenshot({ path: screenshot, fullPage: true }); } catch { /* снимок — не обязателен */ } // Отчёт порталу — ПЕРВЫМ: даже если письмо не уйдёт, задание не зависнет «в работе» // и очередь не встанет колом. - try { await portal.reportFailure(job.id, `${step}: ${e.message}`); } catch { /* портал недоступен */ } + // + // Провал самого отчёта раньше глотался молча — а это худший из исходов: задание + // остаётся «в работе» навсегда, выдача заданий отвечает «работы нет» ВСЕМ клиентам, + // и человек об этом не узнаёт. Теперь причина провала едет в письмо-алярм. + let reportError = null; + try { + await portal.reportFailure(job.id, `${step}: ${e.message}`); + } catch (re) { + reportError = re; + } try { await mailer.alarm({ step, - reason: e.message, + reason: reportError === null + ? e.message + : `${e.message}\n\nПорталу доложить не смог: ${reportError.message}. ` + + `Задание #${job.id} могло остаться «в работе» — проверьте очередь заданий.`, campaignId: job.campaign_id, screenshotPath: existsSync(screenshot) ? screenshot : null, }); @@ -95,4 +91,68 @@ export async function runOnce(config, portal, browser, mailer, { timestamp, work try { rmSync(dir, { recursive: true, force: true }); } catch { /* уже нет */ } try { await context?.close(); } catch { /* уже закрыт */ } } + + // Сюда попадаем ТОЛЬКО когда креативы уже в кабинете. Дальше — один лишь доклад. + const reportError = await reportDoneWithRetries(portal, job.id, config); + + if (reportError !== null) { + // 🔴 «Сбой» тут не докладываем НИКОГДА: работа сделана, врать о ней нельзя. Зовём + // человека письмом — пусть посмотрит кабинет и очередь заданий руками. + try { + await mailer.alarm({ + step: 'доклад порталу', + reason: `Креативы загружены в кабинет, но доложить об этом порталу не удалось: ${reportError.message}. ` + + `Задание #${job.id} могло остаться «в работе». Файлы в кабинете УЖЕ ЕСТЬ — повторная заливка оставит дубли.`, + campaignId: job.campaign_id, + screenshotPath: null, + }); + } catch { /* почта недоступна */ } + + return { ok: true, jobId: job.id, count: files.length, reportedToPortal: false }; + } + + try { + await mailer.report({ campaignId: job.campaign_id, count: files.length }); + } catch { /* письмо — не повод считать работу проваленной */ } + + if (said && said.modalClosed === false) { + // Человека всё равно зовём: окно осталось открытым, и мы хотим увидеть, что + // именно написал кабинет — этого мы ещё ни разу не видели живьём. + try { + await mailer.alarm({ + step: 'загрузка креативов', + reason: `Окно загрузки не закрылось. Кабинет сказал: ${said.cabinetSaid || 'ничего не написал'}`, + campaignId: job.campaign_id, + screenshotPath: null, + }); + } catch { /* см. выше */ } + } + + return { ok: true, jobId: job.id, count: files.length, reportedToPortal: true }; +} + +/** + * Доклад «готово» с повторами. Возвращает null при успехе либо последнюю ошибку. + * + * Повторяем, потому что на этом шаге терять нечего: портал принимает «готово» по заданию + * только пока оно в работе, повторный доклад по уже закрытому он отвергает сам. + */ +async function reportDoneWithRetries(portal, jobId, config, attempts = 3) { + const pauseMs = Number(config?.reportRetryDelayMs ?? 3000); + let last = null; + + for (let i = 1; i <= attempts; i += 1) { + try { + await portal.reportDone(jobId); + + return null; + } catch (e) { + last = e; + if (i < attempts && pauseMs > 0) { + await new Promise((resolve) => { setTimeout(resolve, pauseMs); }); + } + } + } + + return last; } diff --git a/bots/yandex-creatives/test/cabinet.test.js b/bots/yandex-creatives/test/cabinet.test.js index e3849de1..a85f979b 100644 --- a/bots/yandex-creatives/test/cabinet.test.js +++ b/bots/yandex-creatives/test/cabinet.test.js @@ -17,7 +17,7 @@ const FAST = { acceptTimeoutMs: 60, pollMs: 5 }; * Поддельная страница: записывает всё, что робот делал, и умеет притворяться, * что кнопка «Создать» разблокировалась не сразу, а окно закрылось или нет. */ -function fakePage({ enabledAfter = 1, modalCloses = true, modalText = '' } = {}) { +function fakePage({ enabledAfter = 1, modalCloses = true, modalText = '', leavingThrows = false } = {}) { const actions = []; let disabledAsked = 0; @@ -25,7 +25,11 @@ function fakePage({ enabledAfter = 1, modalCloses = true, modalText = '' } = {}) actions, clicked: () => actions.filter((a) => a.type === 'click').map((a) => a.selector), asked: () => disabledAsked, - async goto(url) { actions.push({ type: 'goto', url }); }, + async goto(url) { + actions.push({ type: 'goto', url }); + // Уход со страницы после «Создать» — последний шаг; форму открывали раньше. + if (leavingThrows && !url.includes('banners-edit')) throw new Error('сеть икнула на уходе'); + }, locator(selector) { return { async waitFor(options = {}) { @@ -116,6 +120,18 @@ test('окно не закрылось — НЕ объявляем провал, assert.equal(page.clicked().includes('[data-testid="SaveBannerButton"]'), false); }); +test('сетевая икота на уходе со страницы не считается провалом загрузки', async () => { + // Файлы уже приняты, «Создать» нажата — работа сделана. Уход со страницы это уборка + // за собой, и её осечка не должна выдаваться за «креативы не загрузились»: портал + // пометил бы задание сбойным, а креативы остались бы в кабинете. + const page = fakePage({ leavingThrows: true }); + + const result = await uploadCreatives(page, config, FILES, FAST); + + assert.equal(result.modalClosed, true); + assert.equal(page.clicked().includes('[data-testid="SaveBannerButton"]'), false); +}); + test('пустой список файлов — сразу отказ, в кабинет не ходим', async () => { const page = fakePage(); diff --git a/bots/yandex-creatives/test/config.test.js b/bots/yandex-creatives/test/config.test.js index 9050168d..4b684973 100644 --- a/bots/yandex-creatives/test/config.test.js +++ b/bots/yandex-creatives/test/config.test.js @@ -21,6 +21,11 @@ test('загружает полный конфиг', () => { assert.equal(c.humanDelayMs, 800); }); +test('знает паузу между повторами доклада порталу', () => { + assert.equal(loadConfig(full).reportRetryDelayMs, 3000); + assert.equal(loadConfig({ ...full, REPORT_RETRY_DELAY_MS: '500' }).reportRetryDelayMs, 500); +}); + test('падает с понятным сообщением, если нет обязательной переменной', () => { const { CREATIVE_ROBOT_TOKEN, ...without } = full; assert.throws(() => loadConfig(without), /CREATIVE_ROBOT_TOKEN/); diff --git a/bots/yandex-creatives/test/portal.test.js b/bots/yandex-creatives/test/portal.test.js index 656fa14b..5d289167 100644 --- a/bots/yandex-creatives/test/portal.test.js +++ b/bots/yandex-creatives/test/portal.test.js @@ -94,3 +94,23 @@ test('отчитывается о сбое с причиной', async () => { assert.equal(body.ok, false); assert.equal(body.reason, 'вход слетел'); }); + +test('слишком длинную причину обрезает — иначе портал отвергнет отчёт и очередь встанет колом', async () => { + // Портал принимает причину не длиннее 1024 знаков. Playwright при таймауте штатно + // вываливает «Call log:» на десятки строк — то есть САМЫЙ частый вид сбоя и есть тот, + // чей отчёт портал отверг бы. Отчёт не принят → задание навсегда «в работе» → выдача + // заданий возвращает «работы нет» ВСЕМ, ни одна кампания больше не стартует. + const calls = []; + const fetchStub = async (url, opts) => { + calls.push({ url, opts }); + + return { ok: true, status: 200, json: async () => ({ status: 'failed' }) }; + }; + + await createPortal(config, fetchStub).reportFailure(7, `загрузка креативов: ${'ц'.repeat(5000)}`); + + const body = JSON.parse(calls[0].opts.body); + assert.ok(body.reason.length <= 900, `причина ушла длиной ${body.reason.length}`); + assert.match(body.reason, /^загрузка креативов: цц/, 'начало причины должно уцелеть'); + assert.ok(body.reason.endsWith('…'), 'обрезку надо показать человеку'); +}); diff --git a/bots/yandex-creatives/test/runner.test.js b/bots/yandex-creatives/test/runner.test.js index 81beb7d4..39732797 100644 --- a/bots/yandex-creatives/test/runner.test.js +++ b/bots/yandex-creatives/test/runner.test.js @@ -68,11 +68,14 @@ function stubs({ job = null, uploadThrows = null, uploadResult = { modalClosed: const opts = (workDir) => ({ timestamp: 't', workDir }); +// Без пауз: в проверках ждать нечего, а повтор доклада порталу иначе тянул бы секунды. +const FAST_CONFIG = { humanDelayMs: 0, reportRetryDelayMs: 0 }; + test('без работы ничего не делает и браузер не открывает', async () => { await withWorkDir(async (workDir) => { const s = stubs({ job: null }); - const res = await runOnce({ humanDelayMs: 0 }, s.portal, s.browser, s.mailer, opts(workDir)); + const res = await runOnce(FAST_CONFIG, s.portal, s.browser, s.mailer, opts(workDir)); assert.equal(res.idle, true); assert.equal(s.opened(), 0); @@ -84,7 +87,7 @@ test('успешная загрузка отчитывается «готово await withWorkDir(async (workDir) => { const s = stubs({ job: JOB }); - const res = await runOnce({ humanDelayMs: 0 }, s.portal, s.browser, s.mailer, opts(workDir)); + const res = await runOnce(FAST_CONFIG, s.portal, s.browser, s.mailer, opts(workDir)); assert.equal(res.ok, true); assert.deepEqual(s.reports, [{ ok: true, id: 7 }]); @@ -97,7 +100,7 @@ test('в кабинет уходят ПУТИ к файлам, а не опис await withWorkDir(async (workDir) => { const s = stubs({ job: JOB }); - await runOnce({ humanDelayMs: 0 }, s.portal, s.browser, s.mailer, opts(workDir)); + await runOnce(FAST_CONFIG, s.portal, s.browser, s.mailer, opts(workDir)); const given = s.uploads[0]; assert.equal(given.length, 2); @@ -111,7 +114,7 @@ test('файлы клиента живут в рабочей папке робо await withWorkDir(async (workDir) => { const s = stubs({ job: JOB }); - await runOnce({ humanDelayMs: 0 }, s.portal, s.browser, s.mailer, opts(workDir)); + await runOnce(FAST_CONFIG, s.portal, s.browser, s.mailer, opts(workDir)); assert.ok(s.downloaded.every((p) => p.startsWith(workDir)), `файлы легли мимо: ${s.downloaded[0]}`); }); @@ -121,7 +124,7 @@ test('файлы клиента удаляются при любом исход await withWorkDir(async (workDir) => { const s = stubs({ job: JOB, uploadThrows: 'кабинет упал' }); - await runOnce({ humanDelayMs: 0 }, s.portal, s.browser, s.mailer, opts(workDir)); + await runOnce(FAST_CONFIG, s.portal, s.browser, s.mailer, opts(workDir)); assert.equal(existsSync(join(workDir, 'downloads', 'job-7')), false); assert.equal(s.closed(), 1, 'браузер должен закрыться даже при сбое'); @@ -132,7 +135,7 @@ test('сбой загрузки отчитывается сбоем и шлёт await withWorkDir(async (workDir) => { const s = stubs({ job: JOB, uploadThrows: 'кнопка не найдена' }); - const res = await runOnce({ humanDelayMs: 0 }, s.portal, s.browser, s.mailer, opts(workDir)); + const res = await runOnce(FAST_CONFIG, s.portal, s.browser, s.mailer, opts(workDir)); assert.equal(res.ok, false); assert.equal(s.reports[0].ok, false); @@ -145,7 +148,7 @@ test('слетевший вход не пытается грузить и отч await withWorkDir(async (workDir) => { const s = stubs({ job: JOB, loggedIn: false }); - const res = await runOnce({ humanDelayMs: 0 }, s.portal, s.browser, s.mailer, opts(workDir)); + const res = await runOnce(FAST_CONFIG, s.portal, s.browser, s.mailer, opts(workDir)); assert.equal(res.ok, false); assert.equal(s.uploads.length, 0); @@ -159,7 +162,7 @@ test('окно не закрылось — докладываем «готово // остаться открытым и после удачной загрузки. Но человек должен это увидеть. const s = stubs({ job: JOB, uploadResult: { modalClosed: false, cabinetSaid: 'Файл слишком большой' } }); - const res = await runOnce({ humanDelayMs: 0 }, s.portal, s.browser, s.mailer, opts(workDir)); + const res = await runOnce(FAST_CONFIG, s.portal, s.browser, s.mailer, opts(workDir)); assert.equal(res.ok, true); assert.deepEqual(s.reports, [{ ok: true, id: 7 }]); @@ -167,12 +170,72 @@ test('окно не закрылось — докладываем «готово }); }); +test('обрыв на докладе «готово» НЕ превращается в отчёт о сбое — креативы уже в кабинете', async () => { + await withWorkDir(async (workDir) => { + // Приговор об успехе выносит портал по слепку креативов. Если доклад «готово» не дошёл + // (502 от nginx, обрыв, таймаут), робот раньше падал в общий обработчик сбоя и слал + // «сбой» — на СВОЮ УДАЧНУЮ работу. Портал помечал задание сбойным, а креативы уже лежали + // в кабинете. Хуже того: если портал успел принять «готово», а ответ потерялся, «сбой» + // затирал правильный результат с уже проставленными номерами. + const s = stubs({ job: JOB }); + s.portal.reportDone = async () => { throw new Error('Портал ответил 502 на /done'); }; + + const res = await runOnce(FAST_CONFIG, s.portal, s.browser, s.mailer, opts(workDir)); + + assert.equal( + s.reports.filter((r) => r.ok === false).length, + 0, + `доложен сбой об удачной работе: ${JSON.stringify(s.reports)}`, + ); + const alarm = s.sent.find((m) => m.kind === 'alarm'); + assert.ok(alarm, 'человека надо позвать письмом'); + assert.match(alarm.reason, /502/); + assert.equal(res.ok, true, 'работа сделана: креативы в кабинете'); + }); +}); + +test('доклад «готово» повторяется, если с первого раза не прошёл', async () => { + await withWorkDir(async (workDir) => { + const s = stubs({ job: JOB }); + let tries = 0; + s.portal.reportDone = async (id) => { + tries += 1; + if (tries < 3) throw new Error('обрыв связи'); + s.reports.push({ ok: true, id }); + }; + + const res = await runOnce(FAST_CONFIG, s.portal, s.browser, s.mailer, opts(workDir)); + + assert.equal(tries, 3, `доклад пробовали ${tries} раз(а)`); + assert.deepEqual(s.reports, [{ ok: true, id: 7 }]); + assert.equal(res.ok, true); + }); +}); + +test('портал не принял отчёт о сбое — человек узнаёт об этом из письма', async () => { + await withWorkDir(async (workDir) => { + // Раньше провал отчёта глотался молча. А это самый опасный исход: задание остаётся + // «в работе» навсегда, очередь встаёт колом для ВСЕХ клиентов, и никто об этом + // не знает — в письме написана только исходная причина сбоя. + const s = stubs({ job: JOB, uploadThrows: 'кнопка не найдена' }); + s.portal.reportFailure = async () => { throw new Error('Портал ответил 422 на /done'); }; + + const res = await runOnce(FAST_CONFIG, s.portal, s.browser, s.mailer, opts(workDir)); + + assert.equal(res.ok, false); + const alarm = s.sent.find((m) => m.kind === 'alarm'); + assert.ok(alarm, 'письмо-алярм должно уйти'); + assert.match(alarm.reason, /кнопка не найдена/, 'исходная причина должна остаться'); + assert.match(alarm.reason, /422/, 'провал отчёта порталу должен быть виден человеку'); + }); +}); + test('письмо не ушло — задание всё равно закрыто, робот не зависает', async () => { await withWorkDir(async (workDir) => { const s = stubs({ job: JOB, uploadThrows: 'кабинет упал' }); s.mailer.alarm = async () => { throw new Error('почта недоступна'); }; - const res = await runOnce({ humanDelayMs: 0 }, s.portal, s.browser, s.mailer, opts(workDir)); + const res = await runOnce(FAST_CONFIG, s.portal, s.browser, s.mailer, opts(workDir)); assert.equal(res.ok, false); assert.equal(s.reports[0].ok, false); diff --git a/docs/superpowers/2026-07-27-PROGRESS-pochinka-v12.md b/docs/superpowers/2026-07-27-PROGRESS-pochinka-v12.md index 52f66ffd..3816269b 100644 --- a/docs/superpowers/2026-07-27-PROGRESS-pochinka-v12.md +++ b/docs/superpowers/2026-07-27-PROGRESS-pochinka-v12.md @@ -5,12 +5,12 @@ Обновлено: 27.07.2026, работа идёт. -Р1, Р2 и половина Р3 закоммичены — `d26716ed`. Остальное (вторая половина Р3, Р4, Р5) — +Р1–Р5 закоммичены — `d26716ed` и `b8f75b2a`. Р6, Р7, Р8 — **не закоммичено**. -## Зелёная отметка на момент остановки +## Зелёная отметка после Р1–Р5 (коммит `b8f75b2a`) -Прогнано в одиночку, ничего параллельно: +Прогнано в одиночку, ничего параллельно. Свежая отметка после Р6–Р8 — ниже, в конце файла. - портал: `cd app && DB_DATABASE=liderra_testing_reklama php artisan test --filter=Advertising` → **266/266, 936 проверок, ~44 с** (было 247/247 до починки — добавилось 19 новых тестов); @@ -56,7 +56,7 @@ «готово» → 409 и номер креатива цел; «сбой» поверх закрытого → 409; отчёт по уже `failed` → 409 и первая причина цела). Все четыре были красными до правки. -### Р3 — гонка выдачи заданий ⚠️ сделано наполовину +### Р3 — гонка выдачи заданий ✅ Сделано: @@ -136,9 +136,82 @@ «не писать захват» выживал. Настоящую проверку дало подглядывание изнутри запуска — заглушка Яндекса читает статус кампании в базе в момент, когда запуск уже идёт. +### Р6 — очередь робота встаёт колом навсегда ✅ + +Три части, все три сделаны. + +1. **Робот, обрезка причины.** `portal.js` — `reportFailure()` режет причину до 900 знаков + с многоточием в конце. Портал принимает не длиннее 1024, а сообщения Playwright при + таймауте штатно тянут «Call log:» на десятки строк — то есть отчёт о САМОМ частом виде + сбоя портал отвергал бы целиком. +2. **Робот, видимый провал отчёта.** `runner.js` — ошибка `reportFailure` больше не глотается + молча: её текст едет в письмо-алярм вместе с предупреждением «задание могло остаться + в работе». +3. **Портал, `done()` ловит `Throwable`.** Приём отчёта ходит в живой Яндекс; любая ошибка + API раньше улетала наружу (робот получал 500), и задание навсегда оставалось `taken`. + Теперь — запись в журнал + `failed` с причиной + 200 роботу. +4. **Реаниматор.** Новая команда `creative-jobs:reap` + (`app/app/Console/Commands/ReapStuckCreativeJobs.php`): `taken` старше **30 минут** + возвращается в `queued`, после **3** попыток закрывается `failed`. В расписании каждые + 10 минут (`app/routes/console.php`). 🔴 Ходит через `pgsql_admin` — на дефолтной роли + без tenant-контекста RLS отдал бы ноль строк, и сторож рапортовал бы об успехе, ничего + не разбирая. + +Тесты: 2 новых у робота (`portal.test.js`, `runner.test.js`), 1 в `CreativeRobotEndpointTest.php`, +новый файл `CreativeJobReaperTest.php` (4 теста) + 1 в `AdvertisingScheduleTest.php`. +**Проверено вырезанием:** обрезка причины, захват ошибки отчёта, порог времени сторожа, +лимит попыток сторожа, пометка задания сбойным в `done()` — краснеет каждая. + +### Р7 — робот докладывал «сбой» об успешной работе ✅ + +Обрыв на докладе «готово» (502 от nginx, таймаут) уходил в общий обработчик сбоя, и робот +слал порталу «сбой» — на СВОЮ УДАЧНУЮ работу. Задание умирало, креативы оставались в кабинете. +Хуже: если портал успел принять «готово», а ответ потерялся, «сбой» затирал правильный +результат с уже проставленными номерами. + +- `runner.js` перестроен: `try` заканчивается на `uploadCreatives`, доклад вынесен ЗА него — + под обработчик сбоя он больше не попадает физически; +- доклад повторяется **3 раза** (`reportDoneWithRetries`), пауза между попытками — + `REPORT_RETRY_DELAY_MS`, по умолчанию 3000 мс (новый ключ в `config.js` и `.env.example`); +- не прошло и после повторов — **только письмо человеку**, `reportFailure` не зовётся никогда; +- `cabinet.js` — уход со страницы после «Создать» обёрнут `.catch(() => {})`: работа уже + сделана, сетевая икота на уборке за собой не должна выдаваться за «креативы не загрузились». + +Тесты: 2 в `runner.test.js`, 1 в `cabinet.test.js`, 1 в `config.test.js`. +**Проверено вырезанием:** число повторов, глушение осечки ухода и сам вынос доклада +(возврат старого поведения «докладываем сбой») — краснеет каждое. + +### Р8 — бесконечная заливка мусора в живой кабинет ✅ + +Пустой список ожидаемых размеров `match()` молча считал успехом: цикл не выполнялся, +возвращалась пустота. Пустым он становится сам собой — если после выката не перезапустить +`db/03_service_bypass_policies.sql`, служебная роль не увидит ни одного баннера. Дальше +задание помечалось «готово» с нулём номеров, запуск снова видел баннеры без креативов и +ставил новое задание — робот заливал те же файлы по кругу, оставляя каждый раз пачку +мусорных креативов в живом кабинете. В журнале всё зелёное. + +- `CreativeIdMatcher::match()` — бросает `CreativeMatchFailedException` при пустом списке + ожидаемых размеров, текст прямо называет вероятную причину; +- `CreativeJobService::complete()` — та же проверка по баннерам **до** похода в Яндекс, + задание закрывается сбойным (вынесен общий `markFailed()`). + +Тесты: 2 в `CreativeJobServiceTest.php`, второй проверяет `Http::assertNothingSent()` — +в Яндекс не ходили вовсе. **Проверено вырезанием:** обе защиты, краснеет каждая. + +## Зелёная отметка после Р1–Р8 + +- портал: **274/274, 957 проверок, ~45 с**; +- робот: **41/41**; +- вызовов `AdWalletService->release(` в коде по-прежнему **четыре** — пятого не появилось + (остальные `->release()` в репозитории — снятие замков `Cache::lock`, не деньги). + ## Не начато -Р6, Р7, Р8 — по листу v12 §6. Хвосты П1–П8, Д1–Д6, Р-х1–Р-х6 — §7. Мелочи — §8. +Хвосты П1–П8, Д1–Д6, Р-х1–Р-х6 — лист v12 §7. Мелочи — §8. + +Побочно закрыта половина **Р-х6**: ключ `REPORT_RETRY_DELAY_MS` добавлен сразу в правильной +форме, но `HUMAN_DELAY_MS` и `SMTP_PORT` по-прежнему с той же миной (`??` ловит только +`undefined`, пустая строка в `.env` даёт 0 и `NaN`) — чинить в Р-х6. Отдельно на будущее (из отчёта `rls-reviewer`, в лист v12 не входило): у `crm_app_user` табличный `UPDATE` на `ad_creative_jobs` без ограничения по колонкам. Сейчас безвредно — @@ -156,3 +229,10 @@ v9.07. Чинить не в этой ветке. а `git checkout` использовать только если файл в коммите уже правильный. - 🪤 `->after('колонка')` в миграции на PostgreSQL — **no-op**, колонка встаёт в конец таблицы. В проекте так пишут все соседние миграции; в CHANGELOG формулировку уточнил. +- 🔑 Вместо `git checkout` теперь копирую файл в scratchpad перед мутацией и возвращаю + оттуда — правка цела, мутация снята, лишних движений нет. +- 🔑 `pgsql_admin` в тестах **виден**: `tests/Concerns/SharesAdminPdo.php` подключён глобально + к `Feature` (`tests/Pest.php`), обе connection делят один PDO. Поэтому команду сторожа + можно честно прибить к `pgsql_admin`, не подкладывая ей соединение ради тестов. + 🪤 Обратная сторона: мутация «сменить соединение на дефолтное» в тестах НЕ покраснеет — + это чисто боевая защита, держится на комментарии и на знании про RLS.