fix(auxiliary_client): dedup resolve_provider_client fall-through warnings
The two fall-through branches in resolve_provider_client (unknown provider, unhandled auth_type) logged at WARNING on every retry of a misconfigured provider, spamming logs during retry loops. Demote both to logger.debug with per-process dedup: the first occurrence still surfaces (a provider-name typo or PROVIDER_REGISTRY/auth_type-drift bug is worth seeing once), while identical repeats are suppressed for the process lifetime. Salvaged from #56283 (extracting only the stated auxiliary_client fix; the original PR also bundled ~2800 lines of unrelated changes across 10 other files, which are dropped).
This commit is contained in:
parent
fb7a38ad21
commit
c0d3ceb17e
2 changed files with 78 additions and 3 deletions
|
|
@ -110,6 +110,19 @@ from utils import base_url_host_matches, base_url_hostname, env_float, model_for
|
|||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
# ── resolve_provider_client fall-through dedup ───────────────────────────
|
||||
# Both fall-through warning sites in resolve_provider_client (the "unknown
|
||||
# provider" and "unhandled auth_type" branches) fire on every retry of a
|
||||
# misconfigured provider, spamming the logs. Demote them to logger.debug with
|
||||
# per-process dedup: the FIRST occurrence still surfaces (it carries real
|
||||
# diagnostic value — a provider-name typo or PROVIDER_REGISTRY/auth_type
|
||||
# drift), and identical repeats are suppressed for the lifetime of the
|
||||
# process. Two independent sets keep each branch linear and let tests clear
|
||||
# them independently.
|
||||
_LOGGED_UNKNOWN_PROVIDER_KEYS: set = set()
|
||||
_LOGGED_UNHANDLED_AUTHTYPE_KEYS: set = set()
|
||||
|
||||
|
||||
def _openai_http_client_kwargs(
|
||||
base_url: Optional[str],
|
||||
*,
|
||||
|
|
@ -4336,7 +4349,11 @@ def resolve_provider_client(
|
|||
|
||||
pconfig = PROVIDER_REGISTRY.get(provider)
|
||||
if pconfig is None:
|
||||
logger.warning("resolve_provider_client: unknown provider %r", provider)
|
||||
# Demoted from logger.warning to debug; dedup keyed by provider name
|
||||
# so the first occurrence surfaces but repeated retries stay silent.
|
||||
if provider not in _LOGGED_UNKNOWN_PROVIDER_KEYS:
|
||||
_LOGGED_UNKNOWN_PROVIDER_KEYS.add(provider)
|
||||
logger.debug("resolve_provider_client: unknown provider %r", provider)
|
||||
return None, None
|
||||
|
||||
if pconfig.auth_type == "api_key":
|
||||
|
|
@ -4528,8 +4545,14 @@ def resolve_provider_client(
|
|||
"directly supported, try 'auto'", provider)
|
||||
return None, None
|
||||
|
||||
logger.warning("resolve_provider_client: unhandled auth_type %s for %s",
|
||||
pconfig.auth_type, provider)
|
||||
# Demoted from logger.warning to debug; dedup keyed on (auth_type,
|
||||
# provider) so the first occurrence surfaces (real schema-drift bug) but
|
||||
# per-call retries stay silent.
|
||||
_auth_dedup_key = (pconfig.auth_type, provider)
|
||||
if _auth_dedup_key not in _LOGGED_UNHANDLED_AUTHTYPE_KEYS:
|
||||
_LOGGED_UNHANDLED_AUTHTYPE_KEYS.add(_auth_dedup_key)
|
||||
logger.debug("resolve_provider_client: unhandled auth_type %s for %s",
|
||||
pconfig.auth_type, provider)
|
||||
return None, None
|
||||
|
||||
|
||||
|
|
|
|||
52
tests/agent/test_auxiliary_client_resolve_dedup.py
Normal file
52
tests/agent/test_auxiliary_client_resolve_dedup.py
Normal file
|
|
@ -0,0 +1,52 @@
|
|||
"""Tests for resolve_provider_client fall-through log dedup (salvage #56283).
|
||||
|
||||
Both fall-through branches (unknown provider, unhandled auth_type) were demoted
|
||||
from ``logger.warning`` to ``logger.debug`` with per-process dedup: the first
|
||||
occurrence surfaces for diagnostics; identical repeats are suppressed for the
|
||||
lifetime of the process so a retry loop can't spam the logs.
|
||||
"""
|
||||
|
||||
import logging
|
||||
|
||||
import agent.auxiliary_client as ac
|
||||
from agent.auxiliary_client import resolve_provider_client
|
||||
|
||||
|
||||
class TestUnknownProviderDedup:
|
||||
def setup_method(self):
|
||||
ac._LOGGED_UNKNOWN_PROVIDER_KEYS.clear()
|
||||
|
||||
def test_unknown_provider_logs_debug_once_not_warning(self, caplog):
|
||||
with caplog.at_level(logging.DEBUG, logger="agent.auxiliary_client"):
|
||||
client, model = resolve_provider_client("no_such_provider_xyz", "")
|
||||
assert (client, model) == (None, None)
|
||||
recs = [
|
||||
r for r in caplog.records
|
||||
if "unknown provider" in r.getMessage()
|
||||
]
|
||||
# Exactly one record, and it is DEBUG (never WARNING).
|
||||
assert len(recs) == 1
|
||||
assert recs[0].levelno == logging.DEBUG
|
||||
assert not any(r.levelno >= logging.WARNING for r in recs)
|
||||
|
||||
def test_unknown_provider_repeat_is_suppressed(self, caplog):
|
||||
with caplog.at_level(logging.DEBUG, logger="agent.auxiliary_client"):
|
||||
resolve_provider_client("no_such_provider_xyz", "")
|
||||
resolve_provider_client("no_such_provider_xyz", "")
|
||||
resolve_provider_client("no_such_provider_xyz", "")
|
||||
recs = [
|
||||
r for r in caplog.records
|
||||
if "unknown provider" in r.getMessage()
|
||||
]
|
||||
# Three calls, one log line — dedup suppressed the repeats.
|
||||
assert len(recs) == 1
|
||||
|
||||
def test_distinct_unknown_providers_each_log_once(self, caplog):
|
||||
with caplog.at_level(logging.DEBUG, logger="agent.auxiliary_client"):
|
||||
resolve_provider_client("bogus_a", "")
|
||||
resolve_provider_client("bogus_b", "")
|
||||
recs = [
|
||||
r for r in caplog.records
|
||||
if "unknown provider" in r.getMessage()
|
||||
]
|
||||
assert len(recs) == 2
|
||||
Loading…
Add table
Add a link
Reference in a new issue