fix(tradein/browser): цикл ожидания челленджа падал ровно на успешном исходе
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI Trade-In / backend-tests (pull_request) Has been skipped
CI / changes (pull_request) Successful in 10s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / browser-tests (pull_request) Successful in 53s
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI Trade-In / backend-tests (pull_request) Has been skipped
CI / changes (pull_request) Successful in 10s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / browser-tests (pull_request) Successful in 53s
Найдено при ревью ветки. `_wait_out_pow_challenge` опрашивал `page.content()`
без защиты, а челлендж перезагружает страницу САМ (`window.location =
location.href`). Вызов content(), попавший в момент этой перезагрузки, кидает
«Execution context was destroyed, most likely because of a navigation» — то
есть цикл ронял фетч ровно тогда, когда проверка успешно пройдена и мы
дождались того, ради чего ждали.
На моках дефект не воспроизводился: поддельная page навигацию не рвёт.
Добавлено:
- `_content_during_navigation()` — content(), возвращающий None вместо
исключения, если текст ошибки указывает на гонку с навигацией. Различаем
по тексту, а не по типу: сервис не импортирует playwright, page приходит
готовым. Всё прочее (закрытая страница, упавший браузер) пробрасывается.
- Цикл трактует None как «ещё не устоялось, опроси снова».
- Финальная догидрация тоже защищена: один короткий добор, затем внятная
ошибка вместо падения на гонке.
- Поддельная page в тестах умеет поднимать исключение из content();
три теста на гонку — прохождение, вечная навигация, посторонняя ошибка.
Фальсификация: без правки два новых теста падают именно с
`Execution context was destroyed`. Полный сьют сервиса — 123 passed.
Refs #3045
This commit is contained in:
parent
0f6523f852
commit
5c0fd78c1f
2 changed files with 105 additions and 8 deletions
|
|
@ -971,6 +971,37 @@ def _is_ban_page(html: str) -> bool:
|
|||
return any(marker in lower for marker in _BAN_MARKERS)
|
||||
|
||||
|
||||
# Маркеры исключения playwright «страница прямо сейчас перезагружается». Ловим по
|
||||
# тексту, а не по типу: сервис не импортирует playwright напрямую (page приходит
|
||||
# уже готовым), а Error/TimeoutError у него не образуют отдельной иерархии для
|
||||
# этого случая.
|
||||
_NAVIGATION_RACE_MARKERS: tuple[str, ...] = (
|
||||
"execution context was destroyed",
|
||||
"most likely because of a navigation",
|
||||
"page is navigating",
|
||||
)
|
||||
|
||||
|
||||
async def _content_during_navigation(page: object) -> str | None:
|
||||
"""`page.content()`, устойчивый к перезагрузке страницы под руками.
|
||||
|
||||
PoW-челлендж перезагружает себя сам (`window.location = location.href`), и
|
||||
вызов content(), попавший ровно в этот момент, кидает «Execution context was
|
||||
destroyed». Для нас это НЕ ошибка, а признак того, что перезагрузка — та
|
||||
самая, которую мы ждём, — идёт прямо сейчас. Возвращаем None = «ещё не
|
||||
устоялось, опроси снова», а не роняем фетч на самом успешном исходе.
|
||||
|
||||
Всё остальное (закрытая страница, упавший браузер) пробрасываем как есть.
|
||||
"""
|
||||
try:
|
||||
return await page.content() # type: ignore[attr-defined]
|
||||
except Exception as exc: # noqa: BLE001 — тип не импортируем, различаем по тексту
|
||||
text = str(exc).lower()
|
||||
if any(marker in text for marker in _NAVIGATION_RACE_MARKERS):
|
||||
return None
|
||||
raise
|
||||
|
||||
|
||||
async def _wait_out_pow_challenge(page: object, provider: str, url: str) -> str:
|
||||
"""Опрашивает page.content() пока не исчезнут маркеры PoW-челленджа.
|
||||
|
||||
|
|
@ -985,13 +1016,13 @@ async def _wait_out_pow_challenge(page: object, provider: str, url: str) -> str:
|
|||
"""
|
||||
poll_interval_ms = 1000
|
||||
elapsed_ms = 0
|
||||
html: str = await page.content() # type: ignore[attr-defined]
|
||||
while _is_pow_challenge(html) and elapsed_ms < BROWSER_CHALLENGE_WAIT_MS:
|
||||
html: str | None = await _content_during_navigation(page)
|
||||
while (html is None or _is_pow_challenge(html)) and elapsed_ms < BROWSER_CHALLENGE_WAIT_MS:
|
||||
await page.wait_for_timeout(poll_interval_ms) # type: ignore[attr-defined]
|
||||
elapsed_ms += poll_interval_ms
|
||||
html = await page.content() # type: ignore[attr-defined]
|
||||
html = await _content_during_navigation(page)
|
||||
|
||||
if _is_pow_challenge(html):
|
||||
if html is None or _is_pow_challenge(html):
|
||||
raise ChallengeTimeoutError(
|
||||
f"tradein-browser[{provider}]: PoW-челлендж не снялся за "
|
||||
f"{BROWSER_CHALLENGE_WAIT_MS}мс url={url!r}"
|
||||
|
|
@ -1005,7 +1036,19 @@ async def _wait_out_pow_challenge(page: object, provider: str, url: str) -> str:
|
|||
)
|
||||
if BROWSER_WAIT_MS > 0:
|
||||
await page.wait_for_timeout(BROWSER_WAIT_MS) # type: ignore[attr-defined]
|
||||
return await page.content() # type: ignore[attr-defined]
|
||||
settled = await _content_during_navigation(page)
|
||||
if settled is None:
|
||||
# Догидрация совпала с ещё одной навигацией — даём один короткий добор
|
||||
# вместо того, чтобы падать: контент уже не challenge, гонка чисто
|
||||
# техническая.
|
||||
await page.wait_for_timeout(poll_interval_ms) # type: ignore[attr-defined]
|
||||
settled = await _content_during_navigation(page)
|
||||
if settled is None:
|
||||
raise ChallengeTimeoutError(
|
||||
f"tradein-browser[{provider}]: челлендж снят, но страница не устоялась "
|
||||
f"(навигация не прекращается) url={url!r}"
|
||||
)
|
||||
return settled
|
||||
|
||||
|
||||
async def _fetch_once(
|
||||
|
|
|
|||
|
|
@ -69,9 +69,14 @@ class _ChallengePage:
|
|||
После исчерпания списка повторяет последний элемент (имитирует «страница
|
||||
осталась в этом состоянии»). Фиксирует goto/wait_for_timeout-вызовы для
|
||||
проверки, что бюджет ожидания не тратится там, где не должен.
|
||||
|
||||
Элемент последовательности может быть исключением — тогда content() его
|
||||
поднимает. Это нужно, чтобы воспроизвести гонку с self-reload челленджа:
|
||||
playwright кидает «Execution context was destroyed» ровно в момент той
|
||||
перезагрузки, которую мы ждём, и на моках без этого дефект не виден.
|
||||
"""
|
||||
|
||||
def __init__(self, html_sequence: list[str]) -> None:
|
||||
def __init__(self, html_sequence: list[str | Exception]) -> None:
|
||||
self._html_sequence = html_sequence
|
||||
self._call_count = 0
|
||||
self.goto_urls: list[str] = []
|
||||
|
|
@ -89,9 +94,11 @@ class _ChallengePage:
|
|||
|
||||
async def content(self) -> str:
|
||||
idx = min(self._call_count, len(self._html_sequence) - 1)
|
||||
html = self._html_sequence[idx]
|
||||
item = self._html_sequence[idx]
|
||||
self._call_count += 1
|
||||
return html
|
||||
if isinstance(item, Exception):
|
||||
raise item
|
||||
return item
|
||||
|
||||
async def close(self) -> None:
|
||||
self.closed += 1
|
||||
|
|
@ -217,3 +224,50 @@ def test_fetch_once_normal_page_without_markers_unaffected(
|
|||
assert page.closed == 1
|
||||
assert page.wait_for_timeout_calls == [server.BROWSER_WAIT_MS]
|
||||
assert page.goto_urls == ["https://www.avito.ru/card/1"]
|
||||
|
||||
|
||||
# ── гонка с self-reload челленджа (#3045, найдено при ревью ветки) ──────────────
|
||||
#
|
||||
# Челлендж перезагружает страницу САМ. Вызов page.content(), попавший ровно в этот
|
||||
# момент, кидает «Execution context was destroyed» — то есть цикл ожидания падал бы
|
||||
# именно на успешном исходе, ради которого написан. На моках без явной имитации
|
||||
# это не воспроизводится, поэтому тесты ниже поднимают исключение из content().
|
||||
|
||||
_NAV_RACE = RuntimeError(
|
||||
"Execution context was destroyed, most likely because of a navigation."
|
||||
)
|
||||
|
||||
|
||||
def test_navigation_race_during_reload_is_not_a_failure(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""content() упал на перезагрузке → опрашиваем дальше, отдаём настоящий HTML."""
|
||||
page = _ChallengePage([_CHALLENGE_HTML, _NAV_RACE, _REAL_HTML])
|
||||
_install(monkeypatch, page)
|
||||
|
||||
html = asyncio.run(server._fetch_once("avito", "https://www.avito.ru/x"))
|
||||
|
||||
assert html == _REAL_HTML
|
||||
|
||||
|
||||
def test_permanent_navigation_race_raises_challenge_timeout(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""Навигация не прекращается → своя ошибка, а не сырое исключение playwright."""
|
||||
page = _ChallengePage([_CHALLENGE_HTML, _NAV_RACE])
|
||||
_install(monkeypatch, page)
|
||||
|
||||
with pytest.raises(server.ChallengeTimeoutError):
|
||||
asyncio.run(server._fetch_once("avito", "https://www.avito.ru/x"))
|
||||
|
||||
|
||||
def test_unrelated_content_error_still_propagates(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""Глушим ТОЛЬКО гонку навигации; упавший браузер должен всплыть как есть."""
|
||||
boom = RuntimeError("Target page, context or browser has been closed")
|
||||
page = _ChallengePage([_CHALLENGE_HTML, boom])
|
||||
_install(monkeypatch, page)
|
||||
|
||||
with pytest.raises(RuntimeError, match="has been closed"):
|
||||
asyncio.run(server._fetch_once("avito", "https://www.avito.ru/x"))
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue