Enable redirect following and avoid treating 302s as errors (#685)

* Enable redirect following and avoid treating 302s as errors

* Remove debugging, reformat

* Change download test to fail if we don't follow redirects

* Use raise_for_status only if we have an error response

This works around httpx's preference to raise in the redirect case.

Author: sgillies

planet/http.py

@@ -93,11 +93,12 @@ def _convert_and_raise(cls, error):
 
     @classmethod
     def _raise_for_status(cls, response):
-        try:
-            response.raise_for_status()
-        except httpx.HTTPStatusError as e:
-            e.response.read()
-            cls._convert_and_raise(e)
+        if response.is_error:
+            try:
+                response.raise_for_status()
+            except httpx.HTTPStatusError as e:
+                e.response.read()
+                cls._convert_and_raise(e)
 
         return
 
@@ -241,7 +242,9 @@ def __init__(self, auth: AuthType = None):
 
         LOGGER.info(f'Session read timeout set to {READ_TIMEOUT}.')
         timeout = httpx.Timeout(10.0, read=READ_TIMEOUT)
-        self._client = httpx.AsyncClient(auth=auth, timeout=timeout)
+        self._client = httpx.AsyncClient(auth=auth,
+                                         timeout=timeout,
+                                         follow_redirects=True)
 
         self._client.headers.update({'User-Agent': self._get_user_agent()})
         self._client.headers.update({'X-Planet-App': 'python-sdk'})
@@ -265,11 +268,12 @@ async def alog_response(*args, **kwargs):
 
     @classmethod
     async def _raise_for_status(cls, response):
-        try:
-            response.raise_for_status()
-        except httpx.HTTPStatusError as e:
-            await e.response.aread()
-            cls._convert_and_raise(e)
+        if response.is_error:
+            try:
+                response.raise_for_status()
+            except httpx.HTTPStatusError as e:
+                await e.response.aread()
+                cls._convert_and_raise(e)
 
         return
 

tests/integration/test_orders_api.py

@@ -33,6 +33,7 @@
 TEST_URL = 'http://www.MockNotRealURL.com/api/path'
 TEST_BULK_CANCEL_URL = f'{TEST_URL}/bulk/orders/v2/cancel'
 TEST_DOWNLOAD_URL = f'{TEST_URL}/download'
+TEST_DOWNLOAD_ACTUAL_URL = f'{TEST_URL}/download_actual'
 TEST_ORDERS_URL = f'{TEST_URL}/orders/v2'
 TEST_STATS_URL = f'{TEST_URL}/stats/orders/v2'
 
@@ -540,8 +541,16 @@ async def test_download_asset_md(tmpdir, session):
         'Content-Type': 'application/json',
         'Content-Disposition': 'attachment; filename="metadata.json"'
     }
+
+    mock_redirect = httpx.Response(HTTPStatus.FOUND,
+                                   headers={
+                                       'Location': TEST_DOWNLOAD_ACTUAL_URL,
+                                       'Content-Length': '0'
+                                   })
     mock_resp = httpx.Response(HTTPStatus.OK, json=md_json, headers=md_headers)
-    respx.get(dl_url).return_value = mock_resp
+
+    respx.get(dl_url).return_value = mock_redirect
+    respx.get(TEST_DOWNLOAD_ACTUAL_URL).return_value = mock_resp
 
     cl = OrdersClient(session, base_url=TEST_URL)
     filename = await cl.download_asset(dl_url, directory=str(tmpdir))