From 6ea3187b3311cd5efeda62f45fc19506bfa310de Mon Sep 17 00:00:00 2001 From: disqualifier Date: Fri, 9 Oct 2026 23:22:36 -0400 Subject: [PATCH] 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) Signed-off-by: disqualifier --- src/monitor_utils/runner.py | 31 ++++++++++++++++++++--- tests/test_build_product_details.py | 38 +++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/src/monitor_utils/runner.py b/src/monitor_utils/runner.py index c8f3e19..f17b04f 100644 --- a/src/monitor_utils/runner.py +++ b/src/monitor_utils/runner.py @@ -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. diff --git a/tests/test_build_product_details.py b/tests/test_build_product_details.py index 0d61676..fc55f18 100644 --- a/tests/test_build_product_details.py +++ b/tests/test_build_product_details.py @@ -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