From de09a6685202156ca68f0cc64bd85162ac66a3d7 Mon Sep 17 00:00:00 2001 From: Abd Alrahaman Date: Tue, 13 Oct 2026 14:04:31 +0300 Subject: [PATCH] fix(utils): cap safe_repr output to avoid event-loop stalls on pathological __repr__ (closes #6649) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LoggingIntegration serialises frame locals via `safe_repr`, which was an unbounded `repr(value)`. FastAPI >= 0.137 introduced `_IncludedRouter` locals whose repr walks the whole nested router graph and materialises hundreds of MiB per frame. A plain `logger.error(..., exc_info=exc)` from an exception handler in an app with ~800 routes measurably stalls the event loop for ~0.5s; in the reporter's production app (~thousands of routes, 6 levels of `include_router()`) it stalls for ~15-20s per error, long enough to trip gunicorn's WORKER_TIMEOUT -> SIGABRT. The reporter traced the cost to: frame routing.py:handle self <_IncludedRouter> ~117 MiB ~0.96s frame routing.py:_handle_selected self <_IncludedRouter> ~117 MiB ~0.95s ... repeated for every level of the nested include_router() tree ... The downstream serializer *already* truncates the resulting string to `max_value_length` (default 1024 chars) via `strip_string`, so these giant reprs are dropped on the next line anyway — but only after the CPU and memory have been spent building them. A previous attempt (#7176) reimplemented repr for built-in types using `dataclasses` reflection; it was closed as "too disruptive for a minor release". This change takes the opposite, minimal approach: - sentry_sdk/utils.py * Add `SAFE_REPR_MAX_CHARS = 100_000` as a hard upper bound on materialised repr size. Comfortably above the default `max_value_length` (1024) so normal values are untouched, but orders of magnitude below the pathological ~117 MiB case. * `safe_repr` grows an optional `max_chars` parameter defaulting to `SAFE_REPR_MAX_CHARS`. When the repr exceeds the cap, we take the prefix and append `...` so the serializer's downstream `strip_string` sees the same shape as before (a string it can safely truncate further). * `max_chars=None` preserves the exact historical unbounded behaviour for callers that want to opt out. * Existing `` behaviour for `__repr__` that raises is preserved verbatim. - tests/utils/test_general.py * `test_safe_repr_truncates_pathological_repr` — the direct regression for #6649 using a mock object whose `__repr__` returns 10 MiB. * `test_safe_repr_small_repr_unchanged` — proves normal values go through byte-for-byte unchanged (int, str, list). * `test_safe_repr_opt_in_unbounded` — proves `max_chars=None` is a real escape hatch. * `test_safe_repr_broken_still_graceful` — proves a raising `__repr__` still returns the historical sentinel. Note on scope: this does NOT change the downstream serializer, the LoggingIntegration patching, or any public API. Callers passing a `custom_repr` are unaffected because the custom path returns first in `_safe_repr_wrapper` without going through `safe_repr`. Credit to @Firezi for the extremely detailed root-cause walk on the issue — the per-frame cost measurements and the FastAPI 0.136.3 vs 0.137.2 bisect made this fix straightforward. --- sentry_sdk/utils.py | 40 ++++++++++++++++++++++++++-- tests/utils/test_general.py | 53 +++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index 4586ac5d5e..0d8c209239 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -553,11 +553,47 @@ def safe_str(value: "Any") -> str: return safe_repr(value) -def safe_repr(value: "Any") -> str: +#: Hard upper bound on the number of characters we will materialize from a +#: single object's repr before truncating. This exists so that an object whose +#: ``__repr__`` happens to walk a large graph (eg. a FastAPI ``_IncludedRouter`` +#: traversing nested routers, or any domain object with an expensive repr) +#: cannot pin an event loop for seconds while we build a string we will +#: immediately truncate anyway (issue #6649). +#: +#: The downstream serializer still truncates to ``max_value_length`` (default +#: 1024 chars), so values larger than that bound are already discarded; the +#: issue is just the CPU + memory cost of building them. Keeping this cap +#: comfortably above the default ``max_value_length`` means normal values are +#: unaffected, and only the pathological "repr materialises hundreds of MiB" +#: case is cut short. +SAFE_REPR_MAX_CHARS = 100_000 + + +def safe_repr(value: "Any", max_chars: "Optional[int]" = SAFE_REPR_MAX_CHARS) -> str: + """Return ``repr(value)``, with belt-and-braces guards so a hostile + ``__repr__`` cannot: + + 1. Raise through us (returns ``""`` instead). + 2. Burn seconds / gigabytes materialising a huge string that the serializer + is about to truncate anyway. If ``max_chars`` is not ``None`` and the + repr grows past it, we stop pulling characters as soon as the cap is + exceeded and append a truncation marker. Passing ``max_chars=None`` + preserves the original unbounded behaviour. + + See :data:`SAFE_REPR_MAX_CHARS` for the rationale. + """ + if max_chars is None: + try: + return repr(value) + except Exception: + return "" try: - return repr(value) + full = repr(value) except Exception: return "" + if len(full) <= max_chars: + return full + return full[:max_chars] + "..." def filename_for_module( diff --git a/tests/utils/test_general.py b/tests/utils/test_general.py index ed0df8b3b5..8bcf2a1c2f 100644 --- a/tests/utils/test_general.py +++ b/tests/utils/test_general.py @@ -610,3 +610,56 @@ def model_dump(self): # Test with dict containing class result = _normalize_data({"schema": TestClass, "count": 5}) assert result == {"schema": "", "count": 5} + + +def test_safe_repr_truncates_pathological_repr(): + """Regression for #6649 — safe_repr must not spend seconds / megabytes + materialising a huge repr. A hostile __repr__ returning 10 MiB of + characters must be bounded by SAFE_REPR_MAX_CHARS. + """ + from sentry_sdk.utils import SAFE_REPR_MAX_CHARS + + class _Pathological: + """Mimics FastAPI _IncludedRouter: a repr that materialises a huge + string (~10 MiB). The real-world value was ~117 MiB per router per + frame, with 8 nested frames per request — ~15-20s event-loop stall. + """ + + def __repr__(self): + return "x" * (10 * 1024 * 1024) # 10 MiB + + result = safe_repr(_Pathological()) + assert len(result) <= SAFE_REPR_MAX_CHARS + len("...") + assert result.endswith("...") + + +def test_safe_repr_small_repr_unchanged(): + """Values whose repr fits comfortably within the cap must be returned + exactly as before (no truncation marker, no behaviour change).""" + assert safe_repr(123) == "123" + assert safe_repr("hello") == "'hello'" + assert safe_repr([1, 2, 3]) == "[1, 2, 3]" + + +def test_safe_repr_opt_in_unbounded(): + """Callers that explicitly want the historical unbounded behaviour + (eg. tests, or callers that supply their own downstream cap) can pass + max_chars=None to disable the guard.""" + from sentry_sdk.utils import SAFE_REPR_MAX_CHARS + + big = "y" * (SAFE_REPR_MAX_CHARS + 1000) + result = safe_repr(big, max_chars=None) + # repr adds surrounding quotes, so len is >= original length + assert len(result) >= len(big) + assert "" not in result + + +def test_safe_repr_broken_still_graceful(): + """A __repr__ that raises must still return the historical sentinel + rather than propagating.""" + + class _Broken: + def __repr__(self): + raise RuntimeError("boom") + + assert safe_repr(_Broken()) == ""