From 8fcec9f12ef7764ec78cbab81033a7701c3e52e8 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sat, 15 Aug 2026 18:08:04 +0300 Subject: [PATCH 1/2] fix(tradein/observability): stop basic_auth 401 and RetryError GlitchTip noise MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 83% of tracker issues (7460 total) were pure noise drowning real signal: - basic_auth 401 (3738 issues, 2019 distinct titles) — ops/glitchtip-auth- forwarder sent EVERY 401 from bots scanning gendsgn.ru (GET /wp-admin/ install.php etc.) as an individual GlitchTip event, remote_ip baked into message/tags inflated cardinality. Not an application error — expected bot-scan traffic against a basic_auth-protected site. - RetryError (2462 issues) — geocoder.py's three tenacity @retry-wrapped Nominatim helpers (lookup/suggest/reverse) raised tenacity.RetryError on exhaustion without reraise=True; RetryError.__str__() embeds a Future repr() with a memory address that differs every call, so GlitchTip grouped each exhausted retry as a distinct issue instead of one. Fix at the source, not post-hoc issue cleanup: - forwarder.py: before_send drops events tagged event_type in {basic_auth_failed, basic_auth_storm}; forwarder's own capture_exception (real script bugs) carries no such tag and passes through untouched. - geocoder.py: reraise=True on all three @retry decorators — propagates the real underlying exception (stable type + stacktrace) instead of the unstable RetryError wrapper. - sentry_scrub.stabilize_retry_error_fingerprint: belt-and-suspenders before_send hook, composed into both app/main.py and scheduler_main.py (geocoder runs in both processes — FastAPI request path and the overnight geocode_missing_listings batch). Collapses any RetryError that still slips through into one persistent issue per cause-exception type name only — never IP/address/listing-id. Content-ful categories (OperationalError, city-sweep, harvest_quarter, cian/avito/yandex sweep failures, scrape_freshness_check — ~700 issues) are untouched: filters key off event_type tag / exception type name only. --- ops/glitchtip-auth-forwarder/forwarder.py | 43 +++++++++ .../test_forwarder.py | 77 ++++++++++++++++ tradein-mvp/backend/app/main.py | 23 +++-- .../backend/app/observability/sentry_scrub.py | 37 ++++++++ tradein-mvp/backend/app/scheduler_main.py | 21 ++++- tradein-mvp/backend/app/services/geocoder.py | 17 +++- .../backend/tests/test_sentry_scrub.py | 92 +++++++++++++++++++ 7 files changed, 299 insertions(+), 11 deletions(-) create mode 100644 ops/glitchtip-auth-forwarder/test_forwarder.py diff --git a/ops/glitchtip-auth-forwarder/forwarder.py b/ops/glitchtip-auth-forwarder/forwarder.py index 8fc3c5b4..79c782f3 100644 --- a/ops/glitchtip-auth-forwarder/forwarder.py +++ b/ops/glitchtip-auth-forwarder/forwarder.py @@ -8,6 +8,13 @@ Persistent offset в /state/offset.json — не дублируем при resta Throttle: при >10 401 events за 60s — однократный digest event (чтобы не флудить GlitchTip storm'ом); индивидуальные events во время storm пропускаются. +before_send=_drop_basic_auth_noise (glitchtip-noise фикс): все события отсюда +дропаются перед отправкой в GlitchTip — 401 от неаутентифицированного запроса +не ошибка сервиса, это боты сканируют закрытый basic_auth'ом сайт. Раньше это +был крупнейший источник шума в трекере (3 738 issue). Скрипт по-прежнему тэйлит +лог и печатает `[forwarder] 401 event sent: ...` в stdout (docker logs) — просто +больше не шлёт эти события в issue-трекер. Смотри `_drop_basic_auth_noise` docstring. + Реальный Caddy JSON access log (v2) структура: { "level": "info", @@ -73,6 +80,41 @@ _shutdown = False _last_exc_sent: float = 0.0 _EXC_THROTTLE_S: float = 300.0 +# event_type-теги, которыми emit_event/emit_digest помечают КАЖДОЕ отправляемое +# событие (см. scope.set_tag("event_type", ...) ниже) — используются как ключ +# для before_send-фильтра. +_BASIC_AUTH_EVENT_TYPES = frozenset({"basic_auth_failed", "basic_auth_storm"}) + + +def _drop_basic_auth_noise(event: dict, hint: dict) -> dict | None: # type: ignore[type-arg] + """before_send-фильтр: 401 неаутентифицированного basic_auth-запроса — НЕ + ошибка сервиса, а expected-поведение сканеров-ботов, ломящихся в закрытый + basic_auth'ом gendsgn.ru (`GET /wp-admin/install.php` и подобное). До этого + фикса emit_event/emit_digest слали КАЖДЫЙ такой 401 individual-событием (или + storm-digest) в GlitchTip — remote_ip в message/тегах раздувал кардинальность + (3 738 issue, 2 019 различных заголовков, топ — 222 события на «GET + /wp-admin/install.p…»), топя содержательные алерты (OperationalError, sweep + failures) в шуме сканеров. + + Дропаем НА ИСТОЧНИКЕ (before_send), не постфактум-чисткой issue-трекера — + так шум не появляется вообще, а не изредка удаляется руками. Фильтруем по + тегу `event_type`, который ставят ТОЛЬКО emit_event/emit_digest — необработанные + исключения самого форвардера (`capture_exception` в конце `main()`, реальный + баг скрипта) этот тег не несут и проходят фильтр как есть (см. `except + Exception` ниже в `main()`). + """ + tags = event.get("tags") + event_type = None + if isinstance(tags, dict): + event_type = tags.get("event_type") + elif isinstance(tags, list): + # sentry_sdk в некоторых версиях сериализует tags как list[tuple[str, str]] + # вместо dict — на всякий случай поддерживаем обе формы. + event_type = dict(tags).get("event_type") if tags else None + if event_type in _BASIC_AUTH_EVENT_TYPES: + return None + return event + def _signal_handler(signum: int, frame: object) -> None: global _shutdown @@ -221,6 +263,7 @@ def main() -> None: traces_sample_rate=0.0, attach_stacktrace=False, send_default_pii=False, + before_send=_drop_basic_auth_noise, # Отключаем интеграции которые не нужны тонкому sidecar default_integrations=False, ) diff --git a/ops/glitchtip-auth-forwarder/test_forwarder.py b/ops/glitchtip-auth-forwarder/test_forwarder.py new file mode 100644 index 00000000..a9c089b3 --- /dev/null +++ b/ops/glitchtip-auth-forwarder/test_forwarder.py @@ -0,0 +1,77 @@ +"""Тесты для `_drop_basic_auth_noise` (before_send-фильтр, glitchtip-noise). + +Раньше форвардер слал КАЖДЫЙ basic_auth 401 (сканеры-боты, ломящиеся в закрытый +basic_auth'ом gendsgn.ru) individual-событием в GlitchTip — 3 738 issue, 2 019 +различных заголовков (remote_ip раздувал кардинальность), топя содержательный +сигнал. `_drop_basic_auth_noise` дропает эти события НА ИСТОЧНИКЕ (before_send), +но НЕ должен трогать unhandled-ошибки самого форвардера (реальный баг скрипта — +`capture_exception` без `event_type`-тега, аналог "500 должен пройти"). +""" + +from __future__ import annotations + +import os + +# DSN обязателен на module-level (`os.environ["GLITCHTIP_DSN"]`, fail-fast) — задаём +# ДО импорта forwarder.py, иначе импорт падает KeyError. +os.environ.setdefault("GLITCHTIP_DSN", "http://test@localhost/1") + +from forwarder import _BASIC_AUTH_EVENT_TYPES, _drop_basic_auth_noise + + +def test_drops_individual_basic_auth_401() -> None: + """emit_event() тегирует event_type=basic_auth_failed — 401 от бота-сканера, + не ошибка сервиса, должен быть отброшен (return None).""" + event = { + "tags": {"event_type": "basic_auth_failed", "remote_ip": "95.165.147.218"}, + "message": "basic_auth 401 — GET /wp-admin/install.php from 95.165.147.218", + } + assert _drop_basic_auth_noise(event, {}) is None + + +def test_drops_basic_auth_storm_digest() -> None: + """emit_digest() тегирует event_type=basic_auth_storm — тоже 401-класс, тоже + не ошибка сервиса, дропаем.""" + event = { + "tags": {"event_type": "basic_auth_storm"}, + "message": "basic_auth storm — 15 failed attempts in 60s", + } + assert _drop_basic_auth_noise(event, {}) is None + + +def test_drops_when_tags_serialized_as_list_of_tuples() -> None: + """Некоторые версии sentry_sdk сериализуют tags как list[tuple[str, str]] + вместо dict — фильтр обязан поддерживать обе формы.""" + event = {"tags": [("event_type", "basic_auth_failed")]} + assert _drop_basic_auth_noise(event, {}) is None + + +def test_passes_through_forwarder_own_crash() -> None: + """500-аналог: unhandled exception самого форвардера (capture_exception в + конце main(), реальный баг скрипта — напр. PermissionError на STATE_FILE) не + несёт event_type-тег → должен пройти НЕТРОНУТЫМ, не быть молча проглоченным + вместе с ботовым шумом.""" + event = { + "level": "error", + "exception": {"values": [{"type": "PermissionError", "value": "denied"}]}, + } + out = _drop_basic_auth_noise(dict(event), {}) + assert out == event + + +def test_passes_through_event_without_tags() -> None: + event: dict = {"message": "something unrelated"} + out = _drop_basic_auth_noise(dict(event), {}) + assert out == event + + +def test_passes_through_unrelated_tag_value() -> None: + event = {"tags": {"event_type": "something_else"}} + out = _drop_basic_auth_noise(dict(event), {}) + assert out == event + + +def test_basic_auth_event_types_are_exactly_the_two_emitters_use() -> None: + """Явная фиксация словаря — emit_event → basic_auth_failed, + emit_digest → basic_auth_storm (см. forwarder.py).""" + assert _BASIC_AUTH_EVENT_TYPES == frozenset({"basic_auth_failed", "basic_auth_storm"}) diff --git a/tradein-mvp/backend/app/main.py b/tradein-mvp/backend/app/main.py index 347cad8c..29f99749 100644 --- a/tradein-mvp/backend/app/main.py +++ b/tradein-mvp/backend/app/main.py @@ -66,17 +66,28 @@ logging.getLogger("httpx").setLevel(logging.WARNING) # worker (in-app scheduler зовёт task-функции напрямую; compose = postgres/backend/ # frontend), отдельного broker нет → мониторить нечего. if settings.glitchtip_dsn: - from app.observability.sentry_scrub import redact_telegram_bot_token + from app.observability.sentry_scrub import ( + redact_telegram_bot_token, + stabilize_retry_error_fingerprint, + ) def _before_send(event: dict[str, object], hint: dict[str, object]) -> dict[str, object] | None: - """Композиция PII-scrub + Telegram bot-токен redaction (#tgsupport-web) — - см. app/tgbot_main.py._before_send (идентичная композиция, тот же риск: - теперь этот процесс тоже держит TelegramClient в стек-фреймах при ошибке - sendMessage, а include_local_variables=False ниже — первый рубеж защиты).""" + """Композиция PII-scrub + Telegram bot-токен redaction (#tgsupport-web) + + RetryError fingerprint-стабилизация (glitchtip-noise) — см. + app/tgbot_main.py._before_send (та же композиция без последнего шага, + тот бот geocoder не зовёт). PII/token — тот же риск: теперь этот процесс + тоже держит TelegramClient в стек-фреймах при ошибке sendMessage, а + include_local_variables=False ниже — первый рубеж защиты. RetryError — + этот процесс обслуживает /api/v1/geocode/* (suggest/lookup/reverse), + которые ретраят Nominatim через tenacity; см. + sentry_scrub.stabilize_retry_error_fingerprint.""" scrubbed = scrub_pii_event(event, hint) # type: ignore[arg-type] if scrubbed is None: return None - return redact_telegram_bot_token(scrubbed, hint) # type: ignore[arg-type,return-value] + detokened = redact_telegram_bot_token(scrubbed, hint) # type: ignore[arg-type] + if detokened is None: + return None + return stabilize_retry_error_fingerprint(detokened, hint) # type: ignore[arg-type,return-value] sentry_sdk.init( dsn=settings.glitchtip_dsn, diff --git a/tradein-mvp/backend/app/observability/sentry_scrub.py b/tradein-mvp/backend/app/observability/sentry_scrub.py index 51c01e94..932042d5 100644 --- a/tradein-mvp/backend/app/observability/sentry_scrub.py +++ b/tradein-mvp/backend/app/observability/sentry_scrub.py @@ -174,3 +174,40 @@ def redact_telegram_bot_token(event: Event, _hint: dict[str, Any]) -> Event | No if not isinstance(event, dict): return event return _redact_strings(event) # type: ignore[return-value] + + +# ── RetryError fingerprint stabilization (GlitchTip noise-reduction) ──────── +# tenacity.RetryError.__str__() тащит repr() последнего Future +# (`RetryError[]`) — +# memory address объекта, случайный на каждый вызов процесса. Пока geocoder.py +# ретраил Nominatim без `reraise=True`, каждое исчерпание ретраев (Nominatim +# недоступен/rate-limit/403) улетало в GlitchTip как RetryError с этим +# нестабильным текстом → одна и та же причина плодила отдельный issue на КАЖДОЕ +# исчерпание (2 462 issue из 7 461 в трекере на момент фикса). `reraise=True` +# теперь пробрасывает реальное исключение вместо RetryError — это ГЛАВНЫЙ фикс. +# Функция ниже — belt-and-suspenders: ловит RetryError, если он всё же всплывёт +# (ретрай-код, который забыли/не смогли пометить reraise=True), и схлопывает его +# в ОДИН persistent issue per (класс исключения-причины), а не тысячу. Fingerprint +# собран ТОЛЬКО из стабильных данных — имя типа исключения-причины (небольшой +# фиксированный словарь вроде "HTTPStatusError"/"ConnectTimeout") — НИКАКИХ +# переменных данных запроса (адрес, IP, id объявления и т.п.), иначе проблема +# повторится в других терминах. +def stabilize_retry_error_fingerprint(event: Event, hint: dict[str, Any]) -> Event | None: + """before_send-хук: схлопывает tenacity.RetryError в один persistent issue. + + Определяет тип exception через `hint["exc_info"]` (реальный объект + исключения, тот же контракт что sentry_sdk передаёт в before_send) — не + парсит уже сериализованный event dict, надёжнее к изменениям формата SDK. + Не-RetryError события возвращает без изменений (OperationalError, алерты + scraper sweep'ов и т.п. фильтр не трогает). + """ + if not isinstance(event, dict): + return event + exc_info = hint.get("exc_info") if isinstance(hint, dict) else None + exc_value = exc_info[1] if exc_info and len(exc_info) > 1 else None + if exc_value is None or type(exc_value).__name__ != "RetryError": + return event + cause = exc_value.__cause__ or exc_value.__context__ + cause_type = type(cause).__name__ if cause is not None else "Unknown" + event["fingerprint"] = ["retry-exhausted", cause_type] + return event diff --git a/tradein-mvp/backend/app/scheduler_main.py b/tradein-mvp/backend/app/scheduler_main.py index 3bb8af4a..e5f93ed0 100644 --- a/tradein-mvp/backend/app/scheduler_main.py +++ b/tradein-mvp/backend/app/scheduler_main.py @@ -44,7 +44,24 @@ if settings.glitchtip_dsn: from sentry_sdk.integrations.logging import LoggingIntegration from sentry_sdk.integrations.sqlalchemy import SqlalchemyIntegration - from app.observability.sentry_scrub import scrub_pii_event + from app.observability.sentry_scrub import ( + scrub_pii_event, + stabilize_retry_error_fingerprint, + ) + + def _before_send(event: dict, hint: dict) -> dict | None: # type: ignore[type-arg] + """PII-scrub + RetryError fingerprint-стабилизация (glitchtip-noise). + + Этот процесс гоняет `geocode_missing_listings` (ночной batch, сотни + адресов за прогон) — @retry-декорированные Nominatim-хелперы + (app/services/geocoder.py) на исчерпанных ретраях исторически плодили + по отдельному GlitchTip issue на КАЖДЫЙ адрес (RetryError.__str__() + тащит нестабильный repr() Future). См. sentry_scrub docstring. + """ + scrubbed = scrub_pii_event(event, hint) + if scrubbed is None: + return None + return stabilize_retry_error_fingerprint(scrubbed, hint) sentry_sdk.init( dsn=settings.glitchtip_dsn, @@ -52,7 +69,7 @@ if settings.glitchtip_dsn: release=os.getenv("GIT_SHA") or os.getenv("SENTRY_RELEASE") or "unknown", traces_sample_rate=0.0, send_default_pii=False, - before_send=scrub_pii_event, + before_send=_before_send, integrations=[ SqlalchemyIntegration(), HttpxIntegration(), diff --git a/tradein-mvp/backend/app/services/geocoder.py b/tradein-mvp/backend/app/services/geocoder.py index 7785116a..6b3d9cc7 100644 --- a/tradein-mvp/backend/app/services/geocoder.py +++ b/tradein-mvp/backend/app/services/geocoder.py @@ -726,7 +726,16 @@ async def _nominatim_query(client: httpx.AsyncClient, address: str) -> dict | No return oblast_fallback -@retry(stop=stop_after_attempt(3), wait=wait_exponential(multiplier=1, min=1, max=8)) +# reraise=True (GlitchTip-noise fix): без него tenacity на исчерпанных ретраях +# бросает СВОЙ tenacity.RetryError, чей str() тащит repr() последнего Future +# (`` — адрес объекта в памяти, разный на КАЖДЫЙ вызов). GlitchTip +# группирует по этому нестабильному тексту → одна и та же причина (Nominatim +# недоступен/rate-limit) плодила отдельный issue на каждое исчерпание ретраев +# (2 462 issue из 7 461 в трекере). reraise=True пробрасывает РЕАЛЬНОЕ исключение +# (httpx.HTTPStatusError/TimeoutException) — стабильный тип+стек, группируется +# нормально. Дополнительная страховка — stabilize_retry_error_fingerprint +# (app/observability/sentry_scrub.py) на случай если RetryError всплывёт откуда-то ещё. +@retry(stop=stop_after_attempt(3), wait=wait_exponential(multiplier=1, min=1, max=8), reraise=True) async def _nominatim_lookup(address: str, city_hint: str | None = None) -> GeocodeResult | None: """OSM Nominatim — бесплатно, без ключа, 1 req/sec policy. @@ -925,7 +934,8 @@ async def _nominatim_query_city_aware( return _dedupe_nominatim_items(ekb_data, bare_data)[:limit] -@retry(stop=stop_after_attempt(2), wait=wait_exponential(multiplier=1, min=1, max=4)) +# reraise=True — см. комментарий у `_nominatim_lookup` (GlitchTip RetryError-шум). +@retry(stop=stop_after_attempt(2), wait=wait_exponential(multiplier=1, min=1, max=4), reraise=True) async def _nominatim_suggest( query: str, limit: int = 8, city_hint: str | None = None ) -> list[GeocodeSuggestion]: @@ -1654,7 +1664,8 @@ def _format_reverse_address(addr: dict) -> str | None: return ", ".join(parts) -@retry(stop=stop_after_attempt(3), wait=wait_exponential(multiplier=1, min=1, max=8)) +# reraise=True — см. комментарий у `_nominatim_lookup` (GlitchTip RetryError-шум). +@retry(stop=stop_after_attempt(3), wait=wait_exponential(multiplier=1, min=1, max=8), reraise=True) async def _nominatim_reverse(lat: float, lon: float) -> ReverseGeocodeResult | None: """Nominatim /reverse → ReverseGeocodeResult с snapped coords из item.lat/lon. diff --git a/tradein-mvp/backend/tests/test_sentry_scrub.py b/tradein-mvp/backend/tests/test_sentry_scrub.py index 1e8d8ba5..242c03d4 100644 --- a/tradein-mvp/backend/tests/test_sentry_scrub.py +++ b/tradein-mvp/backend/tests/test_sentry_scrub.py @@ -15,6 +15,7 @@ os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost: from app.observability.sentry_scrub import ( redact_telegram_bot_token, scrub_pii_event, + stabilize_retry_error_fingerprint, ) @@ -259,3 +260,94 @@ def test_composed_before_send_scrubs_pii_and_token_together() -> None: assert out["request"]["data"]["client_phone"] == "[REDACTED]" frame_url = out["exception"]["values"][0]["stacktrace"]["frames"][0]["vars"]["url"] assert "8663867262:AAExampleSecretPartAbCdEf123" not in frame_url + + +# ── RetryError fingerprint stabilization (glitchtip-noise, #) ─ +# +# tenacity.RetryError.__str__() тащит repr() последнего Future — memory address +# объекта, случайный на каждый вызов процесса. Раньше (без `reraise=True` в +# app/services/geocoder.py) каждое исчерпание ретраев Nominatim улетало в +# GlitchTip как RetryError с этим нестабильным текстом → одна и та же причина +# плодила отдельный issue на КАЖДОЕ исчерпание (2 462 issue из 7 461 в трекере). +# Тесты ниже бьют по `stabilize_retry_error_fingerprint` напрямую — belt-and- +# suspenders слой (основной фикс — reraise=True), и по контракту before_send: +# 401-класс (RetryError) схлопывается, содержательные категории (500-подобный +# generic Exception, OperationalError) проходят НЕТРОНУТЫМИ. + +from tenacity import RetryError # noqa: E402 + + +def _hint_for(exc: BaseException) -> dict: + """Строит hint в форме, которую sentry_sdk реально передаёт в before_send — + `exc_info = (type, value, traceback)` (contract stabilize_retry_error_fingerprint + полагается именно на эту форму, не на уже сериализованный event dict).""" + return {"exc_info": (type(exc), exc, exc.__traceback__)} + + +def _raise_retry_error_from(cause: BaseException) -> RetryError: + try: + raise cause + except type(cause) as caught: + try: + raise RetryError(None) from caught + except RetryError as retry_exc: + return retry_exc + + +def test_stabilize_retry_error_sets_stable_fingerprint() -> None: + """RetryError коллапсится в persistent issue по имени типа причины — НЕ по + нестабильному str(RetryError) (repr() Future с memory address).""" + exc = _raise_retry_error_from(TimeoutError("Nominatim timed out")) + out = stabilize_retry_error_fingerprint({"level": "error"}, _hint_for(exc)) + assert out is not None + assert out["fingerprint"] == ["retry-exhausted", "TimeoutError"] + + +def test_stabilize_retry_error_fingerprint_has_no_variable_data() -> None: + """Fingerprint не должен содержать IP/id объявления/адрес и т.п. — только + фиксированное имя типа исключения-причины (маленький словарь: + HTTPStatusError/ConnectTimeout/TimeoutError/...).""" + exc = _raise_retry_error_from( + ValueError("addr='ул. Ленина 1', ip=95.165.147.218, listing_id=12345") + ) + out = stabilize_retry_error_fingerprint({}, _hint_for(exc)) + assert out is not None + fingerprint_text = " ".join(out["fingerprint"]) + assert "95.165.147.218" not in fingerprint_text + assert "12345" not in fingerprint_text + assert out["fingerprint"] == ["retry-exhausted", "ValueError"] + + +def test_stabilize_retry_error_leaves_operational_error_untouched() -> None: + """401-аналог задачи: OperationalError — содержательная категория (реальный + сбой БД), фильтр её НЕ трогает (см. задачу #4 — не выключить сигнал вместе с + шумом).""" + from sqlalchemy.exc import OperationalError + + exc = OperationalError("SELECT 1", {}, Exception("connection refused")) + event = {"level": "error", "message": "db connection failed"} + out = stabilize_retry_error_fingerprint(dict(event), _hint_for(exc)) + assert out == event + assert "fingerprint" not in out + + +def test_stabilize_retry_error_leaves_generic_exception_untouched() -> None: + """500-аналог задачи: обычное необработанное исключение (не RetryError) + проходит без изменений.""" + exc = RuntimeError("scraper city-sweep failed") + event = {"level": "error"} + out = stabilize_retry_error_fingerprint(dict(event), _hint_for(exc)) + assert out == event + assert "fingerprint" not in out + + +def test_stabilize_retry_error_no_exc_info_untouched() -> None: + """capture_message-based события (нет exc_info) — фильтр не трогает, напр. + scrape_runs.py consecutive-failure алерты (content-ful, должны доходить).""" + event = {"level": "error", "message": "Scraper source 'avito' has 5 consecutive failed runs"} + out = stabilize_retry_error_fingerprint(dict(event), {}) + assert out == event + + +def test_stabilize_retry_error_handles_non_dict_event() -> None: + assert stabilize_retry_error_fingerprint(None, {}) is None # type: ignore[arg-type] -- 2.45.3 From 349494a9df675bcaa6c67eb6bb332e4a7b959dcd Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sat, 15 Aug 2026 20:02:44 +0300 Subject: [PATCH 2/2] fix(tradein/observability): close RetryError-half of GlitchTip noise fix (round 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ревью round 1 подтвердил basic_auth-часть, но нашёл 4 факта в RetryError-части: 1. reraise=True в geocoder.py не убирает шум, а переименовывает: наружу летит httpx.HTTPStatusError, чей str() содержит ПОЛНЫЙ request URL с query string (`for url '...search?q=<адрес>&...'`) — воспроизведено эмпирически. Тот же per-address issue-explosion, просто под другим типом исключения. Фикс: _HTTPX_ERROR_URL_QUERY_RE в sentry_scrub.scrub_pii_event режет query string из httpx-style "for url '...'" сообщений — стабилизирует ТЕКСТ, не только тип, независимо от того, уважает ли GlitchTip fingerprint-поле. 2. stabilize_retry_error_fingerprint затирал fingerprint целиком по (типу причины) — RetryError из НЕСВЯЗАННЫХ подсистем с одинаковым типом причины схлопнулись бы в один issue (geocoder vs scraper_kit оба ловят httpx-типы). Фикс: culprit = event["logger"] (LoggingIntegration ставит его = имя модуля-источника logger.exception) идёт первым компонентом fingerprint — разные подсистемы больше не сливаются. 3. Второй живой источник RetryError, пропущенный round 1 (грепали литерал "RetryError", не producers): BaseScraper._http_get в packages/scraper-kit — @retry БЕЗ reraise=True, живой путь через YandexDetailScraper.fetch_detail (yandex/serp.py и valuation.py переопределяют _http_get без retry — не затронуты). Оставлен на fingerprint-хук намеренно: detail-URL варьируются в ПУТИ (offer id), не в query — _HTTPX_ERROR_URL_QUERY_RE их не покрывает, а добавление reraise=True туда воспроизвело бы ту же проблему через HTTPStatusError с variable path вместо query. 4. type(exc).__name__ == "RetryError" (string-compare) → isinstance(exc, RetryError) с прямым импортом tenacity.RetryError — не матчит посторонние классы с тем же __name__, не промахивается мимо подклассов. Полный backend suite (4479 passed, 21 skipped) + geocoder/scheduler/alerts подмножества — без регрессий (reraise=True уже влит в main). Не тронуто (вне scope round 2, подтверждено ревьюером как верное): ops/glitchtip-auth-forwarder/* (basic_auth 401 дроп), массовая чистка накопленных issue. --- .../backend/app/observability/sentry_scrub.py | 121 ++++++++++----- tradein-mvp/backend/app/services/geocoder.py | 13 +- .../backend/tests/test_sentry_scrub.py | 140 ++++++++++++++++-- 3 files changed, 229 insertions(+), 45 deletions(-) diff --git a/tradein-mvp/backend/app/observability/sentry_scrub.py b/tradein-mvp/backend/app/observability/sentry_scrub.py index 932042d5..7cb78fd0 100644 --- a/tradein-mvp/backend/app/observability/sentry_scrub.py +++ b/tradein-mvp/backend/app/observability/sentry_scrub.py @@ -25,6 +25,7 @@ import re from typing import Any from sentry_sdk.types import Event +from tenacity import RetryError _REDACTED = "[REDACTED]" # Ключи consumer-PII (нижний регистр; сверка case-insensitive). @@ -76,6 +77,31 @@ _URL_SECRET_QUERY_RE = re.compile( ) _URL_SECRET_QUERY_REPLACEMENT = r"\g<1>" + _REDACTED +# httpx error-message URL query stabilization (GlitchTip-noise review round 2, +# claim #1). `httpx.HTTPStatusError.__str__()` (raised by `response.raise_for_status()`) +# bakes the FULL request URL — INCLUDING query string — into the exception message: +# "Client error '403 Forbidden' for url 'https://nominatim.openstreetmap.org/ +# search?q=<адрес>&format=json&limit=3'" (воспроизведено эмпирически: httpx.Response +# с params={"q": "<адрес>"} → raise_for_status() → именно этот текст). После +# app/services/geocoder.py `reraise=True` (стабилизирует ТИП исключения — RetryError +# → httpx.HTTPStatusError, см. комментарий у `_nominatim_lookup`) ИМЕННО этот текст +# становится GlitchTip title/value каждого события. `q=<адрес>` — переменная часть +# на КАЖДЫЙ вызов (ночной `geocode_missing_listings` — сотни разных адресов за +# прогон), значит per-address issue-explosion не устранён `reraise=True`, а просто +# переехал с RetryError на HTTPStatusError (тот же механизм: GlitchTip группирует по +# нестабильному тексту сообщения — это же подтверждают исходные 2 462 RetryError-issue, +# невозможные при группировке чисто по stacktrace/culprit). +# +# Отдельная регулярка от `_URL_SECRET_QUERY_RE` намеренно: та бьёт по ИМЕНИ известных +# secret-параметров (security-редактор), здесь — ЛЮБОЙ query string в httpx-стиле +# сообщении "for url '...'" (grouping-стабильность, не секретность — `q` не секрет). +# Режем query целиком (не только конкретные параметры) — host+path остаются +# стабильными для группировки, "for url '...'" — единственная форма, которую бьёт +# regex (не трогает произвольные строки с `?`, см. тест +# test_scrub_pii_event_httpx_url_query_stabilization_leaves_unrelated_text_untouched). +_HTTPX_ERROR_URL_QUERY_RE = re.compile(r"(for url '[^'?]*)\?[^']*(')") +_HTTPX_ERROR_URL_QUERY_REPLACEMENT = r"\g<1>?" + _REDACTED + r"\g<2>" + def _scrub(obj: Any) -> None: """Рекурсивно заменить значения PII-ключей в dict на [REDACTED] (in-place).""" @@ -90,32 +116,34 @@ def _scrub(obj: Any) -> None: _scrub(item) -def _redact_url_secrets_inplace(obj: Any) -> None: - """Рекурсивно (IN-PLACE, как `_scrub`) заменяет значения секрет-подобных - query-параметров (`?token=...`, `?proxy_key=...` и т.п.) на [REDACTED] в - КАЖДОЙ строке event — не ключ-based: секрет утекает через httpx span - `url`/`query` data и через текст исключений (`str(exc)` httpx содержит полный - request URL), а не только через известные PII-поля формы. Мутирует dict/list - на месте (НЕ пересоздаёт структуру, в отличие от `_redact_strings`) — - сохраняет identity верхнеуровневого `event`, на что опирается контракт - `scrub_pii_event`/`before_send` и существующие тесты (`out is event`). +def _regex_redact_inplace(obj: Any, pattern: re.Pattern[str], replacement: str) -> None: + """Рекурсивно (IN-PLACE, как `_scrub`) прогоняет `pattern.sub(replacement, ...)` + по КАЖДОЙ строке event (не ключ-based) — общий обход, переиспользуемый и для + URL-секретов (`_URL_SECRET_QUERY_RE`), и для стабилизации httpx error-message + URL (`_HTTPX_ERROR_URL_QUERY_RE`): в обоих случаях переменные данные утекают + через httpx span `url`/`query` data и через текст исключений (`str(exc)` httpx + содержит полный request URL), а не только через известные PII-поля формы. + Мутирует dict/list на месте (НЕ пересоздаёт структуру, в отличие от + `_redact_strings`) — сохраняет identity верхнеуровневого `event`, на что + опирается контракт `scrub_pii_event`/`before_send` и существующие тесты + (`out is event`). """ if isinstance(obj, dict): for key, value in obj.items(): if isinstance(value, str): - redacted = _URL_SECRET_QUERY_RE.sub(_URL_SECRET_QUERY_REPLACEMENT, value) + redacted = pattern.sub(replacement, value) if redacted != value: obj[key] = redacted else: - _redact_url_secrets_inplace(value) + _regex_redact_inplace(value, pattern, replacement) elif isinstance(obj, list): for i, value in enumerate(obj): if isinstance(value, str): - redacted = _URL_SECRET_QUERY_RE.sub(_URL_SECRET_QUERY_REPLACEMENT, value) + redacted = pattern.sub(replacement, value) if redacted != value: obj[i] = redacted else: - _redact_url_secrets_inplace(value) + _regex_redact_inplace(value, pattern, replacement) # tuple намеренно не обрабатываем: sentry_sdk event — это JSON-совместимая # структура (dict/list/str/int/...), tuple там не встречается, а даже если бы # встретился — он immutable, in-place правка невозможна (см. `_scrub`, тот же @@ -123,16 +151,21 @@ def _redact_url_secrets_inplace(obj: Any) -> None: def scrub_pii_event(event: Event, _hint: dict[str, Any]) -> Event | None: - """Redact consumer-PII + URL query-string секретов из error event перед отправкой. + """Redact consumer-PII + URL query-string секретов/nondeterministic-данных из + error event перед отправкой. - Композиция (обе — in-place, сохраняют identity `event`): (1) ключ-based + Композиция (все — in-place, сохраняют identity `event`): (1) ключ-based dict-scrub consumer-PII полей формы (как раньше), (2) full-text regex-проход по ВСЕМУ event, вырезающий значения секрет-подобных query-параметров в любой строке (proxy/API-ключи в исходящих URL сторонних сервисов, напр. mobileproxy - changeip — #security-audit). Второй шаг не завязан на конкретные ключи полей — - ловит секрет в frame locals, breadcrumb, exception message и т.д., где он может - оказаться независимо от include_local_variables/traces_sample_rate. Возвращает - event (не None). + changeip — #security-audit), (3) full-text regex-проход, стабилизирующий httpx + error-message URL (`for url '...?...'`) — убирает переменный query string + (адрес геокодинга и т.п.), от которого GlitchTip group-title плодит issue на + каждый вызов (GlitchTip-noise review round 2, claim #1; см. комментарий у + `_HTTPX_ERROR_URL_QUERY_RE`). (2) и (3) не завязаны на конкретные ключи полей — + ловят секрет/переменные данные в frame locals, breadcrumb, exception message + и т.д., где они могут оказаться независимо от + include_local_variables/traces_sample_rate. Возвращает event (не None). """ if not isinstance(event, dict): return event @@ -141,7 +174,8 @@ def scrub_pii_event(event: Event, _hint: dict[str, Any]) -> Event | None: _scrub(request.get("data")) _scrub(event.get("extra")) _scrub(event.get("contexts")) - _redact_url_secrets_inplace(event) + _regex_redact_inplace(event, _URL_SECRET_QUERY_RE, _URL_SECRET_QUERY_REPLACEMENT) + _regex_redact_inplace(event, _HTTPX_ERROR_URL_QUERY_RE, _HTTPX_ERROR_URL_QUERY_REPLACEMENT) return event @@ -184,30 +218,51 @@ def redact_telegram_bot_token(event: Event, _hint: dict[str, Any]) -> Event | No # недоступен/rate-limit/403) улетало в GlitchTip как RetryError с этим # нестабильным текстом → одна и та же причина плодила отдельный issue на КАЖДОЕ # исчерпание (2 462 issue из 7 461 в трекере на момент фикса). `reraise=True` -# теперь пробрасывает реальное исключение вместо RetryError — это ГЛАВНЫЙ фикс. -# Функция ниже — belt-and-suspenders: ловит RetryError, если он всё же всплывёт -# (ретрай-код, который забыли/не смогли пометить reraise=True), и схлопывает его -# в ОДИН persistent issue per (класс исключения-причины), а не тысячу. Fingerprint -# собран ТОЛЬКО из стабильных данных — имя типа исключения-причины (небольшой -# фиксированный словарь вроде "HTTPStatusError"/"ConnectTimeout") — НИКАКИХ -# переменных данных запроса (адрес, IP, id объявления и т.п.), иначе проблема -# повторится в других терминах. +# в app/services/geocoder.py устраняет RetryError на этом пути (пробрасывает +# реальное исключение) — но реальное исключение (httpx.HTTPStatusError) само +# несёт нестабильный текст (URL с адресом в query), поэтому group-стабильность +# для geocoder держит НЕ эта функция, а `_HTTPX_ERROR_URL_QUERY_RE` в +# `scrub_pii_event` (см. её комментарий, GlitchTip-noise review round 2 claim #1). +# +# Функция ниже — belt-and-suspenders для ЛЮБОГО кода, который ретраит через +# tenacity БЕЗ `reraise=True` (живой пример на момент фикса: `BaseScraper._http_get` +# в packages/scraper-kit — retry-декоратор НЕ reraise'ит, сознательно оставлен на +# этот фолбэк, а не на URL-стабилизацию: ретраятся listing detail URL БЕЗ query +# string — переменная часть там в ПУТИ (offer id), которую `_HTTPX_ERROR_URL_QUERY_RE` +# не покрывает; см. review round 2 claim #3). Схлопывает RetryError в ОДИН +# persistent issue per (culprit, класс исключения-причины) — culprit обязателен: +# БЕЗ него RetryError с одинаковым типом причины из НЕСВЯЗАННЫХ подсистем (напр. +# geocoder и scraper_kit одновременно ретраят httpx и оба ловят HTTPStatusError) +# схлопнулись бы в ОДИН issue — потеря сигнала хуже исходного шума (review round 2 +# claim #2). Источник culprit — `event["logger"]`: sentry_sdk `LoggingIntegration` +# ставит его в имя logger'а (`logging.getLogger(__name__)`, напр. +# "app.services.geocoder" vs "scraper_kit.providers.yandex.detail") на КАЖДОМ +# `logger.exception(...)`/`logger.error(...)` — стабильно per-модуль, не зависит от +# конкретного запроса. Остальная часть fingerprint собрана ТОЛЬКО из стабильных +# данных — имя типа исключения-причины (небольшой фиксированный словарь вроде +# "HTTPStatusError"/"ConnectTimeout") — НИКАКИХ переменных данных запроса (адрес, +# IP, id объявления и т.п.), иначе проблема повторится в других терминах. def stabilize_retry_error_fingerprint(event: Event, hint: dict[str, Any]) -> Event | None: - """before_send-хук: схлопывает tenacity.RetryError в один persistent issue. + """before_send-хук: схлопывает tenacity.RetryError в один persistent issue per + (источник, тип причины) — РАЗНЫЕ источники (geocoder / scraper_kit / будущий + retry-код) НЕ схлопываются друг с другом, даже если тип причины совпадает. Определяет тип exception через `hint["exc_info"]` (реальный объект исключения, тот же контракт что sentry_sdk передаёт в before_send) — не парсит уже сериализованный event dict, надёжнее к изменениям формата SDK. - Не-RetryError события возвращает без изменений (OperationalError, алерты - scraper sweep'ов и т.п. фильтр не трогает). + `isinstance` (не сравнение `type(...).__name__` со строкой) — иначе любой + посторонний класс с совпадающим именем ложно матчился бы, а подкласс + `tenacity.RetryError` — промахивался бы. Не-RetryError события возвращает без + изменений (OperationalError, алерты scraper sweep'ов и т.п. фильтр не трогает). """ if not isinstance(event, dict): return event exc_info = hint.get("exc_info") if isinstance(hint, dict) else None exc_value = exc_info[1] if exc_info and len(exc_info) > 1 else None - if exc_value is None or type(exc_value).__name__ != "RetryError": + if not isinstance(exc_value, RetryError): return event cause = exc_value.__cause__ or exc_value.__context__ cause_type = type(cause).__name__ if cause is not None else "Unknown" - event["fingerprint"] = ["retry-exhausted", cause_type] + culprit = event.get("logger") or event.get("transaction") or "unknown" + event["fingerprint"] = ["retry-exhausted", str(culprit), cause_type] return event diff --git a/tradein-mvp/backend/app/services/geocoder.py b/tradein-mvp/backend/app/services/geocoder.py index 59b3f50d..2d3c7096 100644 --- a/tradein-mvp/backend/app/services/geocoder.py +++ b/tradein-mvp/backend/app/services/geocoder.py @@ -748,9 +748,16 @@ async def _nominatim_query(client: httpx.AsyncClient, address: str) -> dict | No # группирует по этому нестабильному тексту → одна и та же причина (Nominatim # недоступен/rate-limit) плодила отдельный issue на каждое исчерпание ретраев # (2 462 issue из 7 461 в трекере). reraise=True пробрасывает РЕАЛЬНОЕ исключение -# (httpx.HTTPStatusError/TimeoutException) — стабильный тип+стек, группируется -# нормально. Дополнительная страховка — stabilize_retry_error_fingerprint -# (app/observability/sentry_scrub.py) на случай если RetryError всплывёт откуда-то ещё. +# (httpx.HTTPStatusError/TimeoutException) — стабильный ТИП+стек. НО httpx.HTTPStatusError +# сам несёт нестабильный ТЕКСТ (str() содержит полный request URL, включая query +# string с адресом — `for url '...search?q=<адрес>&...'`) — group-стабильность на +# ЭТОМ пути держит `_HTTPX_ERROR_URL_QUERY_RE` в app/observability/sentry_scrub.py +# (`scrub_pii_event`, часть before_send-композиции обоих entrypoint), которая режет +# query string из httpx-style "for url '...'" сообщений (GlitchTip-noise review +# round 2, claim #1 — reraise=True сам по себе НЕ закрывает per-address explosion). +# Отдельно — `stabilize_retry_error_fingerprint` (та же sentry_scrub.py) на случай +# если голый tenacity.RetryError (не httpx-исключение) всплывёт откуда-то ещё +# (belt-and-suspenders для retry-кода без reraise=True, напр. scraper_kit). @retry(stop=stop_after_attempt(3), wait=wait_exponential(multiplier=1, min=1, max=8), reraise=True) async def _nominatim_lookup(address: str, city_hint: str | None = None) -> GeocodeResult | None: """OSM Nominatim — бесплатно, без ключа, 1 req/sec policy. diff --git a/tradein-mvp/backend/tests/test_sentry_scrub.py b/tradein-mvp/backend/tests/test_sentry_scrub.py index 242c03d4..67d26650 100644 --- a/tradein-mvp/backend/tests/test_sentry_scrub.py +++ b/tradein-mvp/backend/tests/test_sentry_scrub.py @@ -270,9 +270,12 @@ def test_composed_before_send_scrubs_pii_and_token_together() -> None: # GlitchTip как RetryError с этим нестабильным текстом → одна и та же причина # плодила отдельный issue на КАЖДОЕ исчерпание (2 462 issue из 7 461 в трекере). # Тесты ниже бьют по `stabilize_retry_error_fingerprint` напрямую — belt-and- -# suspenders слой (основной фикс — reraise=True), и по контракту before_send: -# 401-класс (RetryError) схлопывается, содержательные категории (500-подобный -# generic Exception, OperationalError) проходят НЕТРОНУТЫМИ. +# suspenders слой для retry-кода БЕЗ reraise=True (напр. scraper_kit — +# geocoder.py `reraise=True` устраняет RetryError на своём пути, но остаётся +# фолбэком общего назначения), и по контракту before_send: 401-класс (RetryError) +# схлопывается ПО ИСТОЧНИКУ (не глобально — review round 2 claim #2: разные +# подсистемы с совпавшим типом причины НЕ сливаются), содержательные категории +# (500-подобный generic Exception, OperationalError) проходят НЕТРОНУТЫМИ. from tenacity import RetryError # noqa: E402 @@ -295,18 +298,20 @@ def _raise_retry_error_from(cause: BaseException) -> RetryError: def test_stabilize_retry_error_sets_stable_fingerprint() -> None: - """RetryError коллапсится в persistent issue по имени типа причины — НЕ по - нестабильному str(RetryError) (repr() Future с memory address).""" + """RetryError коллапсится в persistent issue по (culprit, имени типа причины) — + НЕ по нестабильному str(RetryError) (repr() Future с memory address). Без + `event["logger"]` (напр. capture_exception без LoggingIntegration) culprit + падает на явный "unknown", а не пропадает из fingerprint молча.""" exc = _raise_retry_error_from(TimeoutError("Nominatim timed out")) out = stabilize_retry_error_fingerprint({"level": "error"}, _hint_for(exc)) assert out is not None - assert out["fingerprint"] == ["retry-exhausted", "TimeoutError"] + assert out["fingerprint"] == ["retry-exhausted", "unknown", "TimeoutError"] def test_stabilize_retry_error_fingerprint_has_no_variable_data() -> None: """Fingerprint не должен содержать IP/id объявления/адрес и т.п. — только - фиксированное имя типа исключения-причины (маленький словарь: - HTTPStatusError/ConnectTimeout/TimeoutError/...).""" + culprit (logger-имя модуля) + фиксированное имя типа исключения-причины + (маленький словарь: HTTPStatusError/ConnectTimeout/TimeoutError/...).""" exc = _raise_retry_error_from( ValueError("addr='ул. Ленина 1', ip=95.165.147.218, listing_id=12345") ) @@ -315,7 +320,53 @@ def test_stabilize_retry_error_fingerprint_has_no_variable_data() -> None: fingerprint_text = " ".join(out["fingerprint"]) assert "95.165.147.218" not in fingerprint_text assert "12345" not in fingerprint_text - assert out["fingerprint"] == ["retry-exhausted", "ValueError"] + assert out["fingerprint"] == ["retry-exhausted", "unknown", "ValueError"] + + +def test_stabilize_retry_error_fingerprint_uses_logger_as_culprit() -> None: + """`event["logger"]` (sentry_sdk LoggingIntegration ставит его = имя модуля, + вызвавшего logger.exception/.error) идёт в fingerprint как culprit — стабильно + per-модуль, не переменные данные запроса.""" + exc = _raise_retry_error_from(TimeoutError("timed out")) + out = stabilize_retry_error_fingerprint({"logger": "app.services.geocoder"}, _hint_for(exc)) + assert out is not None + assert out["fingerprint"] == ["retry-exhausted", "app.services.geocoder", "TimeoutError"] + + +def test_stabilize_retry_error_fingerprint_does_not_collapse_unrelated_subsystems() -> None: + """Review round 2 claim #2: RetryError с ОДИНАКОВЫМ типом причины из + НЕСВЯЗАННЫХ подсистем (geocoder vs scraper_kit) НЕ должны схлопнуться в один + issue — разные проблемы не сливаются, даже если типы причины совпали.""" + exc_geocoder = _raise_retry_error_from(TimeoutError("nominatim timed out")) + exc_scraper = _raise_retry_error_from(TimeoutError("yandex detail timed out")) + out_geocoder = stabilize_retry_error_fingerprint( + {"logger": "app.services.geocoder"}, _hint_for(exc_geocoder) + ) + out_scraper = stabilize_retry_error_fingerprint( + {"logger": "scraper_kit.providers.yandex.detail"}, _hint_for(exc_scraper) + ) + assert out_geocoder is not None + assert out_scraper is not None + assert out_geocoder["fingerprint"] != out_scraper["fingerprint"] + + +class _DecoyRetryError(Exception): + """Посторонний класс, СЛУЧАЙНО названный так же, как tenacity.RetryError — + но НЕ его подкласс. Строковое сравнение имён (старый баг, review round 2 + claim #4) ложно матчило бы такое; isinstance — нет.""" + + +_DecoyRetryError.__name__ = "RetryError" # type: ignore[misc] + + +def test_stabilize_retry_error_ignores_lookalike_class_by_name() -> None: + """type(exc).__name__ == "RetryError" НЕ должно быть достаточно — только + реальный tenacity.RetryError (или его подкласс) триггерит fingerprint-хук.""" + exc = _DecoyRetryError("unrelated exception, same class __name__ by accident") + event = {"level": "error"} + out = stabilize_retry_error_fingerprint(dict(event), _hint_for(exc)) + assert out == event + assert "fingerprint" not in out def test_stabilize_retry_error_leaves_operational_error_untouched() -> None: @@ -351,3 +402,74 @@ def test_stabilize_retry_error_no_exc_info_untouched() -> None: def test_stabilize_retry_error_handles_non_dict_event() -> None: assert stabilize_retry_error_fingerprint(None, {}) is None # type: ignore[arg-type] + + +# ── httpx error-message URL query stabilization (glitchtip-noise review round 2, +# claim #1) ──────────────────────────────────────────────────────────────── +# +# `httpx.HTTPStatusError.__str__()` (raised by `response.raise_for_status()`) bakes +# the FULL request URL — including query string — into the exception message. +# Воспроизведено эмпирически (httpx.Response(403, request=...).raise_for_status()): +# "Client error '403 Forbidden' for url 'https://nominatim.openstreetmap.org/ +# search?q=<адрес>&format=json&limit=3'". После `reraise=True` в geocoder.py +# (устраняет RetryError, но НЕ этот текст) именно ЭТА строка становится GlitchTip +# title/value — переменный `q=<адрес>` на каждый вызов воспроизводит тот же +# per-address issue-explosion, который reraise=True должен был устранить, просто +# сменивший класс исключения (RetryError → HTTPStatusError). Тесты бьют по +# `scrub_pii_event` напрямую (композиция, реально применяемая в before_send). + +_NOMINATIM_403_TEMPLATE = ( + "Client error '403 Forbidden' for url " + "'https://nominatim.openstreetmap.org/search?q={query}&format=json&limit=3'\n" + "For more information check: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/403" +) + + +def _httpx_error_event(message: str) -> dict: + return {"exception": {"values": [{"type": "HTTPStatusError", "value": message}]}} + + +def test_scrub_pii_event_stabilizes_httpx_error_url_query() -> None: + """Query string режется целиком из httpx-style 'for url' сообщения — host+path + остаются стабильными для группировки.""" + encoded_ekb = "%D0%95%D0%BA%D0%B0%D1%82%D0%B5%D1%80%D0%B8%D0%BD%D0%B1%D1%83%D1%80%D0%B3" + event = _httpx_error_event(_NOMINATIM_403_TEMPLATE.format(query=encoded_ekb)) + out = scrub_pii_event(event, {}) + assert out is not None + value = out["exception"]["values"][0]["value"] + assert "search?[REDACTED]'" in value + assert "%D0%95" not in value + assert "nominatim.openstreetmap.org/search" in value # host+path сохранены + + +def test_scrub_pii_event_httpx_url_query_stabilization_collapses_different_addresses() -> None: + """Два РАЗНЫХ адреса (переменная часть query) после редактора дают + ИДЕНТИЧНЫЙ текст сообщения — GlitchTip group-title больше не плодит issue + на каждый адрес (review round 2 claim #1).""" + event_a = _httpx_error_event(_NOMINATIM_403_TEMPLATE.format(query="ул.+Ленина+1")) + event_b = _httpx_error_event(_NOMINATIM_403_TEMPLATE.format(query="ул.+Мира+42%2C+кв.+5")) + out_a = scrub_pii_event(event_a, {}) + out_b = scrub_pii_event(event_b, {}) + assert out_a is not None + assert out_b is not None + assert out_a["exception"]["values"][0]["value"] == out_b["exception"]["values"][0]["value"] + + +def test_scrub_pii_event_httpx_url_without_query_untouched() -> None: + """URL без query string (напр. scraper detail page — переменная часть в + ПУТИ, не в query) остаётся нетронутым — regex матчит только `?...`.""" + message = "Client error '404 Not Found' for url 'https://realty.yandex.ru/offer/12345/'" + event = _httpx_error_event(message) + out = scrub_pii_event(event, {}) + assert out is not None + assert out["exception"]["values"][0]["value"] == message + + +def test_scrub_pii_event_httpx_url_query_stabilization_leaves_unrelated_text_untouched() -> None: + """Regex бьёт только по 'for url \\'...?...\\'' — произвольный текст с `?` и + кавычками не должен ложно матчиться.""" + benign = "Вопрос: 'что такое ЖК \"Солнечный\"?' — уточните адрес" + event = {"extra": {"note": benign}} + out = scrub_pii_event(event, {}) + assert out is not None + assert out["extra"]["note"] == benign -- 2.45.3