fix(namegen): retry transient upstream failures in provisioning

src/mud_server/services/character_provisioning.py

@@ -29,6 +29,11 @@
 
 logger = logging.getLogger(__name__)
 
+# Keep retries intentionally small to avoid extending admin provisioning latency
+# too much during degraded upstream conditions.
+_NAMEGEN_MAX_ATTEMPTS = 3
+_NAMEGEN_RETRYABLE_HTTP_STATUS_CODES = frozenset({408, 429, 500, 502, 503, 504})
+
 
 @dataclass(slots=True)
 class CharacterProvisioningResult:
@@ -76,6 +81,14 @@ def _fetch_generated_name(
     """
     Request one name token from the external name-generation API.
 
+    Retry policy:
+        We retry only transient upstream failures:
+        1. Request-layer exceptions (timeout/connection/reset).
+        2. Retryable HTTP statuses (408/429/5xx).
+
+        We intentionally do not retry schema/validation errors because those
+        indicate contract drift rather than transient transport issues.
+
     Returns:
         Tuple ``(name, error_message)``. On success, ``error_message`` is None.
     """
@@ -98,15 +111,54 @@ def _fetch_generated_name(
         "seed": seed,
     }
 
-    try:
-        response = requests.post(
-            endpoint,
-            json=payload,
-            timeout=config.integrations.namegen_timeout_seconds,
-        )
+    last_status_code: int | None = None
+    last_request_error: Exception | None = None
+
+    for attempt in range(1, _NAMEGEN_MAX_ATTEMPTS + 1):
+        try:
+            response = requests.post(
+                endpoint,
+                json=payload,
+                timeout=config.integrations.namegen_timeout_seconds,
+            )
+        except requests.exceptions.RequestException as exc:
+            last_request_error = exc
+            if attempt < _NAMEGEN_MAX_ATTEMPTS:
+                logger.warning(
+                    "Name generation API retry attempt=%s/%s reason=request_exception error=%s",
+                    attempt,
+                    _NAMEGEN_MAX_ATTEMPTS,
+                    exc,
+                )
+                continue
+            logger.warning(
+                "Name generation API request failed after %s attempts: %s",
+                attempt,
+                exc,
+            )
+            return None, "Name generation API unavailable."
+
         if response.status_code != 200:
+            last_status_code = response.status_code
+            if (
+                response.status_code in _NAMEGEN_RETRYABLE_HTTP_STATUS_CODES
+                and attempt < _NAMEGEN_MAX_ATTEMPTS
+            ):
+                logger.warning(
+                    "Name generation API retry attempt=%s/%s reason=http_status status_code=%s",
+                    attempt,
+                    _NAMEGEN_MAX_ATTEMPTS,
+                    response.status_code,
+                )
+                continue
             return None, f"Name generation API returned HTTP {response.status_code}."
-        body = response.json()
+
+        try:
+            body = response.json()
+        except ValueError:
+            logger.warning("Name generation API returned invalid JSON.")
+            return None, "Name generation API returned invalid JSON."
+
         if not isinstance(body, dict):
             return None, "Name generation API returned a non-object payload."
         names = body.get("names")
@@ -116,12 +168,13 @@ def _fetch_generated_name(
         if not generated_name:
             return None, "Name generation API returned an empty name."
         return generated_name, None
-    except requests.exceptions.RequestException as exc:
-        logger.warning("Name generation API request failed: %s", exc)
+
+    if last_status_code is not None:
+        return None, f"Name generation API returned HTTP {last_status_code}."
+    if last_request_error is not None:
+        logger.warning("Name generation API request failed: %s", last_request_error)
         return None, "Name generation API unavailable."
-    except ValueError:
-        logger.warning("Name generation API returned invalid JSON.")
-        return None, "Name generation API returned invalid JSON."
+    return None, "Name generation API unavailable."
 
 
 def fetch_generated_full_name(seed: int) -> tuple[str | None, str | None]:

tests/test_api/test_admin_create_character.py

@@ -77,20 +77,42 @@ def test_fetch_generated_name_handles_empty_base_url(monkeypatch):
 
 @pytest.mark.unit
 def test_fetch_generated_name_handles_non_200(monkeypatch):
-    """Name helper should surface upstream HTTP status failures."""
+    """Retryable HTTP statuses should be retried before surfacing failure."""
     monkeypatch.setattr(character_provisioning.config.integrations, "namegen_enabled", True)
     monkeypatch.setattr(
         character_provisioning.config.integrations, "namegen_base_url", "https://name.example.org"
     )
 
     fake_response = Mock(status_code=503)
     fake_response.json.return_value = {}
-    monkeypatch.setattr(character_provisioning.requests, "post", Mock(return_value=fake_response))
+    post_mock = Mock(return_value=fake_response)
+    monkeypatch.setattr(character_provisioning.requests, "post", post_mock)
 
     name, error = character_provisioning._fetch_generated_name(seed=12)
 
     assert name is None
     assert error == "Name generation API returned HTTP 503."
+    assert post_mock.call_count == 3
+
+
+@pytest.mark.unit
+def test_fetch_generated_name_does_not_retry_on_non_retryable_http_status(monkeypatch):
+    """Non-retryable HTTP statuses should fail immediately without extra calls."""
+    monkeypatch.setattr(character_provisioning.config.integrations, "namegen_enabled", True)
+    monkeypatch.setattr(
+        character_provisioning.config.integrations, "namegen_base_url", "https://name.example.org"
+    )
+
+    fake_response = Mock(status_code=400)
+    fake_response.json.return_value = {}
+    post_mock = Mock(return_value=fake_response)
+    monkeypatch.setattr(character_provisioning.requests, "post", post_mock)
+
+    name, error = character_provisioning._fetch_generated_name(seed=19)
+
+    assert name is None
+    assert error == "Name generation API returned HTTP 400."
+    post_mock.assert_called_once()
 
 
 @pytest.mark.unit
@@ -149,13 +171,15 @@ def test_fetch_generated_name_handles_blank_name(monkeypatch):
 
 @pytest.mark.unit
 def test_fetch_generated_name_handles_request_exception(monkeypatch):
-    """Network-layer errors should become an availability message."""
+    """Request exceptions should retry and then map to availability errors."""
     monkeypatch.setattr(character_provisioning.config.integrations, "namegen_enabled", True)
     monkeypatch.setattr(
         character_provisioning.config.integrations, "namegen_base_url", "https://name.example.org"
     )
+    attempts = {"count": 0}
 
     def _boom(*_args, **_kwargs):  # noqa: ANN002,ANN003 - test double
+        attempts["count"] += 1
         raise character_provisioning.requests.exceptions.RequestException("boom")
 
     monkeypatch.setattr(character_provisioning.requests, "post", _boom)
@@ -164,6 +188,58 @@ def _boom(*_args, **_kwargs):  # noqa: ANN002,ANN003 - test double
 
     assert name is None
     assert error == "Name generation API unavailable."
+    assert attempts["count"] == 3
+
+
+@pytest.mark.unit
+def test_fetch_generated_name_retries_request_exception_then_succeeds(monkeypatch):
+    """Transient request exceptions should succeed when a later attempt succeeds."""
+    monkeypatch.setattr(character_provisioning.config.integrations, "namegen_enabled", True)
+    monkeypatch.setattr(
+        character_provisioning.config.integrations, "namegen_base_url", "https://name.example.org"
+    )
+    attempts = {"count": 0}
+
+    def _post_mock(*_args, **_kwargs):  # noqa: ANN002,ANN003 - test double
+        attempts["count"] += 1
+        if attempts["count"] < 3:
+            raise character_provisioning.requests.exceptions.RequestException("temporary")
+        response = Mock(status_code=200)
+        response.json.return_value = {"names": ["Kira"]}
+        return response
+
+    monkeypatch.setattr(character_provisioning.requests, "post", _post_mock)
+
+    name, error = character_provisioning._fetch_generated_name(seed=72)
+
+    assert error is None
+    assert name == "Kira"
+    assert attempts["count"] == 3
+
+
+@pytest.mark.unit
+def test_fetch_generated_name_retries_retryable_http_status_then_succeeds(monkeypatch):
+    """Retryable status failures should be retried before succeeding."""
+    monkeypatch.setattr(character_provisioning.config.integrations, "namegen_enabled", True)
+    monkeypatch.setattr(
+        character_provisioning.config.integrations, "namegen_base_url", "https://name.example.org"
+    )
+    responses = [
+        Mock(status_code=503),
+        Mock(status_code=504),
+        Mock(status_code=200),
+    ]
+    responses[0].json.return_value = {}
+    responses[1].json.return_value = {}
+    responses[2].json.return_value = {"names": ["Riven"]}
+    post_mock = Mock(side_effect=responses)
+    monkeypatch.setattr(character_provisioning.requests, "post", post_mock)
+
+    name, error = character_provisioning._fetch_generated_name(seed=917)
+
+    assert error is None
+    assert name == "Riven"
+    assert post_mock.call_count == 3
 
 
 @pytest.mark.unit