Restructure mirror responses (#12)

* Log beatmap requests in better quality * Better log detail * Remove redundant logs * type fixes * unused module * set error message
osuAkatsuki · Jun 30, 2024 · 54f040f · 54f040f
1 parent b2a5c09
commit 54f040f
Show file tree

Hide file tree

Showing 7 changed files with 142 additions and 174 deletions.
diff --git a/app/adapters/osu_mirrors/__init__.py b/app/adapters/osu_mirrors/__init__.py
@@ -71,77 +71,50 @@ async def fetch_beatmap_zip_data(beatmapset_id: int) -> bytes | None:
             return None
 
         num_attempts += 1
+
         started_at = datetime.now()
+        mirror_response = await mirror.fetch_beatmap_zip_data(beatmapset_id)
+        ended_at = datetime.now()
 
-        mirror_response: BeatmapMirrorResponse[bytes | None] | None = None
-        try:
-            mirror_response = await mirror.fetch_beatmap_zip_data(beatmapset_id)
+        if mirror_response.is_success:
             if mirror_response.data is not None and not is_valid_zip_file(
                 mirror_response.data,
             ):
-                raise ValueError("Received bad osz data from mirror")
-        except Exception as exc:
-            ended_at = datetime.now()
-            await beatmap_mirror_requests.create(
-                request_url=(
-                    mirror_response.request_url if mirror_response else "unavailable"
-                ),
-                api_key_id=None,
-                mirror_name=mirror.name,
-                success=False,
-                started_at=started_at,
-                ended_at=ended_at,
-                response_status_code=(
-                    mirror_response.status_code
-                    if mirror_response and mirror_response.status_code
-                    else None
-                ),
-                response_size=(
-                    len(mirror_response.data)
-                    if mirror_response and mirror_response.data
-                    else 0
-                ),
-                response_error=str(exc),
-                resource=MirrorResource.OSZ_FILE,
-            )
-            await OSZ_FILE_MIRROR_SELECTOR.update_all_mirror_and_selector_weights()
-            logging.warning(
-                "Failed to fetch beatmapset osz from mirror",
-                exc_info=True,
-                extra={
-                    "response": (
-                        {
-                            "url": mirror_response.request_url,
-                            "status_code": mirror_response.status_code,
-                        }
-                        if mirror_response is not None
-                        else None
-                    ),
-                    "mirror_name": mirror.name,
-                    "mirror_weight": mirror.weight,
-                    "beatmapset_id": beatmapset_id,
-                },
-            )
-            prev_mirror = mirror
-            continue
-        else:
+                mirror_response.is_success = False
+                mirror_response.error_message = "Invalid .osz file"
+
+        await beatmap_mirror_requests.create(
+            request_url=mirror_response.request_url or "unavailable",
+            api_key_id=None,
+            mirror_name=mirror.name,
+            success=mirror_response.is_success,
+            started_at=started_at,
+            ended_at=ended_at,
+            response_status_code=mirror_response.status_code,
+            response_size=len(mirror_response.data) if mirror_response.data else 0,
+            response_error=mirror_response.error_message,
+            resource=MirrorResource.OSZ_FILE,
+        )
+        await OSZ_FILE_MIRROR_SELECTOR.update_all_mirror_and_selector_weights()
+
+        if mirror_response.is_success:
             break
 
-    ended_at = datetime.now()
-
-    await beatmap_mirror_requests.create(
-        request_url=f"{mirror.base_url}/d/{beatmapset_id}",
-        api_key_id=None,
-        mirror_name=mirror.name,
-        success=True,
-        started_at=started_at,
-        ended_at=ended_at,
-        response_status_code=mirror_response.status_code,
-        response_size=len(mirror_response.data) if mirror_response.data else 0,
-        response_error=None,
-        resource=MirrorResource.OSZ_FILE,
-    )
-    await OSZ_FILE_MIRROR_SELECTOR.update_all_mirror_and_selector_weights()
+        logging.warning(
+            "Failed to fetch beatmapset osz from mirror",
+            exc_info=True,
+            extra={
+                "response": {
+                    "url": mirror_response.request_url,
+                    "status_code": mirror_response.status_code,
+                },
+                "mirror_name": mirror.name,
+                "mirror_weight": mirror.weight,
+                "beatmapset_id": beatmapset_id,
+            },
+        )
+        prev_mirror = mirror
+        continue
 
     ms_elapsed = (ended_at.timestamp() - started_at.timestamp()) * 1000
 
@@ -186,75 +159,43 @@ async def fetch_beatmap_background_image(beatmap_id: int) -> bytes | None:
             return None
 
         num_attempts += 1
-        started_at = datetime.now()
 
-        mirror_response: BeatmapMirrorResponse[bytes | None] | None = None
-        try:
-            mirror_response = await mirror.fetch_beatmap_background_image(
-                beatmap_id,
-            )
-        except Exception as exc:
-            ended_at = datetime.now()
-            await beatmap_mirror_requests.create(
-                request_url=(
-                    mirror_response.request_url if mirror_response else "unavailable"
-                ),
-                api_key_id=None,
-                mirror_name=mirror.name,
-                success=False,
-                started_at=started_at,
-                ended_at=ended_at,
-                response_status_code=(
-                    mirror_response.status_code
-                    if mirror_response and mirror_response.status_code
-                    else None
-                ),
-                response_size=(
-                    len(mirror_response.data)
-                    if mirror_response and mirror_response.data
-                    else 0
-                ),
-                response_error=str(exc),
-                resource=MirrorResource.BACKGROUND_IMAGE,
-            )
-            await BACKGROUND_IMAGE_MIRROR_SELECTOR.update_all_mirror_and_selector_weights()
-            logging.warning(
-                "Failed to fetch beatmap background image from mirror",
-                exc_info=True,
-                extra={
-                    "response": (
-                        {
-                            "url": mirror_response.request_url,
-                            "status_code": mirror_response.status_code,
-                        }
-                        if mirror_response is not None
-                        else None
-                    ),
-                    "mirror_name": mirror.name,
-                    "mirror_weight": mirror.weight,
-                    "beatmap_id": beatmap_id,
-                },
-            )
-            prev_mirror = mirror
-            continue
-        else:
+        started_at = datetime.now()
+        mirror_response = await mirror.fetch_beatmap_background_image(beatmap_id)
+        ended_at = datetime.now()
+
+        await beatmap_mirror_requests.create(
+            request_url=mirror_response.request_url or "unavailable",
+            api_key_id=None,
+            mirror_name=mirror.name,
+            success=mirror_response.is_success,
+            started_at=started_at,
+            ended_at=ended_at,
+            response_status_code=mirror_response.status_code,
+            response_size=len(mirror_response.data) if mirror_response.data else 0,
+            response_error=mirror_response.error_message,
+            resource=MirrorResource.BACKGROUND_IMAGE,
+        )
+        await BACKGROUND_IMAGE_MIRROR_SELECTOR.update_all_mirror_and_selector_weights()
+
+        if mirror_response.is_success:
             break
 
-    ended_at = datetime.now()
-
-    await beatmap_mirror_requests.create(
-        request_url=mirror_response.request_url,
-        api_key_id=None,
-        mirror_name=mirror.name,
-        success=True,
-        started_at=started_at,
-        ended_at=ended_at,
-        response_status_code=mirror_response.status_code,
-        response_size=len(mirror_response.data) if mirror_response.data else 0,
-        response_error=None,
-        resource=MirrorResource.BACKGROUND_IMAGE,
-    )
-    await BACKGROUND_IMAGE_MIRROR_SELECTOR.update_all_mirror_and_selector_weights()
+        logging.warning(
+            "Failed to fetch beatmap background image from mirror",
+            exc_info=True,
+            extra={
+                "response": {
+                    "url": mirror_response.request_url,
+                    "status_code": mirror_response.status_code,
+                },
+                "mirror_name": mirror.name,
+                "mirror_weight": mirror.weight,
+                "beatmap_id": beatmap_id,
+            },
+        )
+        prev_mirror = mirror
+        continue
 
     ms_elapsed = (ended_at.timestamp() - started_at.timestamp()) * 1000
 

diff --git a/app/adapters/osu_mirrors/backends/__init__.py b/app/adapters/osu_mirrors/backends/__init__.py
@@ -12,15 +12,13 @@
 T = TypeVar("T", covariant=True)
 
 
-class MirrorRequestError(Exception):
-    pass
-
-
 @dataclass
 class BeatmapMirrorResponse(Generic[T]):
     data: T
-    request_url: str
-    status_code: int
+    is_success: bool
+    request_url: str | None
+    status_code: int | None
+    error_message: str | None = None
 
 
 class AbstractBeatmapMirror(ABC):

diff --git a/app/adapters/osu_mirrors/backends/gatari.py b/app/adapters/osu_mirrors/backends/gatari.py
@@ -1,10 +1,10 @@
 import logging
 
+import httpx
 from typing_extensions import override
 
 from app.adapters.osu_mirrors.backends import AbstractBeatmapMirror
 from app.adapters.osu_mirrors.backends import BeatmapMirrorResponse
-from app.adapters.osu_mirrors.backends import MirrorRequestError
 from app.repositories.beatmap_mirror_requests import MirrorResource
 
 
@@ -18,27 +18,31 @@ async def fetch_beatmap_zip_data(
         self,
         beatmapset_id: int,
     ) -> BeatmapMirrorResponse[bytes | None]:
+        response: httpx.Response | None = None
         try:
-            logging.info(f"Fetching beatmapset osz from gatari: {beatmapset_id}")
             response = await self.http_client.get(
                 f"{self.base_url}/d/{beatmapset_id}",
                 follow_redirects=True,
             )
             if response.status_code == 404:
                 return BeatmapMirrorResponse(
                     data=None,
+                    is_success=True,
                     request_url=str(response.request.url),
                     status_code=response.status_code,
                 )
             response.raise_for_status()
             return BeatmapMirrorResponse(
                 data=response.read(),
+                is_success=True,
                 request_url=str(response.request.url),
                 status_code=response.status_code,
             )
         except Exception as exc:
-            logging.warning(
-                "Failed to fetch beatmap from gatari.pw",
-                exc_info=True,
+            return BeatmapMirrorResponse(
+                data=None,
+                is_success=False,
+                request_url=str(response.request.url) if response else None,
+                status_code=response.status_code if response else None,
+                error_message=str(exc),
             )
-            raise MirrorRequestError() from exc
diff --git a/app/adapters/osu_mirrors/backends/mino.py b/app/adapters/osu_mirrors/backends/mino.py
@@ -5,7 +5,6 @@
 
 from app.adapters.osu_mirrors.backends import AbstractBeatmapMirror
 from app.adapters.osu_mirrors.backends import BeatmapMirrorResponse
-from app.adapters.osu_mirrors.backends import MirrorRequestError
 from app.repositories.beatmap_mirror_requests import MirrorResource
 
 
@@ -19,36 +18,41 @@ async def fetch_beatmap_zip_data(
         self,
         beatmapset_id: int,
     ) -> BeatmapMirrorResponse[bytes | None]:
+        response: httpx.Response | None = None
         try:
-            logging.info(f"Fetching beatmapset osz from mino: {beatmapset_id}")
             response = await self.http_client.get(
                 f"{self.base_url}/d/{beatmapset_id}",
                 timeout=httpx.Timeout(None, connect=2),
             )
             if response.status_code == 404:
                 return BeatmapMirrorResponse(
                     data=None,
+                    is_success=True,
                     request_url=str(response.request.url),
                     status_code=response.status_code,
                 )
             response.raise_for_status()
             return BeatmapMirrorResponse(
                 data=response.read(),
+                is_success=True,
                 request_url=str(response.request.url),
                 status_code=response.status_code,
             )
         except Exception as exc:
-            logging.warning(
-                "Failed to fetch beatmap from catboy.best",
-                exc_info=True,
+            return BeatmapMirrorResponse(
+                data=None,
+                is_success=False,
+                request_url=str(response.request.url) if response else None,
+                status_code=response.status_code if response else None,
+                error_message=str(exc),
             )
-            raise MirrorRequestError() from exc
 
     @override
     async def fetch_beatmap_background_image(
         self,
         beatmap_id: int,
     ) -> BeatmapMirrorResponse[bytes | None]:
+        response: httpx.Response | None = None
         try:
             logging.info(f"Fetching beatmap background from mino: {beatmap_id}")
             response = await self.http_client.get(
@@ -58,12 +62,14 @@ async def fetch_beatmap_background_image(
             if response.status_code == 404:
                 return BeatmapMirrorResponse(
                     data=None,
+                    is_success=True,
                     request_url=str(response.request.url),
                     status_code=response.status_code,
                 )
             response.raise_for_status()
             return BeatmapMirrorResponse(
                 data=response.read(),
+                is_success=True,
                 request_url=str(response.request.url),
                 status_code=response.status_code,
             )
@@ -72,4 +78,10 @@ async def fetch_beatmap_background_image(
                 "Failed to fetch beatmap background from catboy.best",
                 exc_info=True,
             )
-            raise MirrorRequestError() from exc
+            return BeatmapMirrorResponse(
+                data=None,
+                is_success=False,
+                request_url=str(response.request.url) if response else None,
+                status_code=response.status_code if response else None,
+                error_message=str(exc),
+            )