Retry transient photo download failures instead of failing permanently
Photo downloads had zero retries, unlike video (which already retries stalls/timeouts). Confirmed live: a photo whose download hit "500" from VK's own CDN (sun9-87.userapi.com) was back to a normal 200 moments later - a one-off hiccup, not a real problem with the photo - but with no retry, it went straight to the link-only fallback for good, since a published post is never revisited. Now retries up to 2 more times (3s apart) on a 5xx status, timeout, or connection error; a 4xx (permanently gone/bad URL) still fails immediately since retrying that wouldn't help. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
+28
-5
@@ -70,9 +70,34 @@ class MediaProcessor:
|
|||||||
async def download_photo(
|
async def download_photo(
|
||||||
self, session: aiohttp.ClientSession, url: str, prefix: str = "photo_"
|
self, session: aiohttp.ClientSession, url: str, prefix: str = "photo_"
|
||||||
) -> Optional[Path]:
|
) -> Optional[Path]:
|
||||||
|
"""Retries a couple of times on a transient failure (5xx, timeout, connection
|
||||||
|
reset) before giving up - unlike video, this used to have zero retries, so a
|
||||||
|
single one-off hiccup on VK's CDN (confirmed live: a photo that 500'd here was
|
||||||
|
back to 200 OK moments later) sent a perfectly fine photo straight to the
|
||||||
|
link-only fallback, forever, since a published post never gets revisited. A 4xx
|
||||||
|
(bad URL, permanently gone) is not worth retrying and fails immediately."""
|
||||||
if not url.startswith(("http://", "https://")):
|
if not url.startswith(("http://", "https://")):
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
attempts = 3
|
||||||
|
for attempt in range(1, attempts + 1):
|
||||||
|
path, status, exc = await self._download_photo_attempt(session, url, prefix)
|
||||||
|
if path:
|
||||||
|
return path
|
||||||
|
if status is not None and 400 <= status < 500:
|
||||||
|
logger.warning("Photo download failed with status {} (permanent): {}", status, url)
|
||||||
|
return None
|
||||||
|
if attempt < attempts:
|
||||||
|
logger.warning(
|
||||||
|
"Photo download attempt {}/{} failed transiently ({}) for {} - retrying",
|
||||||
|
attempt, attempts, status if status is not None else exc, url,
|
||||||
|
)
|
||||||
|
await asyncio.sleep(3.0)
|
||||||
|
return None
|
||||||
|
|
||||||
|
async def _download_photo_attempt(
|
||||||
|
self, session: aiohttp.ClientSession, url: str, prefix: str
|
||||||
|
) -> tuple[Optional[Path], Optional[int], Optional[Exception]]:
|
||||||
suffix = Path(url.split("?", 1)[0]).suffix[:6] or ".jpg"
|
suffix = Path(url.split("?", 1)[0]).suffix[:6] or ".jpg"
|
||||||
tmp = tempfile.NamedTemporaryFile(
|
tmp = tempfile.NamedTemporaryFile(
|
||||||
dir=self.cache_dir, prefix=prefix, suffix=suffix, delete=False
|
dir=self.cache_dir, prefix=prefix, suffix=suffix, delete=False
|
||||||
@@ -86,19 +111,17 @@ class MediaProcessor:
|
|||||||
url, timeout=aiohttp.ClientTimeout(total=settings.media_download_timeout_sec)
|
url, timeout=aiohttp.ClientTimeout(total=settings.media_download_timeout_sec)
|
||||||
) as resp:
|
) as resp:
|
||||||
if resp.status != 200:
|
if resp.status != 200:
|
||||||
logger.warning("Photo download failed with status {}: {}", resp.status, url)
|
|
||||||
tmp_path.unlink(missing_ok=True)
|
tmp_path.unlink(missing_ok=True)
|
||||||
await unregister_active_path(tmp_path)
|
await unregister_active_path(tmp_path)
|
||||||
return None
|
return None, resp.status, None
|
||||||
with open(tmp_path, "wb") as f:
|
with open(tmp_path, "wb") as f:
|
||||||
async for chunk in resp.content.iter_chunked(64 * 1024):
|
async for chunk in resp.content.iter_chunked(64 * 1024):
|
||||||
f.write(chunk)
|
f.write(chunk)
|
||||||
return tmp_path
|
return tmp_path, None, None
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
logger.warning("Error downloading photo {}: {}", url, exc)
|
|
||||||
tmp_path.unlink(missing_ok=True)
|
tmp_path.unlink(missing_ok=True)
|
||||||
await unregister_active_path(tmp_path)
|
await unregister_active_path(tmp_path)
|
||||||
return None
|
return None, None, exc
|
||||||
|
|
||||||
async def download_video_ytdlp(
|
async def download_video_ytdlp(
|
||||||
self, url: str, prefix: str = "video_"
|
self, url: str, prefix: str = "video_"
|
||||||
|
|||||||
Reference in New Issue
Block a user