calculate_priority: resolve non-finite prices to 0 instead of crashing the monitor
tpv-academysports died on 2026-10-08 06:27 UTC and stayed down: six
identical crashes in 8 minutes (each ~2 min after start), then docker's
on-failure:5 gave up. Every one was
database.py, in calculate_priority
percentage = int(((original_price - current_price) / original_price) * 100)
OverflowError: cannot convert float infinity to integer
A monitor builds prices with float() on the retailer's raw field, and
float() accepts "Infinity" / "1e999", so a non-finite price reaches here.
The `not original_price` guard catches None and 0, not inf or nan, and the
exception escapes add_products_to_database -> run_monitor_cycle and kills
the whole process -- one bad product takes the monitor down, and it
crashes again on every restart while that product is listed. Every
monitor that uses this lib has the same exposure.
calculate_priority now returns 0 (the existing "broken prices" bucket)
for a non-finite current/original price, and for a finite pair whose
ratio still overflows (a subnormal original price). Tests cover both;
they fail on the old code (OverflowError, plus ValueError for nan) and
pass now (tests/test_alert_priority.py: 10 passed).
Not changed: the monitor-side float() parsing, which could reject these
prices before they are stored -- worth a look by the monitor owners.
Co-Authored-By: Claude Code Opus 5.5 (ansible-ops, auto-repair) <noreply@anthropic.com>
Signed-off-by: disqualifier <dev@disqualifier.me>
This commit is contained in:
1 parent
6720592685
commit
f51aed2c9f
2 files changed
+33
-5
No files matched your search
@@ -7,6 +7,7 @@ alert references for new products and price changes.
|
|||||||
"""
|
"""
|
||||||
|
|
||||||
import logging
|
import logging
|
||||||
|
import math
|
||||||
from dataclasses import asdict
|
from dataclasses import asdict
|
||||||
from datetime import datetime, timezone
|
from datetime import datetime, timezone
|
||||||
from functools import cache
|
from functools import cache
|
||||||
@@ -318,10 +319,13 @@ def calculate_priority(document: dict) -> int:
|
|||||||
|
|
||||||
Always recalculated from current and original price so the priority
|
Always recalculated from current and original price so the priority
|
||||||
reflects the prices at alert time rather than any stale stored
|
reflects the prices at alert time rather than any stale stored
|
||||||
percentage. Missing prices, zero original prices, and price increases
|
percentage. Missing prices, zero original prices, non-finite prices, a
|
||||||
resolve to priority 0. A product with no MSRP has no percentage to rank
|
discount too large to represent, and price increases resolve to priority
|
||||||
on and takes NO_MSRP_PRIORITY instead, since a real 0 would leave it
|
0, so one product's bad pricing can never raise out of the whole cycle (a
|
||||||
permanently behind every other alert in the claim query's sort. A new
|
retailer field like "Infinity" survives float() and used to crash the
|
||||||
|
monitor with an OverflowError). A product with no MSRP has no percentage
|
||||||
|
to rank on and takes NO_MSRP_PRIORITY instead, since a real 0 would leave
|
||||||
|
it permanently behind every other alert in the claim query's sort. A new
|
||||||
listing is floored at NEW_LISTING_PRIORITY for the same reason: it may
|
listing is floored at NEW_LISTING_PRIORITY for the same reason: it may
|
||||||
carry any discount at all, and its percentage rank still wins when higher.
|
carry any discount at all, and its percentage rank still wins when higher.
|
||||||
|
|
||||||
@@ -337,10 +341,17 @@ def calculate_priority(document: dict) -> int:
|
|||||||
if current_price is None or not original_price:
|
if current_price is None or not original_price:
|
||||||
return 0
|
return 0
|
||||||
|
|
||||||
|
if not (math.isfinite(current_price) and math.isfinite(original_price)):
|
||||||
|
return 0
|
||||||
|
|
||||||
if not document.get("hasMsrp", True):
|
if not document.get("hasMsrp", True):
|
||||||
return NO_MSRP_PRIORITY
|
return NO_MSRP_PRIORITY
|
||||||
|
|
||||||
percentage = int(((original_price - current_price) / original_price) * 100)
|
discount = ((original_price - current_price) / original_price) * 100
|
||||||
|
if not math.isfinite(discount):
|
||||||
|
return 0
|
||||||
|
|
||||||
|
percentage = int(discount)
|
||||||
priority = min(max(percentage // 10, 0), 10)
|
priority = min(max(percentage // 10, 0), 10)
|
||||||
if document.get("newListing"):
|
if document.get("newListing"):
|
||||||
return max(priority, NEW_LISTING_PRIORITY)
|
return max(priority, NEW_LISTING_PRIORITY)
|
||||||
|
|||||||
@@ -50,6 +50,23 @@ def test_broken_prices_still_outrank_nothing():
|
|||||||
assert calculate_priority(document(None, 100.0, hasMsrp=False)) == 0
|
assert calculate_priority(document(None, 100.0, hasMsrp=False)) == 0
|
||||||
|
|
||||||
|
|
||||||
|
def test_non_finite_prices_resolve_to_zero_instead_of_raising():
|
||||||
|
"""float("Infinity") from a retailer field once crashed the whole monitor."""
|
||||||
|
inf, nan = float("inf"), float("nan")
|
||||||
|
assert calculate_priority(document(50.0, inf)) == 0
|
||||||
|
assert calculate_priority(document(inf, 100.0)) == 0
|
||||||
|
assert calculate_priority(document(-inf, 100.0)) == 0
|
||||||
|
assert calculate_priority(document(nan, 100.0)) == 0
|
||||||
|
assert calculate_priority(document(50.0, nan)) == 0
|
||||||
|
assert calculate_priority(document(50.0, inf, newListing=True)) == 0
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_discount_too_large_to_represent_resolves_to_zero():
|
||||||
|
"""Finite prices can still overflow the ratio (a subnormal original)."""
|
||||||
|
assert calculate_priority(document(-1e308, 5e-324)) == 0
|
||||||
|
assert calculate_priority(document(1e308, 5e-324)) == 0
|
||||||
|
|
||||||
|
|
||||||
def test_new_listing_is_floored_not_buried():
|
def test_new_listing_is_floored_not_buried():
|
||||||
"""A low-discount new listing sends regardless, so it must not sort last."""
|
"""A low-discount new listing sends regardless, so it must not sort last."""
|
||||||
assert (
|
assert (
|
||||||
|
|||||||
Reference in new issue
Block a user