fix: surface experiment-comparison2 fetch failures and retry transient 5xx

py/src/braintrust/logger.py

@@ -728,16 +728,23 @@ def set_http_adapter(adapter: HTTPAdapter) -> None:
         _state._api_conn._reset()
 
 
+#: HTTP status codes that indicate a transient failure worth retrying, rather than a
+#: client error that will fail identically on every attempt.
+RETRYABLE_STATUS_CODES = frozenset({429, 500, 502, 503, 504})
+
+
 class RetryRequestExceptionsAdapter(HTTPAdapter):
     """An HTTP adapter that automatically retries requests on connection exceptions.
 
     This adapter extends requests' HTTPAdapter to add retry logic for common network-related
-    exceptions including connection errors, timeouts, and other HTTP errors. It implements
-    an exponential backoff strategy between retries to avoid overwhelming servers during
-    intermittent connectivity issues.
+    exceptions including connection errors, timeouts, and other HTTP errors, as well as
+    responses that complete with a transient HTTP error status (see `RETRYABLE_STATUS_CODES`).
+    It implements an exponential backoff strategy between retries to avoid overwhelming
+    servers during intermittent connectivity issues.
 
     Attributes:
-        base_num_retries: Maximum number of retries before giving up and re-raising the exception.
+        base_num_retries: Maximum number of retries before giving up and re-raising the exception
+                          (or returning the last error response).
         backoff_factor: A multiplier used to determine the time to wait between retries.
                        The actual wait time is calculated as: backoff_factor * (2 ** retry_count).
         default_timeout_secs: Default timeout in seconds for requests that don't specify one.
@@ -770,6 +777,21 @@ def send(self, *args, **kwargs):
                 # downloading.
                 if not response.is_redirect and response.content:
                     pass
+                if response.status_code in RETRYABLE_STATUS_CODES and num_prev_retries < self.base_num_retries:
+                    # Unlike connection-level failures, a completed response with an error
+                    # status doesn't raise -- retry it here so transient 5xx/429 responses
+                    # get the same backoff treatment as network exceptions.
+                    sleep_s = self.backoff_factor * (2**num_prev_retries)
+                    print(
+                        "Retrying request after HTTP",
+                        response.status_code,
+                        "response",
+                        file=sys.stderr,
+                    )
+                    print("Sleeping for", sleep_s, "seconds", file=sys.stderr)
+                    time.sleep(sleep_s)
+                    num_prev_retries += 1
+                    continue
                 return response
             except (urllib3.exceptions.HTTPError, requests.exceptions.RequestException) as e:
                 if num_prev_retries < self.base_num_retries:
@@ -4362,6 +4384,7 @@ def summarize(
         score_summary = {}
         metric_summary = {}
         comparison_experiment_name = None
+        scores_fetch_error = None
         if summarize_scores:
             # Get the comparison experiment
             if comparison_experiment_id is None:
@@ -4385,6 +4408,7 @@ def summarize(
                     },
                 )
             except Exception as e:
+                scores_fetch_error = str(e)
                 _logger.warning(
                     f"Failed to fetch experiment scores and metrics: {e}\n\nView complete results in Braintrust or run experiment.summarize() again."
                 )
@@ -4413,6 +4437,7 @@ def summarize(
             comparison_experiment_name=comparison_experiment_name,
             scores=score_summary,
             metrics=metric_summary,
+            scores_fetch_error=scores_fetch_error,
         )
 
     def export(self) -> str:
@@ -6002,13 +6027,21 @@ class ExperimentSummary(SerializableDataClass):
     """Summary of the experiment's scores."""
     metrics: dict[str, MetricSummary]
     """Summary of the experiment's metrics."""
+    scores_fetch_error: str | None = None
+    """If set, fetching the score/metric summary from the server failed with this error, and
+    `scores`/`metrics` are empty as a result of that failure -- not because the experiment has
+    no scores. Callers that gate automation (e.g. CI) on `scores` should check this field before
+    treating an empty `scores` dict as "the experiment has no scores"."""
 
     def __str__(self):
         comparison_line = ""
         if self.comparison_experiment_name:
             comparison_line = f"""{self.experiment_name} compared to {self.comparison_experiment_name}:\n"""
