fix(replay-parse-service): handle plain HTTPError from ballchasing client

microservices/replay-parse-service/src/parser.py

@@ -509,6 +509,27 @@ def _parse_carball_full_analysis(path: str, on_progress: Callable[[str], None] =
 #
 ###############################
 
+def _extract_response_and_body(exc: Exception):
+    """
+    Extract (response, body) from an exception raised by the ballchasing
+    library. The library calls Response.raise_for_status(), which produces
+    a requests.HTTPError whose .response attribute is the original Response.
+
+    Returns (None, {}) if the exception has no attached response so callers
+    can re-raise unrecognized exceptions instead of misinterpreting them.
+    """
+    response = getattr(exc, "response", None)
+    if response is None:
+        return None, {}
+    try:
+        body = response.json()
+    except (ValueError, AttributeError):
+        body = {}
+    if not isinstance(body, dict):
+        body = {}
+    return response, body
+
+
 def _is_rate_limit(response: Response, body: dict) -> bool:
     """
     https://ballchasing.com/doc/api#header-rate-limiting
@@ -597,17 +618,19 @@ def _parse_ballchasing(path: str, on_progress: Callable[[str], None] = None) ->
                 ballchasing_id = get_ballchasing_api().upload_replay(replay_file)["id"]
                 break
             except Exception as e:
-                err_body = e.args[1]
+                response, err_body = _extract_response_and_body(e)
+                if response is None:
+                    raise
 
-                if _parse_is_duplicate_replay(*e.args):
+                if _parse_is_duplicate_replay(response, err_body):
                     ballchasing_id = err_body["id"]
                     print(f"Replay already parsed {ballchasing_id}")
                     break
 
                 elif i == MAX_RETRIES - 1:
                     raise Exception(f"Unable to upload replay {ballchasing_id} after {MAX_RETRIES} retries, total delay of {sum(DELAYS)}")
-                
-                elif _parse_is_failed_replay(*e.args):
+
+                elif _parse_is_failed_replay(response, err_body):
                     logging.error(f"Parsing {path} with Ballchasing failed due to bad replay", err_body)
                     raise e
                 else:
@@ -628,10 +651,14 @@ def _parse_ballchasing(path: str, on_progress: Callable[[str], None] = None) ->
             try:
                 body = get_ballchasing_api().get_replay(ballchasing_id)
             except Exception as e:
+                response, err_body = _extract_response_and_body(e)
+                if response is None:
+                    raise
+
                 if i == MAX_RETRIES - 1:
                     raise Exception(f"Unable to parse replay {ballchasing_id} after {MAX_RETRIES} retries, total delay of {sum(DELAYS)}")
 
-                if _is_rate_limit(*e.args):
+                if _is_rate_limit(response, err_body):
                     print(f"Rate limited, retrying in {DELAYS[i+1]} seconds")
                     continue
                 else:

microservices/replay-parse-service/tests/test_ballchasing_error_handling.py

@@ -0,0 +1,146 @@
+"""
+Regression tests for ballchasing HTTPError handling in parser._parse_ballchasing.
+
+The python-ballchasing library raises requests.HTTPError via
+Response.raise_for_status(). The exception's .args is a 1-tuple (message,) but
+the parser previously assumed it was a 2-tuple (response, body) and crashed
+with `IndexError: tuple index out of range` on every duplicate-replay 409.
+"""
+
+import os
+import sys
+import unittest
+from types import ModuleType
+from pathlib import Path
+from unittest.mock import MagicMock, patch
+
+
+SERVICE_ROOT = Path(__file__).resolve().parents[1]
+SRC_ROOT = SERVICE_ROOT / "src"
+
+os.environ.setdefault("CONFIG_DIR", str(SERVICE_ROOT / "config"))
+os.environ.setdefault("ENV", "default")
+os.chdir(SRC_ROOT)
+sys.path.insert(0, str(SRC_ROOT))
+
+if "carball" not in sys.modules:
+    sys.modules["carball"] = ModuleType("carball")
+
+if "ballchasing" not in sys.modules:
+    ballchasing = ModuleType("ballchasing")
+
+    class _Api:
+        def __init__(self, *args, **kwargs):
+            pass
+
+    ballchasing.Api = _Api
+    sys.modules["ballchasing"] = ballchasing
+
+if "requests" not in sys.modules:
+    requests = ModuleType("requests")
+
+    class _Response:
+        pass
+
+    requests.Response = _Response
+    sys.modules["requests"] = requests
+
+import parser as replay_parser  # noqa: E402
+
+
+def _make_http_error(status_code, body):
+    """Build an exception matching what requests.HTTPError looks like:
+    args = (message,), .response is the Response."""
+    response = MagicMock()
+    response.status_code = status_code
+    response.json.return_value = body
+    exc = Exception(f"{status_code} Client Error")
+    exc.response = response
+    return exc
+
+
+class BallchasingDuplicateReplayHandlingTests(unittest.TestCase):
+    def test_duplicate_replay_409_takes_fast_path_without_indexerror(self):
+        """409 + body {error: 'duplicate replay', id: ...} should short-circuit
+        the upload loop and proceed to fetch the existing replay's stats."""
+        api = MagicMock()
+        api.upload_replay.side_effect = _make_http_error(
+            409, {"error": "duplicate replay", "id": "existing-replay-id"}
+        )
+        api.get_replay.return_value = {"status": "ok", "id": "existing-replay-id"}
+
+        on_progress = MagicMock()
+
+        with patch.object(replay_parser, "get_ballchasing_api", return_value=api), \
+                patch.object(replay_parser, "DELAYS", [0]), \
+                patch.object(replay_parser, "MAX_RETRIES", 1), \
+                patch.object(replay_parser, "sleep"):
+            replay_path = str(SERVICE_ROOT / "17A7C1084017DFA7DBE66D9C66D81CBD.replay")
+            result = replay_parser._parse_ballchasing(replay_path, on_progress)
+
+        self.assertEqual(result, {"status": "ok", "id": "existing-replay-id"})
+        api.upload_replay.assert_called_once()
+        api.get_replay.assert_called_once_with("existing-replay-id")
+
+    def test_failed_replay_400_reraises_without_indexerror(self):
+        """400 (failed replay) should re-raise the original exception,
+        not crash with IndexError."""
+        api = MagicMock()
+        original_exc = _make_http_error(400, {"error": "bad replay"})
+        api.upload_replay.side_effect = original_exc
+
+        on_progress = MagicMock()
+
+        with patch.object(replay_parser, "get_ballchasing_api", return_value=api), \
+                patch.object(replay_parser, "DELAYS", [0, 1]), \
+                patch.object(replay_parser, "MAX_RETRIES", 2), \
+                patch.object(replay_parser, "sleep"):
+            replay_path = str(SERVICE_ROOT / "17A7C1084017DFA7DBE66D9C66D81CBD.replay")
+            with self.assertRaises(Exception) as cm:
+                replay_parser._parse_ballchasing(replay_path, on_progress)
+
+        # Must be the original 400, not an IndexError
+        self.assertNotIsInstance(cm.exception, IndexError)
+        self.assertIs(cm.exception, original_exc)
+
+    def test_unknown_exception_without_response_is_reraised(self):
+        """Exceptions with no .response (e.g. ConnectionError) should propagate
+        unchanged rather than be silently misinterpreted as a known status."""
+        api = MagicMock()
+        api.upload_replay.side_effect = ConnectionError("network is down")
+
+        on_progress = MagicMock()
+
+        with patch.object(replay_parser, "get_ballchasing_api", return_value=api), \
+                patch.object(replay_parser, "DELAYS", [0]), \
+                patch.object(replay_parser, "MAX_RETRIES", 1), \
+                patch.object(replay_parser, "sleep"):
+            replay_path = str(SERVICE_ROOT / "17A7C1084017DFA7DBE66D9C66D81CBD.replay")
+            with self.assertRaises(ConnectionError):
+                replay_parser._parse_ballchasing(replay_path, on_progress)
+
+    def test_get_replay_rate_limit_429_retries_without_indexerror(self):
+        """429 from get_replay should be detected as rate limit and trigger
+        a retry rather than crashing with IndexError."""
+        api = MagicMock()
+        api.upload_replay.return_value = {"id": "fresh-id"}
+        api.get_replay.side_effect = [
+            _make_http_error(429, {}),
+            {"status": "ok", "id": "fresh-id"},
+        ]
+
+        on_progress = MagicMock()
+
+        with patch.object(replay_parser, "get_ballchasing_api", return_value=api), \
+                patch.object(replay_parser, "DELAYS", [0, 1]), \
+                patch.object(replay_parser, "MAX_RETRIES", 2), \
+                patch.object(replay_parser, "sleep"):
+            replay_path = str(SERVICE_ROOT / "17A7C1084017DFA7DBE66D9C66D81CBD.replay")
+            result = replay_parser._parse_ballchasing(replay_path, on_progress)
+
+        self.assertEqual(result["status"], "ok")
+        self.assertEqual(api.get_replay.call_count, 2)
+
+
+if __name__ == "__main__":
+    unittest.main()