fix(puppeteer-page-scan): finish closing the post-reload detached-Frame race#883
Merged
Merged
Conversation
…red helper module The inline retry helper used inside `scrollAllOver` is also needed in `@d-zero/puppeteer-page-scan` to absorb the same Chrome main-frame swap window around `page.reload()` resolution. Move it to a dedicated module and export it from the package root so callers can share the same retry shape (3 attempts × 200 ms gap, scoped to transient frame/session errors) instead of duplicating the loop and the error-matching regex.
…detached-Frame errors The `document.body.scrollHeight` read that runs immediately after `page.reload()` could still throw "Attempted to use detached Frame" on rare runs (observed in production on a responsive data-table page). The reload resolves before Chrome's internal main-frame swap is fully complete, so a single read can land inside the swap window. The follow-up `scrollAllOver` already retries its own evaluates, but a throw at this point escapes `beforePageScan` before that retry layer can run. Wrap the read with `evaluateWithFrameRetry` from `@d-zero/puppeteer-scroll`, the same helper used internally by `scrollAllOver`, so the two layers share one retry shape (3 attempts × 200 ms gap).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
#882 のフォローアップ。 nitpicker の本番運用で、稀に
mobile-small: skipped — Attempted to use detached Frameがまだ残ることが確認された経路を塞ぐ。Root cause
前回までの修正で 2 つの経路を塞いだが、 もう一つ未対応の経路があった:
scrollAllOver内のpage.evaluate(scroll step / 初期 scrollHeight / 末尾 scrollTo)#fetchDataの 3 min timeout が#fetchImages(20 min) を内包せず racebeforePageScan内のpage.evaluate(() => document.body.scrollHeight)(reload 直後、 scrollAllOver の前)page.reload()が resolve した直後でも Chrome の internal main-frame swap が完了していない瞬間があり、 そこで実行されたpage.evaluateが detached Frame を投げる。 一度投げるとbeforePageScan全体が reject →scrollAllOverの retry レイヤーに到達する前に per-device catch でスキップ。Changes
@d-zero/puppeteer-scrollevaluateWithFrameRetry/isTransientFrameErrorを inline ヘルパーから独立モジュールevaluate-with-frame-retry.tsに切り出し、 package root から export@d-zero/puppeteer-page-scanbefore-page-scan.tsの post-reloaddocument.body.scrollHeight読み取りをevaluateWithFrameRetryでラップTest plan
yarn test(52 tests in scroll + page-scan、合計 783 tests pass)yarn lint(cspell pass)yarn build(27 projects pass)Verification (本番運用への反映後)
mobile-small: skipped — Attempted to use detached Frameの出現率を確認 (前回は ~5-10% 程度の頻度で残っていた経路)🤖 Generated with Claude Code