+        fetch_error_line = ""
+        if self.scores_fetch_error:
+            fetch_error_line = f"WARNING: failed to fetch scores and metrics ({self.scores_fetch_error}). The summary below does not reflect the experiment's actual scores.\n\n"
         return (
-            f"""\n=========================SUMMARY=========================\n{comparison_line}"""
+            f"""\n=========================SUMMARY=========================\n{fetch_error_line}{comparison_line}"""
             + "\n".join([str(score) for score in self.scores.values()])
             + ("\n\n" if self.scores else "")
             + "\n".join([str(metric) for metric in self.metrics.values()])

py/src/braintrust/test_framework.py

@@ -5,6 +5,7 @@
 
 import pytest
 from braintrust.logger import BraintrustState
+from braintrust.util import AugmentedHTTPError
 
 from .framework import (
     Eval,
@@ -164,6 +165,34 @@ def get_json(path, args=None):
     )
 
 
+def test_experiment_summarize_surfaces_scores_fetch_error(with_memory_logger, with_simulate_login):
+    """A failure to fetch experiment-comparison2 must be distinguishable from a genuine
+    empty-scores result, not silently collapsed into the same `scores == {}` shape.
+
+    See https://github.com/braintrustdata/braintrust-sdk-python/issues/639.
+    """
+    exp = init_test_exp("test-evaluator", "test-project")
+    mock_conn = MagicMock()
+
+    def get_json(path, args=None):
+        if path == "v1/experiment/base-exp-id":
+            return {"name": "base-exp"}
+        if path == "experiment-comparison2":
+            raise AugmentedHTTPError("<html>502 Bad Gateway</html>")
+        raise AssertionError(f"Unexpected get_json call: {path}, {args}")
+
+    mock_conn.get_json.side_effect = get_json
+
+    with patch.object(exp.state, "api_conn", return_value=mock_conn):
+        summary = exp.summarize(comparison_experiment_id="base-exp-id")
+
+    assert summary.scores == {}
+    assert summary.metrics == {}
+    assert summary.scores_fetch_error is not None
+    assert "502 Bad Gateway" in summary.scores_fetch_error
+    assert "WARNING" in str(summary)
+
+
 @pytest.mark.asyncio
 @pytest.mark.skipif(not HAS_PYDANTIC, reason="pydantic not installed")
 async def test_run_evaluator_exposes_validated_parameter_values_to_hooks():

py/src/braintrust/test_http.py

@@ -174,6 +174,105 @@ def test_adapter_resets_pool_on_timeout(self, hanging_server):
         assert HangingConnectionHandler.request_count >= 2
 
 
+class TransientErrorStatusHandler(http.server.BaseHTTPRequestHandler):
+    """HTTP handler that returns a transient error status for the first N requests.
+
+    Simulates a load balancer/proxy hiccup (502/503/504) that completes with a normal
+    HTTP response rather than raising a connection-level exception -- this does not enter
+    the exception-handling retry path, only a status-code-aware one.
+    """
+
+    request_count = 0
+    fail_count = 1
+    error_status = 502
+
+    def log_message(self, format, *args):
+        pass
+
+    def do_GET(self):
+        TransientErrorStatusHandler.request_count += 1
+
+        if TransientErrorStatusHandler.request_count <= TransientErrorStatusHandler.fail_count:
+            self.send_response(TransientErrorStatusHandler.error_status)
+            self.send_header("Content-Type", "text/html")
+            self.end_headers()
+            self.wfile.write(b"<html>Bad Gateway</html>")
+            return
+
+        self.send_response(200)
+        self.send_header("Content-Type", "application/json")
+        self.end_headers()
+        self.wfile.write(b'{"status": "ok"}')
+
+
+@pytest.fixture
+def transient_error_server():
+    """Fixture that creates a server returning an error status for the first request."""
+    TransientErrorStatusHandler.request_count = 0
+    TransientErrorStatusHandler.fail_count = 1
+    TransientErrorStatusHandler.error_status = 502
+
+    server = socketserver.ThreadingTCPServer(("127.0.0.1", 0), TransientErrorStatusHandler)
+    server.daemon_threads = True
+    port = server.server_address[1]
+
+    thread = threading.Thread(target=server.serve_forever)
+    thread.daemon = True
+    thread.start()
+
+    yield f"http://127.0.0.1:{port}"
+
+    server.shutdown()
+    server.server_close()
+
+
+class TestRetryOnHttpErrorStatus:
+    """Tests that the adapter retries completed responses with a transient error status.
+
+    Regression coverage for https://github.com/braintrustdata/braintrust-sdk-python/issues/639:
+    a request that completes with a non-2xx status returns a normal Response rather than
+    raising, so it must be retried by inspecting the status code, not just by catching
+    connection-level exceptions.
+    """
+
+    def test_adapter_retries_on_502(self, transient_error_server):
+        adapter = RetryRequestExceptionsAdapter(base_num_retries=3, backoff_factor=0.05)
+        session = requests.Session()
+        session.mount("http://", adapter)
+
+        resp = session.get(f"{transient_error_server}/experiment-comparison2")
+
+        assert resp.status_code == 200
+        assert resp.json() == {"status": "ok"}
+        assert TransientErrorStatusHandler.request_count == 2
+
+    def test_adapter_gives_up_after_base_num_retries(self, transient_error_server):
+        TransientErrorStatusHandler.fail_count = 100  # always fails
+
+        adapter = RetryRequestExceptionsAdapter(base_num_retries=2, backoff_factor=0.01)
+        session = requests.Session()
+        session.mount("http://", adapter)
+
+        resp = session.get(f"{transient_error_server}/experiment-comparison2")
+
+        # 1 initial attempt + 2 retries, then the caller gets the last error response back.
+        assert resp.status_code == 502
+        assert TransientErrorStatusHandler.request_count == 3
+
+    def test_adapter_does_not_retry_non_retryable_status(self, transient_error_server):
+        TransientErrorStatusHandler.fail_count = 100
+        TransientErrorStatusHandler.error_status = 404
+
+        adapter = RetryRequestExceptionsAdapter(base_num_retries=5, backoff_factor=0.01)
+        session = requests.Session()
+        session.mount("http://", adapter)
+
+        resp = session.get(f"{transient_error_server}/experiment-comparison2")
+
+        assert resp.status_code == 404
+        assert TransientErrorStatusHandler.request_count == 1
+
+
 class TestHTTPConnection:
     """Tests for HTTPConnection timeout configuration."""