mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-10-02 22:06:00 +01:00
fix(download): hand AA challenges to the bypasser immediately (#1212)
## Problem
Two defects in `html_get_page`, either of which is enough to make an
Anna's Archive search fail *without the bypasser ever running*. Found
while chasing why AA search returned nothing on v1.3.7 even with
`USE_CF_BYPASS` on and a working bypasser.
### 1. An AA redirect loop is treated as a network fault
AA serves its DDoS-Guard handshake as a same-host redirect loop:
`/search?…` redirects to `/search?…&check=1`, which redirects back,
indefinitely. The manual redirect follower counts those against
`_MAX_REDIRECTS` and raises `TooManyRedirects`:
```python
redirects_followed += 1
if redirects_followed > _MAX_REDIRECTS:
_raise_too_many_redirects(f"Too many redirects for {current_url}")
```
That lands in the retry path, so every one of the `MAX_RETRY` attempts
re-runs the same 6-redirect loop and the URL is never offered to the
bypasser — which is the only thing that can clear the challenge. With
the default `MAX_RETRY=10` that's ~60 requests to AA per search, all of
which can only fail:
```
Retry 5/10 for https://annas-archive.pk/search?…&check=1: TooManyRedirects
Retry 6/10 for https://annas-archive.pk/search?…&check=1: TooManyRedirects
…
Giving up after 10 attempts: https://annas-archive.pk/search?…&check=1
```
Surfaced to the user as `Unable to reach download source. Network
restricted or mirrors are blocked.`
### 2. Both bypasser handoffs are a no-op at `MAX_RETRY=1`
The existing 403 handoff — and the new redirect one — set a flag and
`continue`:
```python
logger.info("403 detected; switching to bypasser: %s", current_url)
use_bypasser_now = True
continue
```
The branch that acts on `use_bypasser_now` sits at the top of the
**next** retry attempt. With `MAX_RETRY=1` there is no next attempt, so
a 403 simply ends the search and the bypasser never runs. `MAX_RETRY` is
user-configurable down to 1, so this is reachable in normal use.
The redirect handoff had an additional problem: it sits inside the inner
redirect `while`, so a `continue` there re-enters *that* loop rather
than reaching the retry branch at all.
## Change
Both handoffs now invoke the bypasser directly, through a shared
`_run_bypasser()` closure extracted from the existing branch body. No
behaviour change to the bypass itself — same grace handling, same error
reporting, same `finally`.
The redirect handoff also honours `allow_bypasser_fallback`, for the
same reason the 403 path does: callers such as the `/dyn/md5/summary/…`
fetch behind the details modal pass `False` precisely so a best-effort
request fails fast instead of holding the UI open for a minutes-long
browser solve.
## Result
Measured against `/api/releases` for the same book, internal bypasser,
default `MAX_RETRY`:
| | searches returning results |
|---|---|
| before | 4 / 9 |
| after | 3 / 3, then 7 / 7 |
Zero `TooManyRedirects` give-ups after, and the new path is visible in
the logs:
```
redirect loop on https://annas-archive.gl/search?…&check=1; switching to bypasser
Bypass successful using _bypass_method_cdp_gui_click
```
The request volume drop is the other half of the win — a failing search
no longer emits ~60 requests to AA before giving up.
## Notes
- Only `shelfmark/download/http.py` changes; no config or API surface.
- `use_bypasser_now` is still set before each direct call, so the guard
against double-invocation is unchanged.
- Tested with the internal bypasser (seleniumbase). The
external-bypasser path goes through the same `get_bypassed_page()` call
and is unaffected by the control-flow change, though I have not measured
it against DDoS-Guard specifically — in my testing
FlareSolverr-compatible solvers do not clear that challenge regardless.
Co-authored-by: D <d@e>
This commit is contained in:
+58
-26
@@ -287,6 +287,38 @@ def html_get_page(
|
||||
return html, response_url
|
||||
return html
|
||||
|
||||
def _run_bypasser(bypass_url: str) -> str | tuple[str, str]:
|
||||
"""Run the active bypasser for one URL and return its result.
|
||||
|
||||
Factored out so the redirect-loop handoff below can invoke it directly. That
|
||||
call site sits inside the inner redirect `while`, so it cannot reach the
|
||||
retry-loop branch above with `continue`, and with MAX_RETRY=1 there is no
|
||||
later attempt for that branch to run on either.
|
||||
"""
|
||||
if status_callback:
|
||||
status_callback("resolving", "Bypassing protection...")
|
||||
try:
|
||||
# A bypass is one long blocking call with no incremental progress, so
|
||||
# tell the orchestrator up front how long it may legitimately take
|
||||
# instead of trying to fake activity while it runs. Inside the try so a
|
||||
# bypasser that fails to load is still reported as a bypasser error.
|
||||
request_activity_grace(status_callback, _bypass_grace_seconds())
|
||||
result = get_bypassed_page(bypass_url, selector, cancel_flag)
|
||||
return _result(result or "", bypass_url)
|
||||
except _BYPASSER_ERRORS as e:
|
||||
logger.warning("Bypasser error: %s: %s", type(e).__name__, e)
|
||||
# Surface the real reason. Without this the caller only sees an empty
|
||||
# page and the download dies with a generic failure, hiding e.g. a
|
||||
# FlareSolverr 500 behind a silent wait.
|
||||
if status_callback and not isinstance(e, BypassCancelledError):
|
||||
try:
|
||||
status_callback("error", f"Bypass failed: {type(e).__name__}: {e}")
|
||||
except _STATUS_CALLBACK_ERRORS:
|
||||
logger.debug("Bypass error status callback failed", exc_info=True)
|
||||
return _result("", bypass_url)
|
||||
finally:
|
||||
release_activity_grace(status_callback)
|
||||
|
||||
configured_retry = normalize_positive_int(app_config.MAX_RETRY)
|
||||
retry_limit = (
|
||||
retry if retry is not None else (configured_retry if configured_retry is not None else 1)
|
||||
@@ -308,29 +340,7 @@ def html_get_page(
|
||||
cookies: dict[str, str] = {}
|
||||
try:
|
||||
if use_bypasser_now and _is_cf_bypass_enabled():
|
||||
if status_callback:
|
||||
status_callback("resolving", "Bypassing protection...")
|
||||
try:
|
||||
# A bypass is one long blocking call with no incremental progress, so
|
||||
# tell the orchestrator up front how long it may legitimately take
|
||||
# instead of trying to fake activity while it runs. Inside the try so a
|
||||
# bypasser that fails to load is still reported as a bypasser error.
|
||||
request_activity_grace(status_callback, _bypass_grace_seconds())
|
||||
result = get_bypassed_page(current_url, selector, cancel_flag)
|
||||
return _result(result or "", current_url)
|
||||
except _BYPASSER_ERRORS as e:
|
||||
logger.warning("Bypasser error: %s: %s", type(e).__name__, e)
|
||||
# Surface the real reason. Without this the caller only sees an empty
|
||||
# page and the download dies with a generic failure, hiding e.g. a
|
||||
# FlareSolverr 500 behind a silent wait.
|
||||
if status_callback and not isinstance(e, BypassCancelledError):
|
||||
try:
|
||||
status_callback("error", f"Bypass failed: {type(e).__name__}: {e}")
|
||||
except _STATUS_CALLBACK_ERRORS:
|
||||
logger.debug("Bypass error status callback failed", exc_info=True)
|
||||
return _result("", current_url)
|
||||
finally:
|
||||
release_activity_grace(status_callback)
|
||||
return _run_bypasser(current_url)
|
||||
|
||||
logger.debug("GET: %s", current_url)
|
||||
|
||||
@@ -422,6 +432,25 @@ def html_get_page(
|
||||
# Same-host redirect (relative or absolute) - follow manually.
|
||||
redirects_followed += 1
|
||||
if redirects_followed > _MAX_REDIRECTS:
|
||||
# A same-host redirect loop on AA is not a network fault — it is
|
||||
# how an unsolved DDoS-Guard handshake presents: /search redirects
|
||||
# to /search&check=1, which redirects back, indefinitely. Raising
|
||||
# sends it down the retry path, which re-runs the whole loop on
|
||||
# every attempt (10 x 6 = ~60 requests to AA) and never once offers
|
||||
# the URL to the bypasser, so it can only ever fail. Hand it over
|
||||
# directly — not via `continue`, which would target this inner
|
||||
# redirect loop rather than the retry branch above.
|
||||
# allow_bypasser_fallback is honoured here for the same
|
||||
# reason the 403 path honours it: callers such as the
|
||||
# /dyn/md5/summary fetch behind the details modal pass False
|
||||
# precisely so a best-effort request fails fast instead of
|
||||
# holding the UI open for a minutes-long browser solve.
|
||||
if allow_bypasser_fallback and _is_cf_bypass_enabled() and not use_bypasser_now:
|
||||
logger.info(
|
||||
"redirect loop on %s; switching to bypasser", current_url
|
||||
)
|
||||
use_bypasser_now = True
|
||||
return _run_bypasser(current_url)
|
||||
_raise_too_many_redirects(f"Too many redirects for {current_url}")
|
||||
current_url = redirect_url
|
||||
continue
|
||||
@@ -476,10 +505,13 @@ def html_get_page(
|
||||
)
|
||||
continue
|
||||
logger.info("403 detected; switching to bypasser: %s", current_url)
|
||||
if status_callback:
|
||||
status_callback("resolving", "Bypassing protection...")
|
||||
# Invoke it here rather than setting the flag and continuing. The
|
||||
# branch that acts on the flag runs at the top of the *next* retry
|
||||
# attempt, so under the supported MAX_RETRY=1 there is no next
|
||||
# attempt and the bypasser was never reached — a 403 simply ended
|
||||
# the search. Same reasoning as the redirect-loop handoff below.
|
||||
use_bypasser_now = True
|
||||
continue
|
||||
return _run_bypasser(current_url)
|
||||
logger.warning("403 error, giving up: %s", current_url)
|
||||
return _result("", current_url)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user