fix(ptica): нечисловое значение от OSRM/ORS больше не даёт 500 вместо деградации (#2464)
All checks were successful
CI Trade-In / changes (pull_request) Successful in 7s
CI Trade-In / backend-tests (pull_request) Has been skipped
CI Trade-In / browser-tests (pull_request) Has been skipped
CI / changes (pull_request) Successful in 9s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Successful in 2m15s
CI / backend-tests (pull_request) Successful in 17m15s
All checks were successful
CI Trade-In / changes (pull_request) Successful in 7s
CI Trade-In / backend-tests (pull_request) Has been skipped
CI Trade-In / browser-tests (pull_request) Has been skipped
CI / changes (pull_request) Successful in 9s
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Successful in 2m15s
CI / backend-tests (pull_request) Successful in 17m15s
Оба клиента маршрутизации переводят ЛЮБУЮ кривизну ответа в доменную ошибку
(OsrmLocalUnavailableError / OrsUnavailableError), потому что вызывающие ловят
только её и уходят на прямолинейный fallback:
parcels.py:399 и poi_score.py:348.
Конверсия значения была исключением из этого правила:
out.append(float(d)) # osrm_client_local
out.append(float(sec) / 60.0) # ors_client
Голый float() на нечисловом значении поднимает ValueError или TypeError — они
пролетают мимо обработчиков и дают 500 на /analyze вместо деградации к
прямолинейным расстояниям.
В ors_client довод уже был сформулирован ДВУМЯ СТРОКАМИ НИЖЕ: у проверки длины
написано «иначе zip(strict=True) у вызывающего бросит ValueError (не
OrsUnavailableError) → 500. Закрываем как ORS-сбой». Принцип верный, к самой
конверсии его просто не применили.
Тесты: 4 красных на origin/main с настоящими исключениями —
`ValueError: could not convert string to float: 'не число'` и
`TypeError: float() argument must be a string or a real number, not 'list'`.
Проверены оба типа исключений, а не только ValueError.
Контроль отдельно: null в ответе — это «маршрут не построен», законный None, а не
поломка. Зелёный с обеих сторон, иначе правка могла бы превратить легитимный
пропуск в ошибку.
pytest poi_score + ors + osrm + analyze_osrm_distances: 54 passed, rc=0
pytest tests/services: 3074 passed, 14 skipped, rc=0
Прогон повторён после правок pre-commit ruff-format.
This commit is contained in:
parent
e543fcdd81
commit
05841b6825
4 changed files with 98 additions and 10 deletions
|
|
@ -24,9 +24,7 @@ logger = logging.getLogger(__name__)
|
|||
_ORS_MATRIX_BASE = "https://api.openrouteservice.org/v2/matrix"
|
||||
# Профили ORS-routing. foot-walking — пеший радиус (метро/школа/магазин),
|
||||
# driving-car — авто (для будущих авто-категорий).
|
||||
VALID_PROFILES: frozenset[str] = frozenset(
|
||||
{"foot-walking", "cycling-regular", "driving-car"}
|
||||
)
|
||||
VALID_PROFILES: frozenset[str] = frozenset({"foot-walking", "cycling-regular", "driving-car"})
|
||||
# ORS /matrix ограничивает foot-walking ~2000 пар (sources×destinations) на free tier.
|
||||
MAX_MATRIX_DESTINATIONS = 1000
|
||||
_DEFAULT_TIMEOUT_S = 12.0
|
||||
|
|
@ -128,7 +126,16 @@ def matrix_durations_min(
|
|||
if sec is None:
|
||||
out.append(None) # ORS не построил маршрут до этой точки
|
||||
else:
|
||||
out.append(float(sec) / 60.0)
|
||||
# #2464: тот же довод, что у проверки длины ниже — нечисловое значение
|
||||
# дало бы ValueError/TypeError мимо OrsUnavailableError, а вызывающий
|
||||
# (poi_score.py:348) ловит только её. Принцип в этом файле уже
|
||||
# сформулирован, просто не применён к самой конверсии.
|
||||
try:
|
||||
out.append(float(sec) / 60.0)
|
||||
except (TypeError, ValueError) as exc:
|
||||
raise OrsUnavailableError(
|
||||
f"ORS matrix: нечисловая длительность {sec!r} в durations"
|
||||
) from exc
|
||||
# Длина durations должна совпадать с числом destinations — иначе zip(strict=True)
|
||||
# у вызывающего бросит ValueError (не OrsUnavailableError) → 500. Закрываем как ORS-сбой.
|
||||
if len(out) != len(dests):
|
||||
|
|
|
|||
|
|
@ -96,8 +96,7 @@ def get_road_distances_m(
|
|||
|
||||
# Координаты `;`-joined: origin первой (→ sources=0), затем POI по порядку.
|
||||
coords = ";".join(
|
||||
[_fmt_coord(origin_lon, origin_lat)]
|
||||
+ [_fmt_coord(lon, lat) for lon, lat in destinations]
|
||||
[_fmt_coord(origin_lon, origin_lat)] + [_fmt_coord(lon, lat) for lon, lat in destinations]
|
||||
)
|
||||
base = (base_url if base_url is not None else settings.osrm_local_url).rstrip("/")
|
||||
url = f"{base}/table/v1/{profile}/{coords}"
|
||||
|
|
@ -134,7 +133,17 @@ def get_road_distances_m(
|
|||
if d is None:
|
||||
out.append(None) # OSRM не построил маршрут до этой точки
|
||||
else:
|
||||
out.append(float(d))
|
||||
# #2464: конверсия обязана падать в ДОМЕННУЮ ошибку. Весь файл переводит
|
||||
# любую кривизну ответа в OsrmLocalUnavailableError (строки выше), потому
|
||||
# что вызывающий (parcels.py:399) ловит ТОЛЬКО её и уходит на прямолинейный
|
||||
# fallback. Голый float() на нечисловом значении поднял бы ValueError или
|
||||
# TypeError — они пролетят мимо и дадут 500 на /analyze вместо деградации.
|
||||
try:
|
||||
out.append(float(d))
|
||||
except (TypeError, ValueError) as exc:
|
||||
raise OsrmLocalUnavailableError(
|
||||
f"OSRM table: нечисловое расстояние {d!r} в distances"
|
||||
) from exc
|
||||
|
||||
# Длина должна совпадать с числом destinations — иначе zip у вызывающего
|
||||
# рассинхронит POI↔distance. Закрываем как OSRM-сбой → straight-line fallback.
|
||||
|
|
|
|||
|
|
@ -121,3 +121,32 @@ def test_matrix_bad_response_raises(with_key, monkeypatch):
|
|||
_install_transport(monkeypatch, handler)
|
||||
with pytest.raises(ors_client.OrsUnavailableError):
|
||||
ors_client.matrix_durations_min(60.6, 56.8, [(60.61, 56.81)])
|
||||
|
||||
|
||||
def test_non_numeric_duration_degrades_instead_of_500(with_key, monkeypatch):
|
||||
"""#2464: нечисловая длительность → OrsUnavailableError, а не ValueError.
|
||||
|
||||
Довод уже сформулирован в самом файле — у проверки длины ниже написано, что
|
||||
ValueError «не OrsUnavailableError → 500». К самой конверсии принцип применён
|
||||
не был: вызывающий (poi_score.py:348) ловит только доменную ошибку.
|
||||
"""
|
||||
|
||||
def handler(_request):
|
||||
return httpx.Response(200, json={"durations": [[0.0, "не число"]]})
|
||||
|
||||
_install_transport(monkeypatch, handler)
|
||||
|
||||
with pytest.raises(ors_client.OrsUnavailableError):
|
||||
ors_client.matrix_durations_min(60.6, 56.8, [(60.61, 56.81), (60.62, 56.82)])
|
||||
|
||||
|
||||
def test_list_instead_of_duration_also_degrades(with_key, monkeypatch):
|
||||
"""TypeError тоже обязан стать доменной ошибкой, не только ValueError."""
|
||||
|
||||
def handler(_request):
|
||||
return httpx.Response(200, json={"durations": [[0.0, [300]]]})
|
||||
|
||||
_install_transport(monkeypatch, handler)
|
||||
|
||||
with pytest.raises(ors_client.OrsUnavailableError):
|
||||
ors_client.matrix_durations_min(60.6, 56.8, [(60.61, 56.81), (60.62, 56.82)])
|
||||
|
|
|
|||
|
|
@ -41,9 +41,7 @@ def test_builds_correct_url_and_parses_meters(monkeypatch):
|
|||
return httpx.Response(200, json={"code": "Ok", "distances": [[0.0, 1234.5, 6789.0]]})
|
||||
|
||||
_install_transport(monkeypatch, handler)
|
||||
out = osrm.get_road_distances_m(
|
||||
60.6, 56.8, [(60.61, 56.81), (60.62, 56.82)]
|
||||
)
|
||||
out = osrm.get_road_distances_m(60.6, 56.8, [(60.61, 56.81), (60.62, 56.82)])
|
||||
|
||||
# Дорожные расстояния (метры), self-index сброшен.
|
||||
assert out == [1234.5, 6789.0]
|
||||
|
|
@ -164,3 +162,48 @@ def test_length_mismatch_raises_unavailable(monkeypatch):
|
|||
_install_transport(monkeypatch, handler)
|
||||
with pytest.raises(osrm.OsrmLocalUnavailableError, match="!= destinations"):
|
||||
osrm.get_road_distances_m(60.6, 56.8, [(60.61, 56.81), (60.62, 56.82)])
|
||||
|
||||
|
||||
def test_non_numeric_distance_degrades_instead_of_500(monkeypatch):
|
||||
"""#2464: нечисловое значение в distances → доменная ошибка, а не ValueError.
|
||||
|
||||
Весь файл переводит любую кривизну ответа в OsrmLocalUnavailableError, потому что
|
||||
вызывающий (parcels.py:399) ловит ТОЛЬКО её и уходит на прямолинейный fallback.
|
||||
Голый `float(d)` был исключением из этого правила: ValueError/TypeError пролетели
|
||||
бы мимо обработчика и дали 500 на /analyze вместо деградации.
|
||||
"""
|
||||
|
||||
def handler(request: httpx.Request) -> httpx.Response:
|
||||
return httpx.Response(200, json={"code": "Ok", "distances": [[0.0, "не число", 10.0]]})
|
||||
|
||||
_install_transport(monkeypatch, handler)
|
||||
|
||||
with pytest.raises(osrm.OsrmLocalUnavailableError):
|
||||
osrm.get_road_distances_m(60.6, 56.8, [(60.61, 56.81), (60.62, 56.82)])
|
||||
|
||||
|
||||
def test_dict_instead_of_distance_also_degrades(monkeypatch):
|
||||
"""TypeError (не только ValueError) тоже обязан стать доменной ошибкой."""
|
||||
|
||||
def handler(request: httpx.Request) -> httpx.Response:
|
||||
return httpx.Response(200, json={"code": "Ok", "distances": [[0.0, {"m": 5}, 10.0]]})
|
||||
|
||||
_install_transport(monkeypatch, handler)
|
||||
|
||||
with pytest.raises(osrm.OsrmLocalUnavailableError):
|
||||
osrm.get_road_distances_m(60.6, 56.8, [(60.61, 56.81), (60.62, 56.82)])
|
||||
|
||||
|
||||
def test_null_distance_is_still_a_legitimate_none(monkeypatch):
|
||||
"""Контроль: null — это «маршрут не построен», а не поломка ответа.
|
||||
|
||||
Зелёный с обеих сторон: правка не должна превращать законный None в ошибку.
|
||||
"""
|
||||
|
||||
def handler(request: httpx.Request) -> httpx.Response:
|
||||
return httpx.Response(200, json={"code": "Ok", "distances": [[0.0, None, 10.0]]})
|
||||
|
||||
_install_transport(monkeypatch, handler)
|
||||
out = osrm.get_road_distances_m(60.6, 56.8, [(60.61, 56.81), (60.62, 56.82)])
|
||||
|
||||
assert out == [None, 10.0]
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue