From f714fa93d691a264a2f4e2b8cd798aa4276caab0 Mon Sep 17 00:00:00 2001
From: Sebastiaan Zeeff <33516116+SebastiaanZ@users.noreply.github.com>
Date: Sat, 14 Sep 2019 13:02:23 +0200
Subject: Add API response dict to ResponseCodeError

The ReponseCodeError held a reference to `aiohttp.ResonseObject` to
make sure the response data was available. However, the response data
is not actually included in the Response Object, but needs to be
awaited.

Unfortunately, the ResponseCodeError is usually inspected after the
connection has been closed, triggering a ClientConnectionError when the
data was retrieved.

I solved this by adding the awaited reponse data directly to our custom
exception by awaiting the response.json() before raising the exception.
---
 bot/api.py | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/bot/api.py b/bot/api.py
index cd19896e1..36f9cfcd4 100644
--- a/bot/api.py
+++ b/bot/api.py
@@ -10,9 +10,14 @@ log = logging.getLogger(__name__)
 
 
 class ResponseCodeError(ValueError):
-    def __init__(self, response: aiohttp.ClientResponse):
+    def __init__(self, response: aiohttp.ClientResponse, response_data: dict):
+        self.status = response.status
+        self.response_data = response_data
         self.response = response
 
+    def __str__(self):
+        return f"Status: {self.status} Response: {self.response_data}"
+
 
 class APIClient:
     def __init__(self, **kwargs):
@@ -31,28 +36,29 @@ class APIClient:
     def _url_for(endpoint: str):
         return f"{URLs.site_schema}{URLs.site_api}/{quote_url(endpoint)}"
 
-    def maybe_raise_for_status(self, response: aiohttp.ClientResponse, should_raise: bool):
+    async def maybe_raise_for_status(self, response: aiohttp.ClientResponse, should_raise: bool):
         if should_raise and response.status >= 400:
-            raise ResponseCodeError(response=response)
+            response_data = await response.json()
+            raise ResponseCodeError(response=response, response_data=response_data)
 
     async def get(self, endpoint: str, *args, raise_for_status: bool = True, **kwargs):
         async with self.session.get(self._url_for(endpoint), *args, **kwargs) as resp:
-            self.maybe_raise_for_status(resp, raise_for_status)
+            await self.maybe_raise_for_status(resp, raise_for_status)
             return await resp.json()
 
     async def patch(self, endpoint: str, *args, raise_for_status: bool = True, **kwargs):
         async with self.session.patch(self._url_for(endpoint), *args, **kwargs) as resp:
-            self.maybe_raise_for_status(resp, raise_for_status)
+            await self.maybe_raise_for_status(resp, raise_for_status)
             return await resp.json()
 
     async def post(self, endpoint: str, *args, raise_for_status: bool = True, **kwargs):
         async with self.session.post(self._url_for(endpoint), *args, **kwargs) as resp:
-            self.maybe_raise_for_status(resp, raise_for_status)
+            await self.maybe_raise_for_status(resp, raise_for_status)
             return await resp.json()
 
     async def put(self, endpoint: str, *args, raise_for_status: bool = True, **kwargs):
         async with self.session.put(self._url_for(endpoint), *args, **kwargs) as resp:
-            self.maybe_raise_for_status(resp, raise_for_status)
+            await self.maybe_raise_for_status(resp, raise_for_status)
             return await resp.json()
 
     async def delete(self, endpoint: str, *args, raise_for_status: bool = True, **kwargs):
@@ -60,7 +66,7 @@ class APIClient:
             if resp.status == 204:
                 return None
 
-            self.maybe_raise_for_status(resp, raise_for_status)
+            await self.maybe_raise_for_status(resp, raise_for_status)
             return await resp.json()
 
 
-- 
cgit v1.2.3