From 826b036724760837ba2b253fb674f155b1225e28 Mon Sep 17 00:00:00 2001 From: kavore <161734431+kavore@users.noreply.github.com> Date: Thu, 12 Feb 2026 09:27:44 +0300 Subject: [PATCH] fix(payments): enhance payment processing return values Refactor the `process_successful_payment` function to ensure consistent return values, replacing `return` statements with `return False` for error cases. This improves error handling and clarity in payment processing logic. Additionally, update the promo code consumption logic to handle missing discounts more gracefully, ensuring proper logging and flow control. --- bot/handlers/user/payment.py | 57 +++++++++++++++++------------- bot/services/promo_code_service.py | 31 ++++++++++------ 2 files changed, 52 insertions(+), 36 deletions(-) diff --git a/bot/handlers/user/payment.py b/bot/handlers/user/payment.py index 89b32d2..0dd571e 100644 --- a/bot/handlers/user/payment.py +++ b/bot/handlers/user/payment.py @@ -37,7 +37,7 @@ async def process_successful_payment(session: AsyncSession, bot: Bot, subscription_service: SubscriptionService, referral_service: ReferralService, yookassa_service: Optional[YooKassaService] = None, - lknpd_service: Optional[LknpdService] = None): + lknpd_service: Optional[LknpdService] = None) -> bool: metadata = payment_info_from_webhook.get("metadata", {}) user_id_str = metadata.get("user_id") subscription_months_str = metadata.get("subscription_months") @@ -56,7 +56,7 @@ async def process_successful_payment(session: AsyncSession, bot: Bot, logging.error( f"Missing crucial metadata for payment: {payment_info_from_webhook.get('id')}, metadata: {metadata}" ) - return + return False db_user = None try: @@ -69,7 +69,7 @@ async def process_successful_payment(session: AsyncSession, bot: Bot, payment_info_from_webhook.get("id"), payment_db_id_str, ) - return + return False payment_db_id = int(payment_db_id_str) is_auto_renew = bool(auto_renew_subscription_id_str and sale_mode != "traffic") promo_code_id = int( @@ -85,7 +85,7 @@ async def process_successful_payment(session: AsyncSession, bot: Bot, logging.error( f"Payment record {payment_db_id} not found for YK ID {yk_payment_id_from_hook}." ) - return + return False if payment_record.user_id != user_id: logging.error( "Payment ownership mismatch for payment %s: metadata user_id=%s, db user_id=%s", @@ -93,7 +93,7 @@ async def process_successful_payment(session: AsyncSession, bot: Bot, user_id, payment_record.user_id, ) - return + return False # Provider-backed verification (defense-in-depth): verify actual YooKassa payment state if yk_payment_id_from_hook and yookassa_service and yookassa_service.configured: @@ -103,7 +103,7 @@ async def process_successful_payment(session: AsyncSession, bot: Bot, "YooKassa webhook verification failed: payment %s not found via provider API", yk_payment_id_from_hook, ) - return + return False provider_status = str(provider_payment_info.get("status") or "") provider_paid = bool(provider_payment_info.get("paid")) @@ -114,7 +114,7 @@ async def process_successful_payment(session: AsyncSession, bot: Bot, provider_status, provider_paid, ) - return + return False provider_metadata_raw = provider_payment_info.get("metadata") or {} provider_metadata = provider_metadata_raw if isinstance(provider_metadata_raw, dict) else {} @@ -125,7 +125,7 @@ async def process_successful_payment(session: AsyncSession, bot: Bot, provider_metadata.get("user_id"), user_id, ) - return + return False provider_payment_db_id = str(provider_metadata.get("payment_db_id") or "").strip() if provider_payment_db_id != str(payment_db_id): logging.error( @@ -134,7 +134,7 @@ async def process_successful_payment(session: AsyncSession, bot: Bot, provider_metadata.get("payment_db_id"), payment_db_id, ) - return + return False try: provider_amount = float(provider_payment_info.get("amount_value") or 0.0) @@ -145,7 +145,7 @@ async def process_successful_payment(session: AsyncSession, bot: Bot, payment_value, provider_amount, ) - return + return False if payment_record and round(float(payment_record.amount), 2) != round(provider_amount, 2): logging.error( "YooKassa webhook verification failed: DB amount mismatch for payment %s (db %.2f vs provider %.2f)", @@ -153,7 +153,7 @@ async def process_successful_payment(session: AsyncSession, bot: Bot, float(payment_record.amount), provider_amount, ) - return + return False provider_currency = str(provider_payment_info.get("amount_currency") or "").upper() if payment_record and provider_currency and str(payment_record.currency or "").upper() != provider_currency: logging.error( @@ -162,20 +162,20 @@ async def process_successful_payment(session: AsyncSession, bot: Bot, payment_record.currency, provider_currency, ) - return + return False except Exception as e_amount_verify: logging.error( "YooKassa webhook verification failed for payment %s: cannot validate amount (%s)", yk_payment_id_from_hook, e_amount_verify, ) - return + return False if payment_record and payment_record.status == "succeeded": logging.info( f"Skipping duplicate YooKassa webhook for payment {payment_db_id} (YK: {yk_payment_id_from_hook})." ) - return + return True db_user = await user_dal.get_user_by_id(session, user_id) if not db_user: @@ -187,7 +187,7 @@ async def process_successful_payment(session: AsyncSession, bot: Bot, session, payment_db_id, "failed_user_not_found", payment_info_from_webhook.get("id")) - return + return False except (TypeError, ValueError) as e: logging.error( @@ -203,7 +203,7 @@ async def process_successful_payment(session: AsyncSession, bot: Bot, logging.error( f"Failed to update payment status after metadata error: {e_upd}" ) - return + return False try: yk_payment_id_from_hook = payment_info_from_webhook.get("id") @@ -225,7 +225,7 @@ async def process_successful_payment(session: AsyncSession, bot: Bot, yk_payment_id_from_hook, payment_db_id, ) - return + return True marked = await payment_dal.mark_provider_payment_succeeded_once( session, @@ -237,7 +237,7 @@ async def process_successful_payment(session: AsyncSession, bot: Bot, "YooKassa webhook: payment %s already processed atomically", payment_db_id, ) - return + return True should_send_lknpd_receipt = bool( lknpd_service @@ -270,7 +270,7 @@ async def process_successful_payment(session: AsyncSession, bot: Bot, else: display_last4 = None else: - # Wallets, SBP, etc. — use provided title/type; no last4 + # Wallets, SBP, etc. - use provided title/type; no last4 display_network = title or (pm_type.upper() if pm_type else "Payment method") display_last4 = None @@ -379,7 +379,7 @@ async def process_successful_payment(session: AsyncSession, bot: Bot, details_message = _( "payment_successful_traffic_full", traffic_gb=traffic_label, - end_date=final_end_date_for_user.strftime('%Y-%m-%d') if final_end_date_for_user else "—", + end_date=final_end_date_for_user.strftime('%Y-%m-%d') if final_end_date_for_user else "-", config_link=config_link_text, ) details_markup = get_connect_and_main_keyboard( @@ -470,6 +470,8 @@ async def process_successful_payment(session: AsyncSession, bot: Bot, except Exception as e: logging.error(f"Failed to send payment notification: {e}") + return True + except Exception as e_process: logging.error( f"Error during process_successful_payment main try block for user {user_id}: {e_process}", @@ -568,7 +570,7 @@ async def yookassa_webhook_route(request: web.Request): logging.error( f"YooKassa webhook payment {payment_data_from_notification.id} lacks metadata. Cannot process." ) - return web.Response(status=200, text="ok_error_no_metadata") + return web.Response(status=503, text="yookassa_missing_metadata") # Safely extract payment_method details (SDK objects may not have to_dict) pm_obj = getattr(payment_data_from_notification, 'payment_method', None) @@ -636,12 +638,15 @@ async def yookassa_webhook_route(request: web.Request): if payment_dict_for_processing.get( "paid") and payment_dict_for_processing.get( "status") == "succeeded": - await process_successful_payment( + processed = await process_successful_payment( session, bot, payment_dict_for_processing, i18n_instance, settings, panel_service, subscription_service, referral_service, yookassa_service, lknpd_service) + if not processed: + await session.rollback() + return web.Response(status=503, text="yookassa_processing_failed_retry") await session.commit() else: logging.warning( @@ -649,6 +654,8 @@ async def yookassa_webhook_route(request: web.Request): f"but data not as expected: status='{payment_dict_for_processing.get('status')}', " f"paid='{payment_dict_for_processing.get('paid')}'" ) + await session.rollback() + return web.Response(status=503, text="yookassa_invalid_succeeded_payload") elif notification_object.event == YOOKASSA_EVENT_PAYMENT_CANCELED: await process_cancelled_payment( session, bot, payment_dict_for_processing, @@ -743,7 +750,7 @@ async def yookassa_webhook_route(request: web.Request): f"for YK Payment ID {payment_dict_for_processing.get('id')} in DB transaction: {e_webhook_db_processing}", exc_info=True) return web.Response( - status=200, text="ok_internal_processing_error_logged") + status=503, text="yookassa_processing_error_retry") return web.Response(status=200, text="ok") @@ -754,5 +761,5 @@ async def yookassa_webhook_route(request: web.Request): logging.error( f"YooKassa Webhook general processing error: {e_general_webhook}", exc_info=True) - return web.Response(status=200, - text="ok_general_internal_error_logged") + return web.Response(status=503, + text="yookassa_general_error_retry") diff --git a/bot/services/promo_code_service.py b/bot/services/promo_code_service.py index 1e58ccd..99e42f8 100644 --- a/bot/services/promo_code_service.py +++ b/bot/services/promo_code_service.py @@ -407,20 +407,26 @@ class PromoCodeService: user_id, include_expired=True, ) - if active_discount and active_discount.promo_code_id != promo_code_id: + now_utc = datetime.now(timezone.utc) + + if not active_discount: logging.info( - "Active discount promo %s differs from payment promo %s; leaving active discount intact.", + "Discount reservation missing at consumption time (user=%s, promo=%s, payment=%s)", + user_id, + promo_code_id, + payment_id, + ) + return False + + if active_discount.promo_code_id != promo_code_id: + logging.info( + "Active discount promo %s differs from payment promo %s; skipping consumption.", active_discount.promo_code_id, promo_code_id, ) - active_discount = None + return False - now_utc = datetime.now(timezone.utc) - if ( - active_discount - and active_discount.promo_code_id == promo_code_id - and active_discount.expires_at <= now_utc - ): + if active_discount.expires_at <= now_utc: logging.info( "Discount reservation expired before payment consumption (user=%s, promo=%s)", user_id, @@ -466,8 +472,11 @@ class PromoCodeService: ) return False - if active_discount and active_discount.promo_code_id == promo_code_id: - await active_discount_dal.clear_active_discount(session, user_id) + await active_discount_dal.clear_active_discount_if_matches( + session, + user_id=user_id, + promo_code_id=promo_code_id, + ) await session.flush() logging.info(