First real upload lands: three browser-flow bugs, found by doing it
A Gentle Rain is on BGG (collid 148198034, version 701315 — the
English Bloom edition matched from a shelf photo), verified by
re-fetching the collection. The failures on the way, all in the
login gate the docs called "verified" and none in the version-picker
code they called "unverified":
1. BGG's Sign In is an <a class="btn"> with NO href, so it has no
implicit link role: get_by_role("link", name="Sign In") matched
zero elements in EVERY state, and "no Sign In link" was read as
"already signed in". Every run browsed anonymously.
2. The header hydrates after domcontentloaded, so for a moment
neither control exists — a check resting on one absence guesses.
_signed_out() now polls until the page proves one state or the
other (Sign In vs Sign Out) and raises after 30s rather than
assume; login is verified by the transition, and the session file
saves only after that proof.
3. get_by_label("Own") also matched "Prev. Owned" — strict-mode
violation; the Own checkbox is now matched exactly.
Also: Playwright's multi-line call logs no longer break upload_log.csv
into ragged rows (errors flatten to one line). Three unit tests cover
the hydration window, positive detection, and the undeterminable case.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016jXZFSTZQKzAC8fqpWSz9g
This commit is contained in:
co-authored by
Claude Fable 5
parent
481fa63488
commit
6ae2667c58
+40
-7
@@ -213,12 +213,13 @@ def build_queue(
|
||||
|
||||
def _scrub(text: str) -> str:
|
||||
"""Credentials must never reach the log, even via a selector error that
|
||||
echoes filled form values."""
|
||||
echoes filled form values. Playwright errors carry multi-line call
|
||||
logs; a CSV cell keeps one line."""
|
||||
for key in ("BGG_PASSWORD", "BGG_USERNAME"):
|
||||
value = os.environ.get(key)
|
||||
if value:
|
||||
text = text.replace(value, "***")
|
||||
return text
|
||||
return " ".join(text.split())
|
||||
|
||||
|
||||
class LoginError(RuntimeError):
|
||||
@@ -275,11 +276,32 @@ class PlaywrightUploader:
|
||||
# plus per-element auto-waiting is the reliable pattern.
|
||||
self._page.goto(url, wait_until="domcontentloaded")
|
||||
|
||||
# BGG's Sign In control is an <a class="btn"> with NO href, so it has
|
||||
# no implicit link role — a role="link" query never matched it in any
|
||||
# state, and "no Sign In link" silently meant "already logged in".
|
||||
_SIGN_IN = 'a:has-text("Sign In"), button:has-text("Sign In")'
|
||||
_SIGNED_IN = ':text-is("Sign Out"), :text-is("Log Out")'
|
||||
|
||||
def _signed_out(self) -> bool:
|
||||
# Heuristic: the header shows a "Sign In" link only when logged out.
|
||||
return (
|
||||
self._page.get_by_role("link", name=re.compile(r"^sign in$", re.I)).count()
|
||||
> 0
|
||||
"""True = signed out. Waits for the header to prove ONE state or
|
||||
the other: BGG hydrates it after domcontentloaded, so an early read
|
||||
finds NEITHER control — and any check resting on a single absence
|
||||
(the original bug) then guesses, silently, in the unsafe direction.
|
||||
Undeterminable state raises rather than assumes."""
|
||||
page = self._page
|
||||
deadline = time.monotonic() + 30
|
||||
while time.monotonic() < deadline:
|
||||
out = page.locator(self._SIGN_IN).count()
|
||||
inside = page.locator(self._SIGNED_IN).count()
|
||||
if out and not inside:
|
||||
return True
|
||||
if inside and not out:
|
||||
return False
|
||||
page.wait_for_timeout(250)
|
||||
raise LoginError(
|
||||
"could not tell whether this browser is signed in (neither a "
|
||||
"Sign In nor a Sign Out control appeared in 30s) — BGG's header "
|
||||
"may have changed; see docs/bgg-upload-flow.md"
|
||||
)
|
||||
|
||||
def _ensure_logged_in(self) -> None:
|
||||
@@ -313,6 +335,15 @@ class PlaywrightUploader:
|
||||
"login did not complete (wrong credentials, or the login "
|
||||
"page changed — see docs/bgg-upload-flow.md)"
|
||||
) from err
|
||||
# Verify the TRANSITION: the Sign In control was visible a
|
||||
# moment ago, so its disappearance is evidence, not inference.
|
||||
self._goto(f"{BGG}/")
|
||||
if self._signed_out():
|
||||
raise LoginError(
|
||||
"the login form was submitted but the site still offers "
|
||||
"Sign In — credentials rejected, or a challenge is "
|
||||
"pending in the browser window"
|
||||
)
|
||||
self._context.storage_state(path=str(self._storage_state))
|
||||
self._storage_state.chmod(0o600) # session cookies: owner-only
|
||||
self._authed = True
|
||||
@@ -382,7 +413,9 @@ class PlaywrightUploader:
|
||||
dialog.get_by_role(
|
||||
"heading", name=re.compile(re.escape(job.name), re.I)
|
||||
).wait_for(timeout=15_000)
|
||||
dialog.get_by_label("Own").check()
|
||||
# exact: "Own" is a substring of "Prev. Owned", and a loose label
|
||||
# match resolves to both checkboxes (strict-mode violation)
|
||||
dialog.get_by_role("checkbox", name="Own", exact=True).check()
|
||||
status, note = "added", ""
|
||||
if job.version_name and not self._select_version(dialog, job.version_name):
|
||||
status = "added_no_version"
|
||||
|
||||
Reference in New Issue
Block a user