Merge pull request 'build_product_details: skip non-finite prices instead of raising OverflowError' (#26) from development into main

This commit was merged in pull request #26.
This commit is contained in:
dsql committed 2026-10-10 03:24:57 +00:00
commit 1120083e45
2 files changed
+66 -3

No files matched your search

+28 -3
View File
@@ -1,5 +1,6 @@
import asyncio
import logging
import math
from collections.abc import Awaitable, Callable, Iterable
from contextlib import asynccontextmanager, nullcontext
@@ -160,7 +161,10 @@ def build_product_details(
) -> Product | None:
"""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
bypass both thresholds (new listings send at any discount for monitors
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_price_drop: Optional minimum absolute price drop to pass.
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:
The built Product, or None when skipped or below thresholds.
@@ -191,6 +195,19 @@ def build_product_details(
)
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
# 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
@@ -198,7 +215,15 @@ def build_product_details(
# pre-existing threshold path rather than silently routing them as MSRP-less.
has_msrp = product.get("hasMsrp", True)
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
# 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.
+38
View File
@@ -203,3 +203,41 @@ def test_msrpless_doc_with_prices_builds_unthresholded():
product = build_product_details(doc, min_percentage=40)
assert product is not None
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