forked from HAL9000/cleveragents-core
4d3499dcfb
Wire per-service retry policies and circuit breakers into the service layer via ServiceRetryWiring, backed by ServiceRetryPolicyRegistry and configurable through Settings environment variables. Production hardening from code review: - Fix TOCTOU race in CircuitBreaker._on_success (half-open state) - Add half-open probe limit to prevent unbounded concurrent requests - Track all exception types for circuit breaker failure counting - Detect async callables wrapped in functools.partial and callable objects - Enforce spec-compliant 2s minimum for linear backoff strategy - Enforce 0.1s floor for fixed backoff strategy - Add retry amplification guard via contextvars nesting depth tracking - Cap total retry wall-clock time at 300s (MAX_RETRY_TOTAL_TIMEOUT) - Sanitize exception messages in retry logs to prevent secret leakage - Fix wrap_service_method TOCTOU by holding cache lock for full operation - Deep-copy default policies to prevent cross-policy mutation - Warn on unknown override keys in apply_overrides - Guard apply_overrides against non-dict and deeply nested JSON values - Read circuit breaker state under lock in is_circuit_open - Catch RecursionError in JSON config parsing - Add total_timeout + nesting guard to retry_service_operation decorator - Extend secret sanitization to Authorization headers, private_key, connection_string, and access_key patterns - Enforce 0.1s floor on jitter backoff strategy - Cache wait strategies per service in ServiceRetryWiring (M3) - Reset failure_count to 0 when entering half-open from open (M6) - Use cached _get_wait_strategy() in execute()/async_execute() - Move circuit-open logging out of _on_failure lock scope to prevent holding the lock during potentially slow I/O (F1) - Pass total_timeout=MAX_RETRY_TOTAL_TIMEOUT to wrap_service_method retry_service_operation call for consistency with execute() (F4) - Capture failure_count into local variable inside lock scope before logging outside the lock, preventing stale reads from concurrent threads in CircuitBreaker.call() and async_call() (F1) - Deep-copy module-level DEFAULT_DATABASE_RETRY and DEFAULT_CIRCUIT_BREAKER in ServiceRetryPolicyRegistry.get() for auto-generated unknown service policies, preventing shared mutable state corruption (F1) - Unify CircuitBreaker to a single threading.Lock for sync and async (P1-1) - Restore BaseException permit in half-open path to prevent permit leak (P1-6) - Prevent CircuitBreakerOpen cascading into failure_count (S2) - Protect all logger calls with contextlib.suppress (S3, S4) - Replace time.time() with time.monotonic() for monotonic timing (S5) - Add distinct log events for half-open and closed transitions (S11, S12) - Track pre-existing services so second apply_settings_defaults only targets newly registered services (P1-2) - Lazy circuit breaker creation via _get_or_create_cb() (P1-3) - Reject async callables in sync execute() with TypeError (P1-5) - Strengthen retry predicate to retry_if_exception_type(Exception) & retry_if_not_exception_type(CircuitBreakerOpen) (S1) - Add lock on _get_wait_strategy cache access (P2-16) - Truncate raw JSON to 80 chars in override warning (P2-17) - Warn on non-dict JSON overrides (P2-29) - Debug log for nesting guard bypass (S13) - Deep-copy from get() and all_policies() in registry (P1-4) - Thread-safe registry with threading.Lock (P2-15) - Robust exception handling in apply_overrides get() (P2-18) - Log ValidationError details on override failure (P2-19) - Sanitize service_name via _safe_service_name() (P2-28) - Warn on non-dict sub-key values in overrides (P2-30) - Allowlist for is_read_only_plan_operation phases (P2-10) - Cap retry_auto_debug sleep at 60s (P2-11) - Use is-not-None instead of falsy checks for error values (P2-12) - Extend secret regex with bearer, session_id, auth_token, refresh_token, client_secret patterns (P2-25) - Pre-truncate error messages to 2000 chars before regex (P2-26) - Add upper bounds on retry Settings fields (P2-7) - Add cross-field validator max_delay >= base_delay (P2-8) - Case-insensitive backoff strategy validation (P2-21) - Add half_open_max_successes setting (S10) - Remove phantom ContextFragment from services __all__ (ImportError fix) - Export ServiceRetryWiring from application.services package - Include sanitised error context in TypeError logging fallback - Initialise RetryContext.attempt_count to 1 for bare context-manager usage - Introduce CircuitBreakerState StrEnum replacing raw string literals - Fix vacuous CircuitBreakerOpen propagation assertions in BDD steps - Replace tautological logging test with structlog capture verification - Assert circuit breaker existence instead of silently skipping on None - Add Unicode control-char rejection validator to ServiceRetryPolicy.service_name - Add name parameter with service= in all log calls - Add extra="forbid" to all 3 Pydantic models - Deep-copy _SERVICE_DEFAULTS construction - Key normalisation (.strip()) in get() and apply_overrides() - Add cooldown <= recovery_timeout validator - Async guard on RetryContext.execute() - Nesting guard on RetryContext.execute()/async_execute() - stop_after_delay(300.0) on RetryContext - retry_auto_debug async-only guard, dict result fix, sleep guard - Retry-attempt logging in RetryContext - Module-level docs for contextlib.suppress(TypeError) rationale - Exhaustion log on retry failure - Startup log in __init__; name=service_name to CircuitBreaker - log_after_retry guarded to not fire on first-attempt success - get_retry_decorator now includes logging callbacks - Changed retry_backoff_strategy from str to RetryStrategy StrEnum Closes #313
441 lines
15 KiB
Python
441 lines
15 KiB
Python
"""Step definitions for retry decorator and utility tests."""
|
|
|
|
from __future__ import annotations
|
|
|
|
import contextlib
|
|
from typing import Any
|
|
|
|
from behave import given, then, when
|
|
|
|
from cleveragents.core.retry_patterns import (
|
|
_build_wait_strategy,
|
|
is_read_only_plan_operation,
|
|
)
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Read-only plan operation guard
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@given("I have the read-only plan operation guard")
|
|
def step_have_guard(context: Any) -> None:
|
|
context.guard_func = is_read_only_plan_operation
|
|
|
|
|
|
@when("I check a read_only=True operation")
|
|
def step_check_readonly_true(context: Any) -> None:
|
|
context.guard_result = context.guard_func({"read_only": True})
|
|
|
|
|
|
@when("I check a read_only=False operation")
|
|
def step_check_readonly_false(context: Any) -> None:
|
|
context.guard_result = context.guard_func({"read_only": False})
|
|
|
|
|
|
@then("the retry guard should indicate read-only")
|
|
def step_retry_guard_true(context: Any) -> None:
|
|
assert context.guard_result is True
|
|
|
|
|
|
@then("the retry guard should indicate not read-only")
|
|
def step_retry_guard_false(context: Any) -> None:
|
|
assert context.guard_result is False
|
|
|
|
|
|
@when('I check an operation with plan_phase "{phase}"')
|
|
def step_check_plan_phase(context: Any, phase: str) -> None:
|
|
context.guard_result = context.guard_func({"plan_phase": phase})
|
|
|
|
|
|
@when("I check an operation with empty plan_phase")
|
|
def step_check_empty_plan_phase(context: Any) -> None:
|
|
context.guard_result = context.guard_func({"plan_phase": ""})
|
|
|
|
|
|
@when("I check an operation with non-string plan_phase {value}")
|
|
def step_check_non_string_plan_phase(context: Any, value: str) -> None:
|
|
# Convert the string "True" to Python bool True
|
|
if value == "True":
|
|
py_val: Any = True
|
|
elif value == "1":
|
|
py_val = 1
|
|
else:
|
|
py_val = value
|
|
context.guard_result = context.guard_func({"plan_phase": py_val})
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Wait strategy building
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@when('I build a wait strategy with strategy "{strategy}" and jitter {jitter_flag}')
|
|
def step_build_wait_strategy(context: Any, strategy: str, jitter_flag: str) -> None:
|
|
from tenacity import wait_exponential, wait_exponential_jitter
|
|
|
|
jitter = jitter_flag == "True"
|
|
context.built_wait = _build_wait_strategy(strategy, 1.0, 60.0, jitter)
|
|
context.wait_exp_cls = wait_exponential
|
|
context.wait_jitter_cls = wait_exponential_jitter
|
|
|
|
|
|
@then("the wait strategy should be an exponential jitter strategy")
|
|
def step_check_exp_jitter_strategy(context: Any) -> None:
|
|
assert isinstance(context.built_wait, context.wait_jitter_cls)
|
|
|
|
|
|
@then("the wait strategy should be an exponential strategy")
|
|
def step_check_exp_strategy(context: Any) -> None:
|
|
assert isinstance(context.built_wait, context.wait_exp_cls)
|
|
|
|
|
|
@when(
|
|
'I build a wait strategy with strategy "{strategy}" base_delay {base:g} and jitter {jitter_flag}'
|
|
)
|
|
def step_build_wait_strategy_with_base(
|
|
context: Any, strategy: str, base: float, jitter_flag: str
|
|
) -> None:
|
|
from tenacity import wait_exponential
|
|
|
|
jitter = jitter_flag == "True"
|
|
context.built_wait = _build_wait_strategy(strategy, base, 60.0, jitter)
|
|
context.wait_exp_cls = wait_exponential
|
|
|
|
|
|
@then("the effective base delay should be at least {minimum:g}")
|
|
def step_check_min_base_delay(context: Any, minimum: float) -> None:
|
|
# Verify the strategy was built with the floor applied by
|
|
# computing the actual wait time for the first retry attempt.
|
|
from unittest.mock import MagicMock
|
|
|
|
assert isinstance(context.built_wait, context.wait_exp_cls)
|
|
mock_state = MagicMock()
|
|
mock_state.attempt_number = 1
|
|
wait_seconds = context.built_wait(retry_state=mock_state)
|
|
assert wait_seconds >= minimum, (
|
|
f"Wait time {wait_seconds}s is below minimum {minimum}s"
|
|
)
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# _is_async_callable checks
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@when("I check _is_async_callable with a partial-wrapped async function")
|
|
def step_check_is_async_partial(context: Any) -> None:
|
|
from functools import partial
|
|
|
|
from cleveragents.core.retry_patterns import _is_async_callable
|
|
|
|
async def sample_async(x: int) -> int:
|
|
return x
|
|
|
|
wrapped = partial(sample_async, 42)
|
|
context.is_async_result = _is_async_callable(wrapped)
|
|
|
|
|
|
@then("_is_async_callable should return True")
|
|
def step_check_is_async_true(context: Any) -> None:
|
|
assert context.is_async_result is True
|
|
|
|
|
|
@when("I check _is_async_callable with an async callable object")
|
|
def step_check_is_async_callable_obj(context: Any) -> None:
|
|
from cleveragents.core.retry_patterns import _is_async_callable
|
|
|
|
class AsyncCallable:
|
|
async def __call__(self) -> str:
|
|
return "ok"
|
|
|
|
context.is_async_result = _is_async_callable(AsyncCallable())
|
|
|
|
|
|
@when("I check _is_async_callable with a regular sync function")
|
|
def step_check_is_async_sync_func(context: Any) -> None:
|
|
from cleveragents.core.retry_patterns import _is_async_callable
|
|
|
|
def sync_func() -> str:
|
|
return "sync"
|
|
|
|
context.is_async_result = _is_async_callable(sync_func)
|
|
|
|
|
|
@then("_is_async_callable should return False")
|
|
def step_check_is_async_false(context: Any) -> None:
|
|
assert context.is_async_result is False
|
|
|
|
|
|
@when("I check _is_async_callable with a non-callable object")
|
|
def step_check_is_async_non_callable(context: Any) -> None:
|
|
from cleveragents.core.retry_patterns import _is_async_callable
|
|
|
|
context.is_async_result = _is_async_callable("not_callable")
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Backoff strategy floors
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@when("I build a linear wait strategy with base_delay {base:g}")
|
|
def step_build_linear_wait(context: Any, base: float) -> None:
|
|
context.linear_wait = _build_wait_strategy("linear", base, 60.0, False)
|
|
|
|
|
|
@then("the linear wait value should be at least {minimum:g}")
|
|
def step_check_linear_wait_floor(context: Any, minimum: float) -> None:
|
|
from unittest.mock import MagicMock
|
|
|
|
mock_state = MagicMock()
|
|
mock_state.attempt_number = 1
|
|
wait_val = context.linear_wait(retry_state=mock_state)
|
|
assert wait_val >= minimum, f"Linear wait {wait_val}s below minimum {minimum}s"
|
|
|
|
|
|
@when("I build a fixed wait strategy with base_delay {base:g}")
|
|
def step_build_fixed_wait(context: Any, base: float) -> None:
|
|
context.fixed_wait = _build_wait_strategy("fixed", base, 60.0, False)
|
|
|
|
|
|
@then("the fixed wait value should be at least {minimum:g}")
|
|
def step_check_fixed_wait_floor(context: Any, minimum: float) -> None:
|
|
from unittest.mock import MagicMock
|
|
|
|
mock_state = MagicMock()
|
|
mock_state.attempt_number = 1
|
|
wait_val = context.fixed_wait(retry_state=mock_state)
|
|
assert wait_val >= minimum, f"Fixed wait {wait_val}s below minimum {minimum}s"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Jitter and none strategy branches
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@when("I build a jitter wait strategy with base_delay {base:g}")
|
|
def step_build_jitter_wait(context: Any, base: float) -> None:
|
|
context.jitter_wait = _build_wait_strategy("jitter", base, 30.0, False)
|
|
|
|
|
|
@then("the jitter wait strategy should not be None")
|
|
def step_check_jitter_not_none(context: Any) -> None:
|
|
assert context.jitter_wait is not None
|
|
|
|
|
|
@when("I build a none wait strategy")
|
|
def step_build_none_wait(context: Any) -> None:
|
|
context.none_wait = _build_wait_strategy("none", 1.0, 60.0, False)
|
|
|
|
|
|
@then("the none wait strategy should produce zero-second wait")
|
|
def step_check_none_wait_zero(context: Any) -> None:
|
|
from unittest.mock import MagicMock
|
|
|
|
mock_state = MagicMock()
|
|
mock_state.attempt_number = 1
|
|
wait_val = context.none_wait(retry_state=mock_state)
|
|
assert wait_val == 0.0
|
|
|
|
|
|
@then("the jitter wait value should be at least {minimum:g}")
|
|
def step_check_jitter_wait_floor(context: Any, minimum: float) -> None:
|
|
from unittest.mock import MagicMock
|
|
|
|
mock_state = MagicMock()
|
|
mock_state.attempt_number = 1
|
|
wait_val = context.jitter_wait(retry_state=mock_state)
|
|
assert wait_val >= minimum, f"Jitter wait {wait_val}s is below minimum {minimum}s"
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Error sanitization
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@when("I sanitize an error message containing a database URL with credentials")
|
|
def step_sanitize_db_url(context: Any) -> None:
|
|
from cleveragents.core.retry_patterns import _sanitize_error_message
|
|
|
|
error = RuntimeError("Connection failed: postgres://admin:s3cret@db.host/mydb")
|
|
context.sanitized = _sanitize_error_message(error)
|
|
|
|
|
|
@then("the sanitized message should not contain the password")
|
|
def step_check_no_password(context: Any) -> None:
|
|
assert "s3cret" not in context.sanitized
|
|
assert "***@" in context.sanitized
|
|
|
|
|
|
@when("I sanitize an error message containing an API key")
|
|
def step_sanitize_api_key(context: Any) -> None:
|
|
from cleveragents.core.retry_patterns import _sanitize_error_message
|
|
|
|
error = RuntimeError("Auth failed: api_key=sk-12345abcdef token=xyz789")
|
|
context.sanitized = _sanitize_error_message(error)
|
|
|
|
|
|
@then("the sanitized message should redact the key value")
|
|
def step_check_key_redacted(context: Any) -> None:
|
|
assert "sk-12345abcdef" not in context.sanitized
|
|
assert "xyz789" not in context.sanitized
|
|
assert "api_key=***" in context.sanitized
|
|
|
|
|
|
@when("I sanitize an error message containing an Authorization header")
|
|
def step_sanitize_auth_header(context: Any) -> None:
|
|
from cleveragents.core.retry_patterns import _sanitize_error_message
|
|
|
|
error = RuntimeError("Request failed: Authorization: Bearer sk-live-abc123xyz")
|
|
context.sanitized = _sanitize_error_message(error)
|
|
|
|
|
|
@then("the sanitized message should not contain the bearer token")
|
|
def step_check_no_bearer_token(context: Any) -> None:
|
|
assert "sk-live-abc123xyz" not in context.sanitized
|
|
assert "Authorization: ***" in context.sanitized
|
|
|
|
|
|
@when("I sanitize an error message containing a private_key assignment")
|
|
def step_sanitize_private_key(context: Any) -> None:
|
|
from cleveragents.core.retry_patterns import _sanitize_error_message
|
|
|
|
error = RuntimeError("Config error: private_key=AAAA-BBBB-CCCC-DDDD")
|
|
context.sanitized = _sanitize_error_message(error)
|
|
|
|
|
|
@then("the sanitized message should redact the private_key value")
|
|
def step_check_private_key_redacted(context: Any) -> None:
|
|
assert "AAAA-BBBB-CCCC-DDDD" not in context.sanitized
|
|
assert "private_key=***" in context.sanitized
|
|
|
|
|
|
@when("I sanitize an error message containing a connection_string")
|
|
def step_sanitize_connection_string(context: Any) -> None:
|
|
from cleveragents.core.retry_patterns import _sanitize_error_message
|
|
|
|
error = RuntimeError("DB error: connection_string=Server=db;Password=s3cret")
|
|
context.sanitized = _sanitize_error_message(error)
|
|
|
|
|
|
@then("the sanitized message should redact the connection_string value")
|
|
def step_check_connection_string_redacted(context: Any) -> None:
|
|
assert "Server=db" not in context.sanitized
|
|
assert "connection_string=***" in context.sanitized
|
|
|
|
|
|
@when("I sanitize an error message containing an access_key")
|
|
def step_sanitize_access_key(context: Any) -> None:
|
|
from cleveragents.core.retry_patterns import _sanitize_error_message
|
|
|
|
error = RuntimeError("AWS error: access_key=AKIAIOSFODNN7EXAMPLE")
|
|
context.sanitized = _sanitize_error_message(error)
|
|
|
|
|
|
@then("the sanitized message should redact the access_key value")
|
|
def step_check_access_key_redacted(context: Any) -> None:
|
|
assert "AKIAIOSFODNN7EXAMPLE" not in context.sanitized
|
|
assert "access_key=***" in context.sanitized
|
|
|
|
|
|
@when("I sanitize an error message longer than 200 characters")
|
|
def step_sanitize_long_message(context: Any) -> None:
|
|
from cleveragents.core.retry_patterns import _sanitize_error_message
|
|
|
|
long_msg = "A" * 300
|
|
error = RuntimeError(long_msg)
|
|
context.sanitized = _sanitize_error_message(error)
|
|
|
|
|
|
@then("the sanitized message should be truncated with ellipsis")
|
|
def step_check_truncated_with_ellipsis(context: Any) -> None:
|
|
assert context.sanitized.endswith("...")
|
|
|
|
|
|
@then("the sanitized message length should not exceed {max_len:d}")
|
|
def step_check_sanitized_length(context: Any, max_len: int) -> None:
|
|
assert len(context.sanitized) <= max_len
|
|
|
|
|
|
@when("I sanitize a None error")
|
|
def step_sanitize_none_error(context: Any) -> None:
|
|
from cleveragents.core.retry_patterns import _sanitize_error_message
|
|
|
|
context.sanitized = _sanitize_error_message(None)
|
|
|
|
|
|
@then("the sanitized result should be None")
|
|
def step_check_sanitized_none(context: Any) -> None:
|
|
assert context.sanitized is None
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Decorator nesting behavior
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@when("I call a decorated function that nests another decorated call")
|
|
def step_call_decorated_nested(context: Any) -> None:
|
|
from cleveragents.core.retry_patterns import retry_service_operation
|
|
|
|
inner_call_count = {"n": 0}
|
|
|
|
@retry_service_operation(
|
|
service_name="session_service",
|
|
operation_name="inner_decorated",
|
|
max_attempts=3,
|
|
base_delay=0.01,
|
|
max_delay=0.02,
|
|
total_timeout=10.0,
|
|
)
|
|
def inner_decorated() -> str:
|
|
inner_call_count["n"] += 1
|
|
raise RuntimeError("inner decorated failure")
|
|
|
|
@retry_service_operation(
|
|
service_name="plan_service",
|
|
operation_name="outer_decorated",
|
|
max_attempts=2,
|
|
base_delay=0.01,
|
|
max_delay=0.02,
|
|
total_timeout=10.0,
|
|
)
|
|
def outer_decorated() -> str:
|
|
with contextlib.suppress(RuntimeError):
|
|
inner_decorated()
|
|
return "outer_ok"
|
|
|
|
context.nested_decorated_result = outer_decorated()
|
|
context.inner_decorated_call_count = inner_call_count["n"]
|
|
|
|
|
|
@then("the inner decorated call should execute without retries")
|
|
def step_check_inner_decorated_no_retries(context: Any) -> None:
|
|
assert context.nested_decorated_result == "outer_ok"
|
|
# The outer decorator catches the inner exception and returns "outer_ok",
|
|
# so the outer always succeeds on the first attempt. Inner is therefore
|
|
# called exactly once (no retries due to the nesting guard).
|
|
assert context.inner_decorated_call_count == 1
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# Decorator total timeout parameter
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
@when("I create a retry_service_operation decorator with total_timeout {timeout:g}")
|
|
def step_create_decorator_with_timeout(context: Any, timeout: float) -> None:
|
|
from cleveragents.core.retry_patterns import retry_service_operation
|
|
|
|
context.timeout_decorator = retry_service_operation(
|
|
service_name="plan_service",
|
|
operation_name="timeout_test",
|
|
max_attempts=100,
|
|
total_timeout=timeout,
|
|
)
|
|
|
|
|
|
@then("the decorator should accept the total_timeout parameter")
|
|
def step_check_timeout_decorator(context: Any) -> None:
|
|
assert callable(context.timeout_decorator)
|