Log media requests and raise on bad status

Author: tulir

CHANGELOG.md

@@ -1,3 +1,8 @@
+## v0.20.4 (unreleased)
+
+* *(client)* Changed media download methods to log requests and to raise
+  exceptions on non-successful status codes.
+
 ## v0.20.3 (2023-11-10)
 
 * *(client)* Deprecated MSC2716 methods and added new Beeper-specific batch

mautrix/api.py

@@ -275,7 +275,7 @@ def _log_request(
         self,
         method: Method,
         url: URL,
-        content: str | bytes | bytearray | AsyncBody,
+        content: str | bytes | bytearray | AsyncBody | None,
         orig_content,
         query_params: dict[str, str],
         headers: dict[str, str],
@@ -314,7 +314,7 @@ def _log_request(
         )
 
     def _log_request_done(
-        self, path: PathBuilder, req_id: int, duration: float, status: int
+        self, path: PathBuilder | str, req_id: int, duration: float, status: int
     ) -> None:
         level = 5 if path == Path.v3.sync else 10
         duration_str = f"{duration * 1000:.1f}ms" if duration < 1 else f"{duration:.3f}s"
@@ -334,6 +334,16 @@ def _full_path(self, path: PathBuilder | str) -> str:
             base_path += "/"
         return urllib_join(base_path, path)
 
+    def log_download_request(self, url: URL, query_params: dict[str, str]) -> int:
+        req_id = _next_global_req_id()
+        self._log_request(Method.GET, url, None, None, query_params, {}, req_id, False)
+        return req_id
+
+    def log_download_request_done(
+        self, url: URL, req_id: int, duration: float, status: int
+    ) -> None:
+        self._log_request_done(url.path.removeprefix("/_matrix/media/"), req_id, duration, status)
+
     async def request(
         self,
         method: Method,

mautrix/client/api/modules/media_repository.py

@@ -182,8 +182,16 @@ async def download_media(self, url: ContentURI, timeout_ms: int | None = None) -
         query_params: dict[str, Any] = {"allow_redirect": "true"}
         if timeout_ms is not None:
             query_params["timeout_ms"] = timeout_ms
+        req_id = self.api.log_download_request(url, query_params)
+        start = time.monotonic()
         async with self.api.session.get(url, params=query_params) as response:
-            return await response.read()
+            try:
+                response.raise_for_status()
+                return await response.read()
+            finally:
+                self.api.log_download_request_done(
+                    url, req_id, time.monotonic() - start, response.status
+                )
 
     async def download_thumbnail(
         self,
@@ -227,8 +235,16 @@ async def download_thumbnail(
             query_params["allow_remote"] = allow_remote
         if timeout_ms is not None:
             query_params["timeout_ms"] = timeout_ms
+        req_id = self.api.log_download_request(url, query_params)
+        start = time.monotonic()
         async with self.api.session.get(url, params=query_params) as response:
-            return await response.read()
+            try:
+                response.raise_for_status()
+                return await response.read()
+            finally:
+                self.api.log_download_request_done(
+                    url, req_id, time.monotonic() - start, response.status
+                )
 
     async def get_url_preview(self, url: str, timestamp: int | None = None) -> MXOpenGraph:
         """