diff --git a/src/youtube_extension/backend/config/logging_config.py b/src/youtube_extension/backend/config/logging_config.py index 6a033e419..959f31bc6 100644 --- a/src/youtube_extension/backend/config/logging_config.py +++ b/src/youtube_extension/backend/config/logging_config.py @@ -145,13 +145,28 @@ def _describe_exception(exc: BaseException) -> str: Used only by the serialization fallback, which exists precisely because a hostile ``__str__`` can raise. The exception caught there may *be* one raised from a call site's own ``__str__``, so interpolating ``exc`` can - raise a second time — inside the handler for the first. The type name is a - plain attribute lookup and is always safe. + raise a second time — inside the handler for the first. + + ``type(exc).__name__`` is the natural retreat from that, and is *not* + sufficient on its own: ``__name__`` on a class is looked up on its + **metaclass**, so a metaclass defining ``__name__`` as a raising property + defeats it too. That second raise would leave this function entirely, + past the ``_JSON_UNSERIALIZABLE_RECORD`` tier, and cost the record (#1576). + + ``object.__getattribute__(type(exc), "__name__")`` does not help — it still + routes through the metaclass descriptor. Fetching the descriptor from + ``type.__dict__`` and binding it directly is what bypasses an override, + and it returns the ordinary name for ordinary classes. """ try: return f"{type(exc).__name__}: {exc}" + except Exception: # noqa: BLE001 - fall through to a narrower attempt + pass + try: + name: str = type.__dict__["__name__"].__get__(type(exc)) + return name except Exception: # noqa: BLE001 - the fallback must not need a fallback - return type(exc).__name__ + return "UnrenderableException" class StructuredFormatter(logging.Formatter): diff --git a/tests/unit/test_logging_config_crlf.py b/tests/unit/test_logging_config_crlf.py index 12f7611bb..bd180a2b7 100644 --- a/tests/unit/test_logging_config_crlf.py +++ b/tests/unit/test_logging_config_crlf.py @@ -522,6 +522,61 @@ def test_describe_exception_survives_an_exception_that_cannot_be_stringified(): assert _describe_exception(_NastyError()) == "_NastyError" +# --------------------------------------------------------------------------- +# #1576: the retreat from an unstringifiable exception is itself defeatable. +# +# `_NastyError` above has an ordinary metaclass, so it only exercises one of +# the two ways naming an exception can raise. `__name__` on a class resolves +# through its *metaclass*, so a metaclass can make the `except` branch raise +# too — outside any guard, so it escapes even the constant-record tier. +# +# The tests below fail on 5473bcc. +# --------------------------------------------------------------------------- + + +class _HostileMeta(type): + """Makes `type(exc).__name__` raise — the usual retreat from a bad `__str__`.""" + + @property + def __name__(cls): # noqa: ANN204 - the override is the point + raise RuntimeError("__name__ exploded") + + +class _NamelessError(Exception, metaclass=_HostileMeta): + """Neither `str(exc)` nor `type(exc).__name__` can be rendered.""" + + def __str__(self) -> str: + raise RuntimeError("str() exploded too") + + +class _RaisesNameless: + def __str__(self) -> str: + raise _NamelessError() + + +def test_describe_exception_survives_a_hostile_metaclass(): + # `object.__getattribute__(type(exc), "__name__")` also raises here; only + # binding the descriptor from `type.__dict__` bypasses the override. + assert _describe_exception(_NamelessError()) == "_NamelessError" + + +def test_hostile_metaclass_exception_does_not_cost_the_record(): + records = _emit_three("json-hostile-metaclass", _RaisesNameless()) + + assert len(records) == 3 + poisoned = records[1] + assert poisoned["level"] == "INFO" + assert poisoned["message"] == "poisoned" + assert "correlation_id" not in poisoned + assert poisoned["serialization_error"] == "_NamelessError" + + +def test_describe_exception_is_unchanged_for_ordinary_exceptions(): + # The added tier must not alter the normal path, which the pre-existing + # fallback tests assert exact strings against. + assert _describe_exception(ValueError("msg")) == "ValueError: msg" + + @pytest.fixture def _restore_root_logging(): """`setup_logging` calls dictConfig, which mutates global logging state."""