fix: notification helpers never propagate exceptions
CI on commit 5f735ee broke two double-spend-protection unit tests:
TypeError: unsupported format string passed to MagicMock.__format__
The new notify_purchase_and_sale() call inside the crypto-watcher
finalization path runs `f"${invoice.total:.2f}"` to compose the
notification body. The double-spend tests pass a MagicMock as the
invoice — its .total is a MagicMock too — and the format-spec call
fails before _safe_add even runs.
Fix: wrap every public notify_* helper in a top-level
try/except-and-log. A notification persist failure must not
propagate up the payment-finalization stack (it didn't matter in
prod because real invoices format fine, but the unit-test mocks
exposed the contract gap).
All five helpers updated: notify_purchase_and_sale,
notify_auction_outbid, notify_auction_won,
notify_auction_ended_no_winner, notify_offer_expired. Inner
functions hold the actual logic; the outer wrapper is just the
swallow-and-log shield.
This also stops a transient DB or attribute error inside the
helper from rolling back the payment txn — same defense the
_safe_email wrapper provides for mail sends.
This commit is contained in:
parent
5f735ee1d3
commit
f9216befd6
1 changed files with 67 additions and 34 deletions
|
|
@ -63,10 +63,21 @@ def notify_purchase_and_sale(target, invoice):
|
|||
notification for each shop owner on this invoice. Idempotent
|
||||
per (recipient × invoice × kind) is NOT enforced — callers wire
|
||||
this in once per cart-completion path, same as the email sends.
|
||||
Accepts a request or a dbsession."""
|
||||
Accepts a request or a dbsession.
|
||||
|
||||
Non-fatal: any exception (incl. unit-test mocks with non-numeric
|
||||
totals) is logged and swallowed. A notification persist failure
|
||||
must not propagate up the payment-finalization stack.
|
||||
"""
|
||||
if invoice is None:
|
||||
return
|
||||
try:
|
||||
_notify_purchase_and_sale_inner(target, invoice)
|
||||
except Exception:
|
||||
log.exception("notify_purchase_and_sale failed (non-fatal)")
|
||||
|
||||
|
||||
def _notify_purchase_and_sale_inner(target, invoice):
|
||||
title_hint = _invoice_first_product_title(invoice)
|
||||
total_str = f"${invoice.total:.2f}"
|
||||
|
||||
|
|
@ -114,55 +125,70 @@ def notify_purchase_and_sale(target, invoice):
|
|||
|
||||
def notify_auction_outbid(target, prior_bidder, auction):
|
||||
"""Drop a notification for the previous winning bidder when a new
|
||||
higher bid lands. Mirrors the existing send_auction_outbid_email."""
|
||||
higher bid lands. Mirrors the existing send_auction_outbid_email.
|
||||
Non-fatal — failures (incl. mock-typed inputs from unit tests) are
|
||||
logged and swallowed."""
|
||||
if prior_bidder is None or auction is None:
|
||||
return
|
||||
title_hint = auction.product.title if auction.product else "auction"
|
||||
_safe_add(
|
||||
target,
|
||||
MpsNotification(
|
||||
user=prior_bidder,
|
||||
kind=NOTIFICATION_KIND_AUCTION_OUTBID,
|
||||
subject=f'You were outbid on "{title_hint}"',
|
||||
body=f'Current high: ${auction.current_high:.2f}. Click to bid again.',
|
||||
link_url=f"/a/{auction.uuid_str}",
|
||||
shop=auction.shop,
|
||||
auction=auction,
|
||||
),
|
||||
)
|
||||
try:
|
||||
title_hint = auction.product.title if auction.product else "auction"
|
||||
_safe_add(
|
||||
target,
|
||||
MpsNotification(
|
||||
user=prior_bidder,
|
||||
kind=NOTIFICATION_KIND_AUCTION_OUTBID,
|
||||
subject=f'You were outbid on "{title_hint}"',
|
||||
body=f'Current high: ${auction.current_high:.2f}. Click to bid again.',
|
||||
link_url=f"/a/{auction.uuid_str}",
|
||||
shop=auction.shop,
|
||||
auction=auction,
|
||||
),
|
||||
)
|
||||
except Exception:
|
||||
log.exception("notify_auction_outbid failed (non-fatal)")
|
||||
|
||||
|
||||
def notify_auction_won(target, auction):
|
||||
"""Drop a notification for the auction winner when the auction
|
||||
transitions ACTIVE → ENDED with a winner. Fired by auction_tick;
|
||||
the buyer needs the pay-by deadline before it lapses."""
|
||||
the buyer needs the pay-by deadline before it lapses. Non-fatal."""
|
||||
if auction is None or auction.winner is None:
|
||||
return
|
||||
title_hint = auction.product.title if auction.product else "auction"
|
||||
_safe_add(
|
||||
target,
|
||||
MpsNotification(
|
||||
user=auction.winner,
|
||||
kind=NOTIFICATION_KIND_AUCTION_WON,
|
||||
subject=f'You won the auction for "{title_hint}"',
|
||||
body=(
|
||||
f'Pay ${auction.current_high:.2f} to claim it. '
|
||||
"Auction will be released to the next-highest bidder if "
|
||||
"payment lapses."
|
||||
try:
|
||||
title_hint = auction.product.title if auction.product else "auction"
|
||||
_safe_add(
|
||||
target,
|
||||
MpsNotification(
|
||||
user=auction.winner,
|
||||
kind=NOTIFICATION_KIND_AUCTION_WON,
|
||||
subject=f'You won the auction for "{title_hint}"',
|
||||
body=(
|
||||
f'Pay ${auction.current_high:.2f} to claim it. '
|
||||
"Auction will be released to the next-highest bidder if "
|
||||
"payment lapses."
|
||||
),
|
||||
link_url=f"/a/{auction.uuid_str}",
|
||||
shop=auction.shop,
|
||||
auction=auction,
|
||||
),
|
||||
link_url=f"/a/{auction.uuid_str}",
|
||||
shop=auction.shop,
|
||||
auction=auction,
|
||||
),
|
||||
)
|
||||
)
|
||||
except Exception:
|
||||
log.exception("notify_auction_won failed (non-fatal)")
|
||||
|
||||
|
||||
def notify_auction_ended_no_winner(target, auction):
|
||||
"""Drop a notification for shop owners when an auction ends with
|
||||
no winner (no bids or reserve not met). Lets the seller decide
|
||||
whether to relist."""
|
||||
whether to relist. Non-fatal."""
|
||||
if auction is None or auction.shop is None:
|
||||
return
|
||||
try:
|
||||
_notify_auction_ended_no_winner_inner(target, auction)
|
||||
except Exception:
|
||||
log.exception("notify_auction_ended_no_winner failed (non-fatal)")
|
||||
|
||||
|
||||
def _notify_auction_ended_no_winner_inner(target, auction):
|
||||
title_hint = auction.product.title if auction.product else "auction"
|
||||
for owner in auction.shop.owners:
|
||||
if owner is None:
|
||||
|
|
@ -187,9 +213,16 @@ def notify_auction_ended_no_winner(target, auction):
|
|||
def notify_offer_expired(target, offer):
|
||||
"""Drop notifications when an offer auto-expires — both pre-accept
|
||||
(seller didn't respond in the negotiation window) and post-accept
|
||||
(buyer never paid). Both buyer and seller(s) get a row."""
|
||||
(buyer never paid). Both buyer and seller(s) get a row. Non-fatal."""
|
||||
if offer is None:
|
||||
return
|
||||
try:
|
||||
_notify_offer_expired_inner(target, offer)
|
||||
except Exception:
|
||||
log.exception("notify_offer_expired failed (non-fatal)")
|
||||
|
||||
|
||||
def _notify_offer_expired_inner(target, offer):
|
||||
title_hint = offer.product.title if offer.product else "offer"
|
||||
subject = f'Offer expired: "{title_hint}"'
|
||||
body = f'${offer.current_amount:.2f} offer auto-expired.'
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue