Skip to content

Commit 31792dc

Browse files
authored
refactor: split current view download polling (#54)
Co-authored-by: axisrow <axisrow@users.noreply.github.com>
1 parent 071f6bc commit 31792dc

1 file changed

Lines changed: 42 additions & 74 deletions

File tree

src/wordstat/collector.py

Lines changed: 42 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -1367,26 +1367,11 @@ async def _download_current_view(
13671367
) -> tuple[Path, str | None]:
13681368
"""Download the CSV for the currently selected view.
13691369
1370-
Returns ``(path, escape_warning)``. ``escape_warning`` is ``None`` on
1371-
the common path; it carries a message when a stray escaped download
1372-
(see ``DownloadEscapedError`` below) was observed in the very same
1373-
poll tick as the legitimate CSV (issue #27 follow-up). That case
1374-
must not fail the view — the view's own CSV is fine and already on
1375-
disk — but the operator still needs to know Chrome dropped a file
1376-
outside ``downloads_path`` somewhere. Checking ``new_escaped`` only
1377-
applies when no legitimate CSV was found in this tick would silently
1378-
lose that signal forever: ``session.downloaded_files`` is
1379-
session-lifetime and append-only, so the same path is already inside
1380-
next call's ``before_escaped`` baseline and would never show up in a
1381-
future ``new_escaped`` diff either — this is not a "check it next
1382-
time" gap, the escape is gone from view for good once masked by a
1383-
same-tick success.
1370+
The baselines are captured before clicking: ``downloaded_files`` is a
1371+
session-lifetime log, not a per-download collection.
13841372
"""
1385-
# macOS resolves /tmp to /private/tmp; the downloads directory and
1386-
# session.downloaded_files can report the same physical file under
1387-
# different unresolved paths, which would otherwise look like two
1388-
# distinct downloads. Compare resolved paths, keep the original for
1389-
# return.
1373+
# Resolve paths while taking the snapshot so /tmp and /private/tmp do
1374+
# not make one physical file look like two downloads.
13901375
before = self._resolved_file_snapshot(downloads_path, session)
13911376
before_escaped = self._escaped_download_paths(downloads_path, session)
13921377
# "Скачать" now opens a format menu (CSV / XLSX) instead of downloading
@@ -1396,72 +1381,55 @@ async def _download_current_view(
13961381
page, f"() => Boolean(document.querySelector({json.dumps(DOWNLOAD_CSV_MENU_ITEM_SELECTOR)}))"
13971382
)
13981383
await self._click(page, DOWNLOAD_CSV_MENU_ITEM_SELECTOR)
1384+
return await self._poll_current_view_download(
1385+
session, downloads_path, before, before_escaped
1386+
)
1387+
1388+
async def _poll_current_view_download(
1389+
self,
1390+
session: BrowserSession,
1391+
downloads_path: Path,
1392+
before: dict[Path, Path],
1393+
before_escaped: set[Path],
1394+
) -> tuple[Path, str | None]:
1395+
"""Poll for one non-empty CSV, while checking escaped downloads."""
13991396
deadline = time.monotonic() + self.timeout_seconds
14001397
while time.monotonic() < deadline:
14011398
current = self._resolved_file_snapshot(downloads_path, session)
1402-
new_resolved = set(current) - set(before)
1403-
csv_files = [current[resolved] for resolved in new_resolved if resolved.suffix.lower() == ".csv"]
1404-
# Evaluated every tick, including the one that finds the
1405-
# legitimate CSV: a stray escaped path can land in the exact
1406-
# same tick as a good download, and (per the docstring above)
1407-
# that escape would never be detected on any later call either
1408-
# once masked here — this is the only tick in which it is ever
1409-
# observable at all.
1410-
new_escaped = self._escaped_download_paths(downloads_path, session) - before_escaped
1411-
escape_warning = (
1412-
"Chrome reported a download outside the run's downloads directory "
1413-
f"({downloads_path}): {sorted(str(path) for path in new_escaped)}. "
1414-
"The file was left untouched; it is not safe to move or delete "
1415-
"automatically. Move it manually if it belongs to this run."
1416-
if new_escaped
1417-
else None
1399+
csv_files = self._new_csv_files(before, current)
1400+
escape_warning = self._escape_warning(
1401+
downloads_path,
1402+
self._escaped_download_paths(downloads_path, session) - before_escaped,
14181403
)
14191404
if len(csv_files) == 1 and csv_files[0].stat().st_size > 0:
1420-
# The view's own download succeeded — a stray escape seen in
1421-
# this same tick is reported as a warning, not a failure: the
1422-
# data this view needed is safely on disk, and raising here
1423-
# would discard it for no reason (see the docstring above for
1424-
# why this is the only chance to report the escape at all).
1405+
# An escape in this same tick must be returned as a warning;
1406+
# it will be hidden by the next call's session-log baseline.
14251407
return csv_files[0], escape_warning
14261408
if len(csv_files) > 1:
14271409
raise DownloadTimeoutError("Wordstat produced more than one new CSV for a single export")
1428-
# Chrome can report a download at a path outside downloads_path
1429-
# despite Browser.setDownloadBehavior having been configured for
1430-
# this session (issue #27 — observed live on the fourth view of a
1431-
# phrase, landing under the real ~/Downloads). Root-cause
1432-
# investigated live (CDP :9223, issue #27 fix): every table
1433-
# view's export link is `a[download]` with an `href="blob:..."`
1434-
# and `target="_self"` — DYNAMICS and REGIONS are structurally
1435-
# identical on this point (dumped both live), so a per-target
1436-
# blob/`_self` explanation was ruled out; browser-use's own
1437-
# Browser.setDownloadBehavior call (downloads_watchdog.py) is
1438-
# also browser-level, not per-target, so it should not degrade
1439-
# between views either. Several live full 4-view runs (single
1440-
# phrase and a 2-phrase batch, both with --keep-raw, one against
1441-
# --output-dir inside the repo and one outside it) all completed
1442-
# cleanly with every file landing inside downloads_path — the
1443-
# escape did not reproduce on demand, meaning it's an
1444-
# intermittent Chrome-side race (not deterministically tied to
1445-
# "the fourth view" or any specific view), not a bug in how this
1446-
# collector configures downloads_path. So this containment check
1447-
# can't be "fixed away" upstream; treating the intermittent
1448-
# escape as an honest, loud failure instead of a silent
1449-
# mistargeted move/delete is the correct and sufficient fix. This
1450-
# is never treated as "the" download for this view — that file
1451-
# is not ours to move or delete (it may not even be from this
1452-
# run) — but it must fail loudly and specifically instead of a
1453-
# generic DownloadTimeoutError that leaves the operator guessing
1454-
# whether Wordstat ever produced anything at all.
1455-
if new_escaped:
1456-
raise DownloadEscapedError(
1457-
"Chrome reported a download outside the run's downloads directory "
1458-
f"({downloads_path}): {sorted(str(path) for path in new_escaped)}. "
1459-
"The file was left untouched; it is not safe to move or delete "
1460-
"automatically. Move it manually if it belongs to this run."
1461-
)
1410+
if escape_warning is not None:
1411+
raise DownloadEscapedError(escape_warning)
14621412
await asyncio.sleep(0.25)
14631413
raise DownloadNoNewPathError("Wordstat did not produce a new CSV before the download timeout")
14641414

1415+
@staticmethod
1416+
def _new_csv_files(before: dict[Path, Path], current: dict[Path, Path]) -> list[Path]:
1417+
"""Return CSVs newly observed in the downloads directory."""
1418+
new_resolved = set(current) - set(before)
1419+
return [current[path] for path in new_resolved if path.suffix.lower() == ".csv"]
1420+
1421+
@staticmethod
1422+
def _escape_warning(directory: Path, escaped: set[Path]) -> str | None:
1423+
"""Format an escaped-download warning, or return ``None``."""
1424+
if not escaped:
1425+
return None
1426+
return (
1427+
"Chrome reported a download outside the run's downloads directory "
1428+
f"({directory}): {sorted(str(path) for path in escaped)}. "
1429+
"The file was left untouched; it is not safe to move or delete "
1430+
"automatically. Move it manually if it belongs to this run."
1431+
)
1432+
14651433
async def _click(self, page, selector: str) -> None:
14661434
result = await page.evaluate(
14671435
"""(...args) => {

0 commit comments

Comments
 (0)