feat(python-sdk): make REST transport retries configurable via TenacityConfig

Author: BloggerBust

sdks/python/hatchet_sdk/clients/rest/tenacity_utils.py

@@ -21,7 +21,7 @@
 R = TypeVar("R")
 
 # Pattern to extract HTTP method from exception reason
-_METHOD_PATTERN = re.compile(r"method=(\w+)", re.IGNORECASE)
+_METHOD_PATTERN = re.compile(r"\bmethod=(\w+)\b", re.IGNORECASE)
 
 
 def tenacity_retry(func: Callable[P, R], config: TenacityConfig) -> Callable[P, R]:

sdks/python/tests/test_tenacity_transport_retry.py

@@ -4,6 +4,7 @@
 1. Default behavior: RestTransportError is NOT retried (even for GET)
 2. Opt-in behavior: RestTransportError retried for configured methods only
 3. Existing HTTP error retry behavior unchanged
+4. Method extraction from exception reason strings
 """
 
 import pytest
@@ -17,81 +18,50 @@
     RestTransportError,
     ServiceException,
 )
-from hatchet_sdk.clients.rest.tenacity_utils import tenacity_should_retry
+from hatchet_sdk.clients.rest.tenacity_utils import (
+    _extract_method_from_reason,
+    tenacity_should_retry,
+)
 from hatchet_sdk.config import TenacityConfig
 
 # --- Default behavior tests (transport errors NOT retried) ---
 
 
-def test_default__rest_transport_error_not_retried() -> None:
-    """By default, RestTransportError should NOT be retried."""
-    exc = RestTransportError(status=0, reason="method=GET\nurl=http://test")
-    config = TenacityConfig()
-    assert tenacity_should_retry(exc, config) is False
-
-
-def test_default__rest_timeout_error_not_retried() -> None:
-    """By default, RestTimeoutError should NOT be retried."""
-    exc = RestTimeoutError(status=0, reason="method=GET\nurl=http://test")
-    config = TenacityConfig()
-    assert tenacity_should_retry(exc, config) is False
-
-
-def test_default__rest_connection_error_not_retried() -> None:
-    """By default, RestConnectionError should NOT be retried."""
-    exc = RestConnectionError(status=0, reason="method=GET\nurl=http://test")
-    config = TenacityConfig()
-    assert tenacity_should_retry(exc, config) is False
-
-
-def test_default__rest_tls_error_not_retried() -> None:
-    """By default, RestTLSError should NOT be retried."""
-    exc = RestTLSError(status=0, reason="method=GET\nurl=http://test")
-    config = TenacityConfig()
-    assert tenacity_should_retry(exc, config) is False
-
-
-def test_default__rest_protocol_error_not_retried() -> None:
-    """By default, RestProtocolError should NOT be retried."""
-    exc = RestProtocolError(status=0, reason="method=GET\nurl=http://test")
+@pytest.mark.parametrize(
+    "exc_class",
+    [RestTransportError, RestTimeoutError],
+    ids=["base-class", "subclass"],
+)
+def test_default__transport_errors_not_retried(exc_class: type) -> None:
+    """By default, RestTransportError and subclasses should not be retried."""
+    exc = exc_class(status=0, reason="method=GET\nurl=http://test")
     config = TenacityConfig()
     assert tenacity_should_retry(exc, config) is False
 
 
 # --- Opt-in behavior tests (transport errors retried for allowed methods) ---
 
 
-def test_optin__get_request_retried() -> None:
-    """When enabled, GET requests with transport errors should be retried."""
-    exc = RestTimeoutError(status=0, reason="method=GET\nurl=http://test")
-    config = TenacityConfig(retry_transport_errors=True)
-    assert tenacity_should_retry(exc, config) is True
-
-
-def test_optin__delete_request_retried() -> None:
-    """When enabled, DELETE requests with transport errors should be retried."""
-    exc = RestConnectionError(status=0, reason="method=DELETE\nurl=http://test")
+@pytest.mark.parametrize(
+    "method",
+    ["GET", "DELETE"],
+    ids=["get", "delete"],
+)
+def test_optin__idempotent_methods_retried(method: str) -> None:
+    """When enabled, GET and DELETE requests with transport errors should be retried."""
+    exc = RestTimeoutError(status=0, reason=f"method={method}\nurl=http://test")
     config = TenacityConfig(retry_transport_errors=True)
     assert tenacity_should_retry(exc, config) is True
 
 
-def test_optin__post_request_not_retried() -> None:
-    """POST requests should NOT be retried even when transport retry is enabled."""
-    exc = RestTimeoutError(status=0, reason="method=POST\nurl=http://test")
-    config = TenacityConfig(retry_transport_errors=True)
-    assert tenacity_should_retry(exc, config) is False
-
-
-def test_optin__put_request_not_retried() -> None:
-    """PUT requests should NOT be retried even when transport retry is enabled."""
-    exc = RestConnectionError(status=0, reason="method=PUT\nurl=http://test")
-    config = TenacityConfig(retry_transport_errors=True)
-    assert tenacity_should_retry(exc, config) is False
-
-
-def test_optin__patch_request_not_retried() -> None:
-    """PATCH requests should NOT be retried even when transport retry is enabled."""
-    exc = RestProtocolError(status=0, reason="method=PATCH\nurl=http://test")
+@pytest.mark.parametrize(
+    "method",
+    ["POST", "PUT", "PATCH"],
+    ids=["post", "put", "patch"],
+)
+def test_optin__non_idempotent_methods_not_retried(method: str) -> None:
+    """Non-idempotent requests should not be retried even when transport retry is enabled."""
+    exc = RestTimeoutError(status=0, reason=f"method={method}\nurl=http://test")
     config = TenacityConfig(retry_transport_errors=True)
     assert tenacity_should_retry(exc, config) is False
 
@@ -101,65 +71,88 @@ def test_optin__custom_methods_list() -> None:
     exc = RestTimeoutError(status=0, reason="method=POST\nurl=http://test")
     config = TenacityConfig(
         retry_transport_errors=True,
-        retry_transport_methods=["POST"],  # allow POST explicitly
+        retry_transport_methods=["POST"],
     )
     assert tenacity_should_retry(exc, config) is True
 
 
-def test_optin__custom_methods_excludes_get() -> None:
-    """Custom retry_transport_methods can exclude GET."""
+def test_optin__custom_methods_excludes_default() -> None:
+    """Custom retry_transport_methods can exclude default methods like GET."""
     exc = RestTimeoutError(status=0, reason="method=GET\nurl=http://test")
     config = TenacityConfig(
         retry_transport_errors=True,
-        retry_transport_methods=["DELETE"],  # only DELETE, not GET
+        retry_transport_methods=["DELETE"],
     )
     assert tenacity_should_retry(exc, config) is False
 
 
 # --- Regression tests: existing HTTP error retry behavior unchanged ---
 
 
-def test_regression__service_exception_still_retried() -> None:
-    """ServiceException (5xx) should still be retried."""
-    exc = ServiceException(status=500, reason="Internal Server Error")
-    config = TenacityConfig()
-    assert tenacity_should_retry(exc, config) is True
-
-
-def test_regression__not_found_exception_still_retried() -> None:
-    """NotFoundException (404) should still be retried."""
-    exc = NotFoundException(status=404, reason="Not Found")
+@pytest.mark.parametrize(
+    ("exc", "desc"),
+    [
+        (ServiceException(status=500, reason="Internal Server Error"), "5xx"),
+        (NotFoundException(status=404, reason="Not Found"), "404"),
+    ],
+    ids=["service-exception", "not-found"],
+)
+def test_regression__http_errors_still_retried(exc: Exception, desc: str) -> None:
+    """ServiceException (5xx) and NotFoundException (404) should still be retried."""
     config = TenacityConfig()
     assert tenacity_should_retry(exc, config) is True
 
 
-def test_regression__service_exception_retried_without_config() -> None:
+def test_regression__backward_compat_no_config() -> None:
     """ServiceException should be retried even without config (backward compat)."""
     exc = ServiceException(status=500, reason="Internal Server Error")
-    # Call without config to test backward compatibility
     assert tenacity_should_retry(exc) is True
 
 
-# --- Edge cases ---
-
-
-def test_edge__missing_method_in_reason_not_retried() -> None:
-    """If method cannot be extracted from reason, should not retry."""
-    exc = RestTimeoutError(status=0, reason="some error without method")
-    config = TenacityConfig(retry_transport_errors=True)
-    assert tenacity_should_retry(exc, config) is False
+# --- Unit tests for _extract_method_from_reason ---
+
+
+@pytest.mark.parametrize(
+    ("reason", "expected"),
+    [
+        ("method=GET\nurl=http://test", "GET"),
+        ("method=POST\nurl=http://test", "POST"),
+        ("method=delete\nurl=http://test", "delete"),
+        ("prefix method=PUT suffix", "PUT"),
+        ("some error without method", None),
+        ("method=\nurl=http://test", None),
+        ("", None),
+        (None, None),
+    ],
+    ids=[
+        "get-uppercase",
+        "post-uppercase",
+        "lowercase-preserved",
+        "embedded-in-text",
+        "no-method-field",
+        "empty-method-value",
+        "empty-string",
+        "none",
+    ],
+)
+def test_extract_method__parses_reason(
+    reason: str | None, expected: str | None
+) -> None:
+    """_extract_method_from_reason should correctly parse HTTP method from reason."""
+    assert _extract_method_from_reason(reason) == expected
 
 
-def test_edge__empty_reason_not_retried() -> None:
-    """If reason is empty, should not retry."""
-    exc = RestConnectionError(status=0, reason="")
-    config = TenacityConfig(retry_transport_errors=True)
-    assert tenacity_should_retry(exc, config) is False
+# --- Edge cases for retry behavior ---
 
 
-def test_edge__none_reason_not_retried() -> None:
-    """If reason is None, should not retry."""
-    exc = RestTLSError(status=0, reason=None)
+@pytest.mark.parametrize(
+    "reason",
+    ["some error without method", "", None],
+    ids=["no-method-field", "empty-string", "none"],
+)
+def test_edge__unparseable_reason_not_retried(reason: str | None) -> None:
+    """If method cannot be extracted from reason, should not retry."""
+    exc = RestTimeoutError(status=0, reason=reason)
     config = TenacityConfig(retry_transport_errors=True)
     assert tenacity_should_retry(exc, config) is False