diff --git a/app/Actions/Scrape/ScrapeAction.php b/app/Actions/Scrape/ScrapeAction.php index 56e4e42..a2a84e2 100644 --- a/app/Actions/Scrape/ScrapeAction.php +++ b/app/Actions/Scrape/ScrapeAction.php @@ -17,8 +17,22 @@ public function __invoke(?SiteName $siteName, LoggerInterface $logger): void { $siteNames = $siteName instanceof SiteName ? [$siteName] : SiteName::cases(); - foreach ($this->handlerFactory->create($siteNames) as $handler) { - $handler($logger); + $failure = null; + + foreach ($this->handlerFactory->create($siteNames) as $index => $handler) { + try { + $handler($logger); + } catch (\Throwable $th) { + $logger->error('site failed', [$siteNames[$index]->value, $th]); + $failure ??= $th; + } + } + + // 全サイトを試行した後で改めて投げ直す。呼び出し元(ScrapeCommand)の + // report()/終了コード/last_crawl 更新スキップが、1サイトの失敗でも + // 引き続き働くようにするため。 + if ($failure instanceof \Throwable) { + throw $failure; } } } diff --git a/docs/known-risks.md b/docs/known-risks.md index bcec316..1b6bc07 100644 --- a/docs/known-risks.md +++ b/docs/known-risks.md @@ -12,7 +12,7 @@ | ID | 保護すべき挙動 / Expected Outcome | 制御 | テスト | Status | 是正条件 | 記録日 | |----|----------------------------------|------|--------|--------|----------|--------| -| A1 | 1 サイト/1URL の失敗で他サイト・他 URL の処理が止まらない | Preventive（ハンドラ毎の try/catch） | 2（`Extract/Japan/HandlerIsolationTest` + `Scrape/Japan/HandlerIsolationTest`） | 🟢 OK | — | 2026-06-22 | +| A1 | 1 サイト/1URL の失敗で他サイト・他 URL の処理が止まらない | Preventive（ハンドラ毎の try/catch + `ScrapeAction` でのサイト毎 try/catch。全サイト試行後、失敗があれば1件だけ再送出し、呼び出し元(`ScrapeCommand`)の `report()`/終了コード/`last_crawl` 更新スキップを維持） | 3（`Extract/Japan/HandlerIsolationTest` + `Scrape/Japan/HandlerIsolationTest` + `Scrape/ScrapeActionTest::test_one_site_failure_does_not_stop_other_sites`） | 🟢 OK | — | 2026-06-22（2026-08-20 是正: `ScrapeAction` に一覧取得段階の失敗が他サイトを巻き込む欠落を発見、try/catch追加。当初案はDiscord通知/`last_crawl`更新スキップも巻き添えで失っていたため、全サイト試行後に再送出する形に修正。`ExtractAction` にも同型の欠落が残存、未対応） | | A2 | HTTP 失敗時に RawPage を空/部分 HTML で上書きしない | Preventive（`FetchHtml` で `Http::get()->throw()`。非2xx も例外として扱い upsert に到達しない。4xx は解消しないため即失敗、5xx/接続エラーのみ `retry()` で再試行） | 5（`FetchHtmlTest::test_throws_on_non_2xx...` + `test_does_not_retry_on_4xx...` + `test_retries_on_5xx...` + `Scrape/Japan/HandlerFailureTest`×2） | 🟢 OK | — | 2026-06-22 | | A3 | 同一 RawPage に extract を再実行しても Page 重複が出ない（冪等） | Preventive（`pages_url_unique` + HasOne） | 2（`Extract/UpdateOrCreatePageTest`） | 🟢 OK | — | 2026-06-22 | | A4 | 抽出失敗時にスクレイプ済データ（RawPage）を破壊しない | Preventive（`MarkExtractFailed`：削除せず `extract_failed_at` で隔離、成功時にクリア。`extract_failed_at` に index 追加済み） | 4（`MarkExtractFailedTest`×3 + `HandlerIsolationTest`） | 🟢 OK | — | 2026-06-22 | diff --git a/tests/Feature/Actions/Scrape/ScrapeActionTest.php b/tests/Feature/Actions/Scrape/ScrapeActionTest.php index a73d3d6..02b02b5 100644 --- a/tests/Feature/Actions/Scrape/ScrapeActionTest.php +++ b/tests/Feature/Actions/Scrape/ScrapeActionTest.php @@ -9,6 +9,7 @@ use App\Actions\Scrape\ScrapeAction; use App\Enums\SiteName; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Http\Client\ConnectionException; use Illuminate\Support\Facades\Http; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; @@ -51,4 +52,33 @@ public function test_invokes_specific_handler_when_site_provided(): void Http::assertSent(fn ($request): bool => str_contains((string) $request->url(), 'japanese.simutrans.com')); Http::assertNotSent(fn ($request): bool => str_contains((string) $request->url(), 'wikiwiki.jp/twitrans')); } + + public function test_one_site_failure_does_not_stop_other_sites(): void + { + Http::fake([ + 'https://japanese.simutrans.com?cmd=list' => fn (): never => throw new ConnectionException('connection failed'), + '*' => Http::response('', 200), + ]); + + // Mock PortalHandler to avoid database queries in CI where the portal connection is unmigrated + $this->app->bind(Handler::class, fn (): HandlerInterface => new class implements HandlerInterface + { + public function __invoke(LoggerInterface $logger): void {} + }); + + $scrapeAction = app(ScrapeAction::class); + + // Japan's list fetch fails entirely, but Twitrans must still run before + // the failure is rethrown so the caller (ScrapeCommand) can still + // report()/exit non-zero/skip the last_crawl cache update. + try { + $scrapeAction(null, new NullLogger); + $this->fail('Expected ConnectionException was not thrown.'); + } catch (ConnectionException) { + // expected + } + + Http::assertNotSent(fn ($request): bool => str_contains((string) $request->url(), 'japanese.simutrans.com/index.php')); + Http::assertSent(fn ($request): bool => str_contains((string) $request->url(), 'wikiwiki.jp/twitrans')); + } }