build_product_details: skip non-finite prices instead of raising OverflowError
PR #25 guarded calculate_priority but missed the same calculation here. On 2026-10-09 06:25:50Z the consumer hit a product with an infinite price; runner.py:201 percentage = int(((original_price - current_price) / original_price) * 100) raised `OverflowError: cannot convert float infinity to integer`. The Discord leg dropped that one alert, but the API leg's caller does not catch OverflowError, so the exception escaped and the consumer logged CRITICAL "api deal loop stopped" -- no deals reached the API from then on (nothing sent at all after 22:00Z), while the bot itself stayed "alive". build_product_details now returns None (with a warning, like the existing missing-price skip) when either price is a non-finite float, on BOTH paths: the MSRP-less path never computes a percentage and would otherwise carry inf into the Product and the API payload. A finite pair whose ratio still overflows (a subnormal original price) is skipped too. Ints and other types keep their existing behaviour. Tests (tests/test_build_product_details.py): five new cases fail on the old code -- OverflowError, ValueError for nan, and the MSRP-less case building a Product with inf -- and all 25 pass now. Ruff findings on the two files are unchanged from main (5, all pre-existing). Rebuild every service that installs monitor-utils with --no-cache to pick this up (it is pulled unpinned from main). 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
f51aed2c9f
commit
6ea3187b33
2 files changed
+66
-3
No files matched your search
@@ -1,5 +1,6 @@
|
|||||||
import asyncio
|
import asyncio
|
||||||
import logging
|
import logging
|
||||||
|
import math
|
||||||
from collections.abc import Awaitable, Callable, Iterable
|
from collections.abc import Awaitable, Callable, Iterable
|
||||||
from contextlib import asynccontextmanager, nullcontext
|
from contextlib import asynccontextmanager, nullcontext
|
||||||
|
|
||||||
@@ -160,7 +161,10 @@ def build_product_details(
|
|||||||
) -> Product | None:
|
) -> Product | None:
|
||||||
"""Build a Product from a Mongo document if it passes discount thresholds.
|
"""Build a Product from a Mongo document if it passes discount thresholds.
|
||||||
|
|
||||||
Skips documents with a missing or zero original price. Passing requires
|
Skips documents with a missing or zero original price, a non-finite
|
||||||
|
price, or a discount too large to represent: a retailer field like
|
||||||
|
"Infinity" survives float(), and the percentage math used to raise
|
||||||
|
OverflowError out of every caller. Passing requires
|
||||||
meeting min_percentage, or min_price_drop when provided. The caller may
|
meeting min_percentage, or min_price_drop when provided. The caller may
|
||||||
bypass both thresholds (new listings send at any discount for monitors
|
bypass both thresholds (new listings send at any discount for monitors
|
||||||
that opt in); which alerts qualify is the caller's decision, not read
|
that opt in); which alerts qualify is the caller's decision, not read
|
||||||
@@ -173,7 +177,7 @@ def build_product_details(
|
|||||||
min_percentage: Minimum discount percentage to pass.
|
min_percentage: Minimum discount percentage to pass.
|
||||||
min_price_drop: Optional minimum absolute price drop to pass.
|
min_price_drop: Optional minimum absolute price drop to pass.
|
||||||
bypass_thresholds: Build the product even when it fails both
|
bypass_thresholds: Build the product even when it fails both
|
||||||
thresholds. The missing-price skip still applies.
|
thresholds. The missing- and non-finite-price skips still apply.
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
The built Product, or None when skipped or below thresholds.
|
The built Product, or None when skipped or below thresholds.
|
||||||
@@ -191,6 +195,19 @@ def build_product_details(
|
|||||||
)
|
)
|
||||||
return
|
return
|
||||||
|
|
||||||
|
# Skipped on BOTH paths, not just the percentage one: an MSRP-less product
|
||||||
|
# would otherwise carry the infinite price into its payload. Only floats can
|
||||||
|
# be non-finite, so ints and other types keep their existing behaviour.
|
||||||
|
if any(
|
||||||
|
isinstance(price, float) and not math.isfinite(price)
|
||||||
|
for price in (current_price, original_price)
|
||||||
|
):
|
||||||
|
LOGGER.warning(
|
||||||
|
"non-finite price, skipping",
|
||||||
|
extra={"product_id": product_id},
|
||||||
|
)
|
||||||
|
return
|
||||||
|
|
||||||
percentage = 0
|
percentage = 0
|
||||||
# Defaulted, not indexed: documents written before the field existed have no
|
# Defaulted, not indexed: documents written before the field existed have no
|
||||||
# hasMsrp at all, and stale ones that drop out of a monitor's feed are never
|
# hasMsrp at all, and stale ones that drop out of a monitor's feed are never
|
||||||
@@ -198,7 +215,15 @@ def build_product_details(
|
|||||||
# pre-existing threshold path rather than silently routing them as MSRP-less.
|
# pre-existing threshold path rather than silently routing them as MSRP-less.
|
||||||
has_msrp = product.get("hasMsrp", True)
|
has_msrp = product.get("hasMsrp", True)
|
||||||
if has_msrp:
|
if has_msrp:
|
||||||
percentage = int(((original_price - current_price) / original_price) * 100)
|
discount = ((original_price - current_price) / original_price) * 100
|
||||||
|
if not math.isfinite(discount):
|
||||||
|
LOGGER.warning(
|
||||||
|
"discount not representable, skipping",
|
||||||
|
extra={"product_id": product_id},
|
||||||
|
)
|
||||||
|
return
|
||||||
|
|
||||||
|
percentage = int(discount)
|
||||||
price_drop = original_price - current_price
|
price_drop = original_price - current_price
|
||||||
# A percentage code compounds onto the sale price, so an item can clear the
|
# A percentage code compounds onto the sale price, so an item can clear the
|
||||||
# threshold on the combined discount even when the base discount would not.
|
# threshold on the combined discount even when the base discount would not.
|
||||||
|
|||||||
@@ -203,3 +203,41 @@ def test_msrpless_doc_with_prices_builds_unthresholded():
|
|||||||
product = build_product_details(doc, min_percentage=40)
|
product = build_product_details(doc, min_percentage=40)
|
||||||
assert product is not None
|
assert product is not None
|
||||||
assert product.pricing.has_msrp is False
|
assert product.pricing.has_msrp is False
|
||||||
|
|
||||||
|
|
||||||
|
# ------------------------------------------------------------ non-finite prices
|
||||||
|
|
||||||
|
|
||||||
|
def test_infinite_price_skips_instead_of_raising():
|
||||||
|
"""Regression (2026-10-09 06:25Z): float("Infinity") from a retailer field
|
||||||
|
raised OverflowError here and stopped the consumer's API deal loop."""
|
||||||
|
|
||||||
|
inf = float("inf")
|
||||||
|
assert build_product_details(_doc(originalPrice=inf), min_percentage=40) is None
|
||||||
|
assert build_product_details(_doc(currentPrice=inf), min_percentage=40) is None
|
||||||
|
assert build_product_details(_doc(currentPrice=-inf), min_percentage=40) is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_nan_price_skips_instead_of_raising():
|
||||||
|
nan = float("nan")
|
||||||
|
assert build_product_details(_doc(currentPrice=nan), min_percentage=40) is None
|
||||||
|
assert build_product_details(_doc(originalPrice=nan), min_percentage=40) is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_msrpless_infinite_price_skips_rather_than_reaching_the_payload():
|
||||||
|
"""Without the skip the MSRP-less path would carry inf into the Product."""
|
||||||
|
|
||||||
|
doc = _doc(hasMsrp=False, currentPrice=float("inf"))
|
||||||
|
assert build_product_details(doc, min_percentage=40) is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_bypass_does_not_rescue_a_non_finite_price():
|
||||||
|
doc = _doc(originalPrice=float("inf"))
|
||||||
|
assert build_product_details(doc, min_percentage=40, bypass_thresholds=True) is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_unrepresentable_discount_skips_instead_of_raising():
|
||||||
|
"""Finite prices can still overflow the ratio (a subnormal original)."""
|
||||||
|
|
||||||
|
doc = _doc(currentPrice=-1e308, originalPrice=5e-324)
|
||||||
|
assert build_product_details(doc, min_percentage=40) is None
|
||||||
Reference in new issue
Block a user