Merge pull request #1264 from dandi/more-retry-logs

Log more information about retried HTTP requests
dandi · Apr 5, 2023 · af5eaa0 · af5eaa0
2 parents 0be71d3 + 522c5ec
commit af5eaa0
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 6 deletions.
diff --git a/dandi/consts.py b/dandi/consts.py
@@ -198,3 +198,5 @@ class DandiInstance(NamedTuple):
     "additional",
     "required_if_not_empty",
 }
+
+REQUEST_RETRIES = 12
diff --git a/dandi/dandiapi.py b/dandi/dandiapi.py
@@ -40,6 +40,7 @@
 from .consts import (
     DRAFT,
     MAX_CHUNK_SIZE,
+    REQUEST_RETRIES,
     RETRY_STATUSES,
     ZARR_DELETE_BATCH_SIZE,
     DandiInstance,
@@ -217,13 +218,11 @@ def request(
                     retry=tenacity.retry_if_exception_type(
                         (requests.ConnectionError, requests.HTTPError)
                     ),
-                    stop=tenacity.stop_after_attempt(12),
+                    stop=tenacity.stop_after_attempt(REQUEST_RETRIES),
                     reraise=True,
                 )
             ):
                 with attempt:
-                    if attempt.retry_state.attempt_number > 1:
-                        lgr.warning("Retrying %s %s", method.upper(), url)
                     result = self.session.request(
                         method,
                         url,
@@ -237,9 +236,26 @@ def request(
                     if result.status_code in [*RETRY_STATUSES, *retry_statuses] or (
                         retry_if is not None and retry_if(result)
                     ):
+                        if attempt.retry_state.attempt_number < REQUEST_RETRIES:
+                            lgr.warning(
+                                "Will retry: Error %d while sending %s request to %s: %s",
+                                result.status_code,
+                                method,
+                                url,
+                                result.text,
+                            )
                         result.raise_for_status()
-        except Exception:
-            lgr.exception("HTTP connection failed")
+        except Exception as e:
+            if isinstance(e, requests.HTTPError):
+                lgr.error(
+                    "HTTP request failed repeatedly: Error %d while sending %s request to %s: %s",
+                    e.response.status_code,
+                    method,
+                    url,
+                    e.response.text,
+                )
+            else:
+                lgr.exception("HTTP connection failed")
             raise
 
         if i > 0:

diff --git a/dandi/tests/test_dandiapi.py b/dandi/tests/test_dandiapi.py
@@ -568,7 +568,7 @@ def test_retry_logging(caplog: pytest.LogCaptureFixture) -> None:
     assert (
         "dandi",
         logging.WARNING,
-        "Retrying GET https://test.nil/api/info/",
+        "Will retry: Error 503 while sending GET request to https://test.nil/api/info/: ",
     ) in caplog.record_tuples
     assert (
         "dandi",