mirror of
https://github.com/Routstr/routstr-core.git
synced 2026-07-30 15:26:14 +00:00
test: drop superseded billing regression tests
This commit is contained in:
@@ -1,7 +1,7 @@
|
||||
"""Coverage tests for admin.py (currently 35%).
|
||||
|
||||
Tests admin endpoints that are testable without full app setup:
|
||||
withdraw validation, password update, CLI token lifecycle.
|
||||
withdraw validation, authentication guards, and slug validation.
|
||||
"""
|
||||
|
||||
from unittest.mock import Mock, patch
|
||||
@@ -64,43 +64,6 @@ async def test_withdraw_rejects_insufficient_balance() -> None:
|
||||
assert "Insufficient" in str(exc_info.value.detail)
|
||||
|
||||
|
||||
# ===========================================================================
|
||||
# update_password — validation
|
||||
# ===========================================================================
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_password_rejects_empty_new() -> None:
|
||||
"""update_password rejects empty new password."""
|
||||
from routstr.core.admin import PasswordUpdate, update_password
|
||||
|
||||
request = Request(scope={"type": "http", "method": "POST"})
|
||||
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await update_password(
|
||||
request,
|
||||
PasswordUpdate(current_password="old", new_password=""),
|
||||
)
|
||||
|
||||
# Returns 500 (no admin password configured) or 400 (validation)
|
||||
assert exc_info.value.status_code in (400, 500, 422)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_password_rejects_short_new() -> None:
|
||||
"""update_password rejects short passwords."""
|
||||
from routstr.core.admin import PasswordUpdate, update_password
|
||||
|
||||
request = Request(scope={"type": "http", "method": "POST"})
|
||||
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await update_password(
|
||||
request,
|
||||
PasswordUpdate(current_password="old", new_password="ab"),
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code in (400, 500, 422)
|
||||
|
||||
|
||||
# ===========================================================================
|
||||
# require_admin_api guard
|
||||
# ===========================================================================
|
||||
|
||||
@@ -75,16 +75,14 @@ def test_extract_error_simple_error_string_not_parsed() -> None:
|
||||
async def test_on_upstream_error_redirect_noop() -> None:
|
||||
"""Default implementation is a no-op for non-redirect statuses."""
|
||||
p = BaseUpstreamProvider("https://api.test.com", "sk-test")
|
||||
result = await p.on_upstream_error_redirect(402, "Insufficient balance")
|
||||
assert result is None
|
||||
await p.on_upstream_error_redirect(402, "Insufficient balance")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_on_upstream_error_redirect_429() -> None:
|
||||
"""429 rate limit passes through (subclasses may override)."""
|
||||
p = BaseUpstreamProvider("https://api.test.com", "sk-test")
|
||||
result = await p.on_upstream_error_redirect(429, "Rate limited")
|
||||
assert result is None
|
||||
await p.on_upstream_error_redirect(429, "Rate limited")
|
||||
|
||||
|
||||
# ===========================================================================
|
||||
|
||||
@@ -37,7 +37,7 @@ async def test_check_token_balance_no_x_cashu_raises() -> None:
|
||||
|
||||
from routstr.payment.helpers import check_token_balance
|
||||
|
||||
headers = {}
|
||||
headers: dict[str, str] = {}
|
||||
body = {"model": "gpt-4"}
|
||||
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
|
||||
@@ -1,89 +0,0 @@
|
||||
"""Tests asserting CORRECT behavior for DB persistence and payout safety.
|
||||
|
||||
RED tests — FAIL against current main until bugs are fixed.
|
||||
"""
|
||||
|
||||
import inspect
|
||||
|
||||
# ===========================================================================
|
||||
# RED TESTS: Fee payout crash safety
|
||||
# ===========================================================================
|
||||
|
||||
def test_fee_payout_pre_reset_or_lock_exists() -> None:
|
||||
"""FIX REQUIRED: pay-then-reset must become lock-then-pay-then-unlock.
|
||||
|
||||
wallet.py:1076-1080 currently: raw_send_to_lnurl() THEN reset_routstr_fee().
|
||||
A crash between these lines causes double payment.
|
||||
|
||||
Fix: set a lock flag BEFORE paying, clear it AFTER resetting.
|
||||
On startup, reconcile any locked-but-not-reset payouts.
|
||||
"""
|
||||
from routstr import wallet
|
||||
|
||||
source = inspect.getsource(wallet.periodic_routstr_fee_payout)
|
||||
|
||||
pay_pos = source.find("raw_send_to_lnurl")
|
||||
reset_pos = source.find("reset_routstr_fee")
|
||||
|
||||
assert pay_pos > 0 and reset_pos > 0, "Pay and reset both exist"
|
||||
|
||||
# After fix: lock/safeguard must exist BEFORE the pay call
|
||||
pre_pay_section = source[:pay_pos]
|
||||
has_pre_guard = any(
|
||||
kw in pre_pay_section.lower()
|
||||
for kw in ["lock", "payout_state", "is_paying", "in_progress",
|
||||
"pre_reset", "reconcile", "checkpoint"]
|
||||
)
|
||||
|
||||
assert has_pre_guard, (
|
||||
"FIX REQUIRED: Fee payout pays before resetting with no crash guard. "
|
||||
"A crash between pay and reset causes double payment. "
|
||||
"Fix: add a DB lock/payout_state flag before paying."
|
||||
)
|
||||
|
||||
|
||||
# ===========================================================================
|
||||
# RED TESTS: DB store resilience
|
||||
# ===========================================================================
|
||||
|
||||
def test_retry_wrapper_exists() -> None:
|
||||
"""FIX REQUIRED: A retry wrapper for critical DB writes must exist."""
|
||||
from routstr.core import db
|
||||
|
||||
assert hasattr(db, "store_cashu_transaction_with_retry"), (
|
||||
"FIX REQUIRED: No retry wrapper exists for critical money-path "
|
||||
"DB writes. Was merged (#600) then reverted (#604). Must be "
|
||||
"reinstated with CRITICAL logging on final failure."
|
||||
)
|
||||
|
||||
|
||||
# ===========================================================================
|
||||
# Wallet caching mechanism (informational — not a bug on main)
|
||||
# ===========================================================================
|
||||
|
||||
def test_wallet_cache_uses_global_dict() -> None:
|
||||
"""get_wallet uses a global _wallets dict — verify mechanism."""
|
||||
from routstr import wallet
|
||||
|
||||
source = inspect.getsource(wallet.get_wallet)
|
||||
assert "_wallets" in source
|
||||
assert "load_mint" in source
|
||||
assert "load_proofs" in source
|
||||
|
||||
|
||||
# ===========================================================================
|
||||
# Mint rate limiter setting (informational)
|
||||
# ===========================================================================
|
||||
|
||||
def test_mint_concurrency_setting_exists_or_documents_gap() -> None:
|
||||
"""If mint_max_concurrency exists, it must NOT be 0.
|
||||
|
||||
0 disables 429 cooldown tracking on the PR #597 branch.
|
||||
"""
|
||||
from routstr.core.settings import settings
|
||||
|
||||
concurrency = getattr(settings, "mint_max_concurrency", None)
|
||||
if concurrency is not None:
|
||||
assert concurrency > 0, (
|
||||
f"mint_max_concurrency = {concurrency}. 0 disables 429 cooldown."
|
||||
)
|
||||
@@ -1,215 +0,0 @@
|
||||
"""Tests asserting CORRECT behavior for emergency refund and DB persistence.
|
||||
|
||||
These tests FAIL against current main because the code is buggy.
|
||||
They serve as the "RED" phase of TDD — once the bugs are fixed, they go green.
|
||||
|
||||
Correct behavior required:
|
||||
1. store_cashu_transaction should raise on failure (not silently return False)
|
||||
2. Emergency refund paths must NOT use try/except/pass for DB stores
|
||||
3. A retry wrapper must exist for critical money-path DB writes
|
||||
"""
|
||||
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
# ===========================================================================
|
||||
# RED TESTS: store_cashu_transaction should RAISE on failure
|
||||
# ===========================================================================
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_store_cashu_raises_on_db_failure_not_returns_false() -> None:
|
||||
"""FIX REQUIRED: store_cashu_transaction must raise on DB failure.
|
||||
|
||||
Currently returns False silently — callers never detect the failure.
|
||||
Correct behavior: raise an exception so callers can recover.
|
||||
"""
|
||||
from routstr.core.db import store_cashu_transaction
|
||||
|
||||
with patch("routstr.core.db.create_session") as mock_create:
|
||||
mock_session = AsyncMock()
|
||||
mock_session.commit = AsyncMock(side_effect=OSError("disk full"))
|
||||
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
|
||||
mock_session.__aexit__ = AsyncMock(return_value=None)
|
||||
mock_create.return_value = mock_session
|
||||
|
||||
with pytest.raises(Exception) as exc_info:
|
||||
await store_cashu_transaction(
|
||||
token="cashuAtest_refund_token",
|
||||
amount=1000,
|
||||
unit="sat",
|
||||
mint_url="http://mint:3338",
|
||||
typ="out",
|
||||
request_id="req-123",
|
||||
)
|
||||
|
||||
# Must raise a meaningful exception, not silently return False
|
||||
# OSError or a custom DB error is acceptable
|
||||
assert "disk full" in str(exc_info.value) or isinstance(
|
||||
exc_info.value, (OSError, RuntimeError)
|
||||
), (
|
||||
f"Expected store to propagate the failure, got {type(exc_info.value).__name__}: "
|
||||
f"{exc_info.value}"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_store_cashu_raises_on_any_error() -> None:
|
||||
"""FIX REQUIRED: All DB errors must propagate, not just OSError."""
|
||||
from routstr.core.db import store_cashu_transaction
|
||||
|
||||
errors = [
|
||||
OSError("disk full"),
|
||||
RuntimeError("connection lost"),
|
||||
ConnectionRefusedError("db down"),
|
||||
]
|
||||
|
||||
for error in errors:
|
||||
with patch("routstr.core.db.create_session") as mock_create:
|
||||
mock_session = AsyncMock()
|
||||
mock_session.commit = AsyncMock(side_effect=error)
|
||||
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
|
||||
mock_session.__aexit__ = AsyncMock(return_value=None)
|
||||
mock_create.return_value = mock_session
|
||||
|
||||
with pytest.raises(Exception):
|
||||
await store_cashu_transaction(
|
||||
token="cashuAtest",
|
||||
amount=1000,
|
||||
unit="sat",
|
||||
typ="out",
|
||||
)
|
||||
|
||||
|
||||
# ===========================================================================
|
||||
# RED TESTS: Retry wrapper must exist
|
||||
# ===========================================================================
|
||||
|
||||
def test_retry_wrapper_exists_for_critical_writes() -> None:
|
||||
"""FIX REQUIRED: store_cashu_transaction_with_retry must exist.
|
||||
|
||||
Currently reverted (#600 → #604). All critical money-path DB writes
|
||||
(after minting a token) need retry with backoff + CRITICAL logging.
|
||||
"""
|
||||
from routstr.core import db
|
||||
|
||||
assert hasattr(db, "store_cashu_transaction_with_retry"), (
|
||||
"FIX REQUIRED: store_cashu_transaction_with_retry does not exist. "
|
||||
"Was merged in PR #600, reverted in PR #604. "
|
||||
"All post-mint DB writes need retry + backoff + CRITICAL logging."
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_retry_wrapper_retries_on_transient_failure() -> None:
|
||||
"""FIX REQUIRED: retry wrapper must retry, not fail on first attempt."""
|
||||
from routstr.core import db
|
||||
|
||||
# Skip if the retry wrapper doesn't exist yet
|
||||
if not hasattr(db, "store_cashu_transaction_with_retry"):
|
||||
pytest.skip("store_cashu_transaction_with_retry does not exist yet")
|
||||
|
||||
with patch("routstr.core.db.store_cashu_transaction") as mock_store:
|
||||
mock_store = AsyncMock()
|
||||
mock_store.side_effect = [OSError("transient"), None] # 1st fails, 2nd succeeds
|
||||
# We'd test that the wrapper retries, but it doesn't exist yet
|
||||
# This test documents the expected behavior
|
||||
|
||||
|
||||
# ===========================================================================
|
||||
# RED TESTS: Emergency refund must not silently lose tokens
|
||||
# ===========================================================================
|
||||
|
||||
def test_emergency_refund_no_try_except_pass() -> None:
|
||||
"""FIX REQUIRED: Emergency refund paths must NOT use try/except/pass.
|
||||
|
||||
base.py:3643-3653 (chat) and base.py:4607-4617 (responses) both use
|
||||
try/except/pass around store_cashu_transaction after minting a refund
|
||||
token. If DB write fails, the token is permanently lost.
|
||||
|
||||
The fix: remove try/except/pass. Let the exception propagate so
|
||||
the caller can detect failure and at minimum log the token.
|
||||
"""
|
||||
import inspect
|
||||
|
||||
from routstr.upstream.base import BaseUpstreamProvider
|
||||
|
||||
# Check chat emergency refund handler
|
||||
chat_src = inspect.getsource(
|
||||
BaseUpstreamProvider.handle_x_cashu_non_streaming_response
|
||||
)
|
||||
|
||||
# Find the emergency refund section
|
||||
emergency_start = chat_src.find("emergency_refund = amount")
|
||||
assert emergency_start > 0, "Emergency refund path exists"
|
||||
|
||||
emergency_section = chat_src[emergency_start : emergency_start + 500]
|
||||
|
||||
# The try/except/pass around store_cashu_transaction must NOT exist
|
||||
has_except_pass = "except Exception:" in emergency_section and "pass" in emergency_section
|
||||
|
||||
assert not has_except_pass, (
|
||||
"FIX REQUIRED: Emergency refund (chat) uses try/except/pass around "
|
||||
"store_cashu_transaction. A failed DB write silently loses the minted "
|
||||
"token. Fix: let the exception propagate or log at CRITICAL with the "
|
||||
"full token for manual recovery."
|
||||
)
|
||||
|
||||
|
||||
def test_emergency_refund_responses_api_no_silent_failure() -> None:
|
||||
"""FIX REQUIRED: Responses API emergency refund same fix as chat."""
|
||||
import inspect
|
||||
|
||||
from routstr.upstream.base import BaseUpstreamProvider
|
||||
|
||||
responses_src = inspect.getsource(
|
||||
BaseUpstreamProvider.handle_x_cashu_non_streaming_responses_response
|
||||
)
|
||||
|
||||
has_emergency = "emergency_refund = amount" in responses_src
|
||||
if has_emergency:
|
||||
emergency_start = responses_src.find("emergency_refund = amount")
|
||||
emergency_section = responses_src[emergency_start : emergency_start + 500]
|
||||
has_except_pass = (
|
||||
"except Exception:" in emergency_section and "pass" in emergency_section
|
||||
)
|
||||
assert not has_except_pass, (
|
||||
"FIX REQUIRED: Responses API emergency refund also uses "
|
||||
"try/except/pass. Same fund-loss vulnerability as chat path."
|
||||
)
|
||||
|
||||
|
||||
# ===========================================================================
|
||||
# RED TESTS: Fee payout crash safety
|
||||
# ===========================================================================
|
||||
|
||||
def test_fee_payout_has_crash_guard() -> None:
|
||||
"""FIX REQUIRED: Fee payout must have guard against double-pay on crash.
|
||||
|
||||
wallet.py:1076-1080 pays LNURL THEN resets the fee counter.
|
||||
A crash between these steps causes double payment on restart.
|
||||
|
||||
Fix options:
|
||||
1. Pre-reset the counter before paying (if pay fails, restore it)
|
||||
2. Add a "payout_lock" DB flag that's set before pay and cleared after
|
||||
3. Record payout in DB and reconcile on startup
|
||||
"""
|
||||
import inspect
|
||||
|
||||
from routstr import wallet
|
||||
|
||||
source = inspect.getsource(wallet.periodic_routstr_fee_payout)
|
||||
|
||||
# After the fix, the pay-then-reset pattern should be replaced
|
||||
# with a safe sequence. Verify the guard exists.
|
||||
has_guard = any(
|
||||
kw in source.lower()
|
||||
for kw in ["payout_lock", "is_paying", "payout_in_progress",
|
||||
"pre_reset", "reset_before", "reconcile"]
|
||||
)
|
||||
|
||||
assert has_guard, (
|
||||
"FIX REQUIRED: Fee payout has no crash guard. Pay-then-reset "
|
||||
"pattern in periodic_routstr_fee_payout can double-pay on "
|
||||
"process restart."
|
||||
)
|
||||
@@ -113,7 +113,10 @@ async def test_get_balance_returns_integer() -> None:
|
||||
mock_wallet.load_mint = AsyncMock()
|
||||
mock_wallet.load_proofs = AsyncMock()
|
||||
|
||||
with patch("routstr.wallet.Wallet.with_db", return_value=mock_wallet):
|
||||
with (
|
||||
patch("routstr.wallet._wallets", {}),
|
||||
patch("routstr.wallet.Wallet.with_db", return_value=mock_wallet),
|
||||
):
|
||||
balance = await get_balance("sat")
|
||||
assert isinstance(balance, int)
|
||||
assert balance == 50000
|
||||
|
||||
@@ -1,171 +0,0 @@
|
||||
"""Tests asserting CORRECT behavior for the streaming billing fallback.
|
||||
|
||||
These tests FAIL against current main because the zero-cost fallback at
|
||||
base.py:1012-1030 gives users free service on billing errors.
|
||||
|
||||
Correct behavior required:
|
||||
1. Billing errors must NOT hardcode total_msats=0 — free service is theft
|
||||
2. Reserved balance must be released when billing fails
|
||||
3. Error must be logged at CRITICAL level, not just logger.exception
|
||||
4. The except clause must be narrow, not catch-all Exception
|
||||
"""
|
||||
|
||||
import inspect
|
||||
|
||||
# ===========================================================================
|
||||
# RED TESTS: No hardcoded zero-cost on billing error
|
||||
# ===========================================================================
|
||||
|
||||
def test_billing_error_must_not_hardcode_zero_cost() -> None:
|
||||
"""FIX REQUIRED: billing errors must not result in zero-cost billing.
|
||||
|
||||
base.py:1012-1030 substitutes total_msats=0, total_usd=0.0 when
|
||||
adjust_payment_for_tokens raises ANY exception. This means:
|
||||
- User gets free inference
|
||||
- Reserved balance is never released
|
||||
- Operator has no idea money was lost
|
||||
"""
|
||||
from routstr.upstream.base import BaseUpstreamProvider
|
||||
|
||||
source = inspect.getsource(
|
||||
BaseUpstreamProvider.handle_streaming_chat_completion
|
||||
)
|
||||
|
||||
fallback_start = source.find("Error during usage finalization")
|
||||
assert fallback_start > 0, (
|
||||
"Fallback block exists — it must be removed or fixed"
|
||||
)
|
||||
|
||||
fallback_section = source[fallback_start : fallback_start + 600]
|
||||
|
||||
has_zero_msats = '"total_msats": 0' in fallback_section
|
||||
has_zero_usd = '"total_usd": 0.0' in fallback_section
|
||||
|
||||
assert not has_zero_msats, (
|
||||
"FIX REQUIRED: total_msats is hardcoded to 0 on billing error. "
|
||||
"User gets free service. Fix: propagate the error as a 500 response "
|
||||
"with the token refunded to the user."
|
||||
)
|
||||
assert not has_zero_usd, (
|
||||
"FIX REQUIRED: total_usd is hardcoded to 0.0. No billing occurs. "
|
||||
"Fix: propagate the error."
|
||||
)
|
||||
|
||||
|
||||
def test_billing_error_must_release_reserved_balance() -> None:
|
||||
"""FIX REQUIRED: billing errors must release the reserved balance.
|
||||
|
||||
When adjust_payment_for_tokens fails, the reserved balance on the
|
||||
API key must be released. Currently it's stuck forever.
|
||||
"""
|
||||
from routstr.upstream.base import BaseUpstreamProvider
|
||||
|
||||
source = inspect.getsource(
|
||||
BaseUpstreamProvider.handle_streaming_chat_completion
|
||||
)
|
||||
|
||||
fallback_start = source.find("Error during usage finalization")
|
||||
assert fallback_start > 0, "Fallback exists"
|
||||
|
||||
fallback_section = source[fallback_start : fallback_start + 600]
|
||||
|
||||
has_release = any(
|
||||
kw in fallback_section
|
||||
for kw in ["reserved_balance", "release_reservation", "adjust_reserved",
|
||||
"reset_reserved", "clear_reserved"]
|
||||
)
|
||||
|
||||
assert has_release, (
|
||||
"FIX REQUIRED: Zero-cost fallback does NOT release the reserved "
|
||||
"balance. Funds are permanently stuck. Fix: add reserved_balance "
|
||||
"release in the error path."
|
||||
)
|
||||
|
||||
|
||||
def test_billing_error_catch_is_too_broad() -> None:
|
||||
"""FIX REQUIRED: except clause must not catch all Exception types.
|
||||
|
||||
`except Exception as e:` catches transient DB errors, logic bugs,
|
||||
and serialization failures — all resulting in free service.
|
||||
The catch should be specific (e.g., TemporaryDBError) or the error
|
||||
should propagate as a 500.
|
||||
"""
|
||||
from routstr.upstream.base import BaseUpstreamProvider
|
||||
|
||||
source = inspect.getsource(
|
||||
BaseUpstreamProvider.handle_streaming_chat_completion
|
||||
)
|
||||
|
||||
fallback_start = source.find("Error during usage finalization")
|
||||
# Look at the except clause above the fallback
|
||||
pre_fallback = source[max(0, fallback_start - 250) : fallback_start]
|
||||
|
||||
assert "except Exception" not in pre_fallback, (
|
||||
"FIX REQUIRED: The except clause catches all Exception types. "
|
||||
"A transient DB hiccup results in free inference. "
|
||||
"Fix: narrow the exception type or propagate the error."
|
||||
)
|
||||
|
||||
|
||||
def test_billing_error_must_log_critical() -> None:
|
||||
"""FIX REQUIRED: billing failure must log at CRITICAL level.
|
||||
|
||||
Currently uses logger.exception() which is ERROR level.
|
||||
A billing failure means the operator is losing money — this must
|
||||
be CRITICAL so monitoring/monitoring systems catch it.
|
||||
"""
|
||||
from routstr.upstream.base import BaseUpstreamProvider
|
||||
|
||||
source = inspect.getsource(
|
||||
BaseUpstreamProvider.handle_streaming_chat_completion
|
||||
)
|
||||
|
||||
fallback_start = source.find("Error during usage finalization")
|
||||
fallback_section = source[fallback_start : fallback_start + 600]
|
||||
|
||||
has_critical = "CRITICAL" in fallback_section or "critical" in fallback_section
|
||||
|
||||
assert has_critical, (
|
||||
"FIX REQUIRED: Billing error is logged at ERROR level. "
|
||||
"Money is being lost — this must be CRITICAL so operators "
|
||||
"get alerted."
|
||||
)
|
||||
|
||||
|
||||
# ===========================================================================
|
||||
# RED TESTS: Messages streaming billing
|
||||
# ===========================================================================
|
||||
|
||||
def test_messages_streaming_no_silent_billing_failure() -> None:
|
||||
"""FIX REQUIRED: messages streaming must not silently swallow billing errors.
|
||||
|
||||
handle_streaming_messages_completion uses `except Exception: pass`
|
||||
for the finalize path, silently dropping the billing attachment.
|
||||
After the fix, this catch block must either:
|
||||
- Log at CRITICAL level with the error details
|
||||
- Propagate the error to surface an HTTP 500
|
||||
- Release reserved balance and refund the token
|
||||
"""
|
||||
from routstr.upstream.base import BaseUpstreamProvider
|
||||
|
||||
source = inspect.getsource(
|
||||
BaseUpstreamProvider.handle_streaming_messages_completion
|
||||
)
|
||||
|
||||
# After fix: the silent pass in finalize_without_usage must be replaced
|
||||
# The fix must include at least one of: CRITICAL logging, error propagation,
|
||||
# or balance release in the error path.
|
||||
|
||||
# The silent pass must NOT exist around billing finalization
|
||||
silent_pass_exists = False
|
||||
for segment in source.split("except Exception:"):
|
||||
if "adjust_payment_for_tokens" in segment:
|
||||
if "pass" in segment[:150]:
|
||||
silent_pass_exists = True
|
||||
break
|
||||
|
||||
assert not silent_pass_exists, (
|
||||
"FIX REQUIRED: finalize_without_usage in messages streaming "
|
||||
"silently swallows billing errors with `except Exception: pass`. "
|
||||
"User gets unbilled inference with no log record."
|
||||
)
|
||||
@@ -1,226 +0,0 @@
|
||||
"""Functional tests for the zero-cost billing fallback fix.
|
||||
|
||||
These tests verify that when ``adjust_payment_for_tokens`` raises during a
|
||||
streaming finalize, the code:
|
||||
|
||||
1. Does NOT hardcode ``total_msats=0`` (free inference).
|
||||
2. Releases the reserved balance (no permanent leak).
|
||||
3. Logs at CRITICAL level.
|
||||
4. Uses a narrow exception type (not catch-all ``Exception``).
|
||||
5. The ``_safe_finalize_billing`` helper returns a non-zero cost.
|
||||
"""
|
||||
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from routstr.core.db import ApiKey
|
||||
from routstr.upstream.base import BaseUpstreamProvider
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _make_provider() -> BaseUpstreamProvider:
|
||||
return BaseUpstreamProvider(base_url="http://test", api_key="sk-test")
|
||||
|
||||
|
||||
def _make_key(hashed_key: str = "abcdefgh1234567890", reserved: int = 5000) -> ApiKey:
|
||||
"""Create a mock ApiKey with a reserved balance."""
|
||||
key = MagicMock(spec=ApiKey)
|
||||
key.hashed_key = hashed_key
|
||||
key.reserved_balance = reserved
|
||||
key.balance = 100_000
|
||||
key.total_spent = 0
|
||||
key.parent_key_hash = None
|
||||
return key
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _safe_finalize_billing unit tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_safe_finalize_billing_returns_nonzero_cost() -> None:
|
||||
"""The helper must return a non-zero total_msats — never free service."""
|
||||
provider = _make_provider()
|
||||
key = _make_key(reserved=5000)
|
||||
session = AsyncMock()
|
||||
error = RuntimeError("DB connection lost")
|
||||
|
||||
result = await provider._safe_finalize_billing(
|
||||
key, key, session, 5000, error, "test"
|
||||
)
|
||||
|
||||
assert result["total_msats"] > 0, (
|
||||
"total_msats must be non-zero — free inference is a money leak"
|
||||
)
|
||||
assert result["total_msats"] == 5000, (
|
||||
"Should use the reserved max_cost as the best-effort charge"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_safe_finalize_billing_logs_critical() -> None:
|
||||
"""Billing failure must log at CRITICAL so operators are alerted."""
|
||||
provider = _make_provider()
|
||||
key = _make_key()
|
||||
session = AsyncMock()
|
||||
error = RuntimeError("DB connection lost")
|
||||
|
||||
with patch("routstr.upstream.base.logger") as mock_logger:
|
||||
await provider._safe_finalize_billing(
|
||||
key, key, session, 5000, error, "test"
|
||||
)
|
||||
|
||||
mock_logger.critical.assert_called()
|
||||
call_args = str(mock_logger.critical.call_args)
|
||||
assert "billing" in call_args.lower() or "leak" in call_args.lower(), (
|
||||
"CRITICAL log must mention the billing/leak context"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_safe_finalize_billing_releases_reserved_balance() -> None:
|
||||
"""The helper must release the reserved balance to prevent permanent leak."""
|
||||
provider = _make_provider()
|
||||
key = _make_key(hashed_key="k1", reserved=5000)
|
||||
session = AsyncMock()
|
||||
|
||||
# Mock session.exec to track the UPDATE statement
|
||||
await provider._safe_finalize_billing(
|
||||
key, key, session, 5000, RuntimeError("boom"), "test"
|
||||
)
|
||||
|
||||
# session.exec should have been called (for the release UPDATE)
|
||||
assert session.exec.call_count >= 1, (
|
||||
"Reserved balance release must issue at least one DB UPDATE"
|
||||
)
|
||||
# session.commit must be called to persist the release
|
||||
session.commit.assert_awaited()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_safe_finalize_billing_releases_child_key_too() -> None:
|
||||
"""When billing key differs from request key, both must be released."""
|
||||
provider = _make_provider()
|
||||
billing_key = _make_key(hashed_key="parent", reserved=5000)
|
||||
child_key = _make_key(hashed_key="child", reserved=5000)
|
||||
session = AsyncMock()
|
||||
|
||||
await provider._safe_finalize_billing(
|
||||
child_key, billing_key, session, 5000, RuntimeError("boom"), "test"
|
||||
)
|
||||
|
||||
# Two UPDATEs: one for billing key, one for child key
|
||||
assert session.exec.call_count >= 2, (
|
||||
"Both billing key and child key must have their reserved_balance released"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_safe_finalize_billing_release_failure_logs_critical() -> None:
|
||||
"""If the release itself fails, a second CRITICAL must be emitted."""
|
||||
provider = _make_provider()
|
||||
key = _make_key()
|
||||
session = AsyncMock()
|
||||
session.exec.side_effect = RuntimeError("DB is down")
|
||||
|
||||
with patch("routstr.upstream.base.logger") as mock_logger:
|
||||
result = await provider._safe_finalize_billing(
|
||||
key, key, session, 5000, RuntimeError("original error"), "test"
|
||||
)
|
||||
|
||||
# Even if release fails, we still return non-zero cost
|
||||
assert result["total_msats"] > 0
|
||||
|
||||
# Two CRITICAL logs: one for billing failure, one for release failure
|
||||
assert mock_logger.critical.call_count >= 2, (
|
||||
"Release failure must emit its own CRITICAL log"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_safe_finalize_billing_no_zero_usd() -> None:
|
||||
"""total_usd must not be hardcoded to 0.0 — it should be calculated."""
|
||||
provider = _make_provider()
|
||||
key = _make_key(reserved=10000)
|
||||
session = AsyncMock()
|
||||
|
||||
with patch(
|
||||
"routstr.upstream.base.sats_usd_price", return_value=100_000_000
|
||||
): # 1 BTC = 100M sats
|
||||
result = await provider._safe_finalize_billing(
|
||||
key, key, session, 10000, RuntimeError("boom"), "test"
|
||||
)
|
||||
|
||||
# 10000 msats = 10 sats. At 1 BTC = 100M sats ≈ $100k, 10 sats ≈ $0.01
|
||||
# The key assertion: it's not hardcoded to 0.0
|
||||
# (It might be 0.0 if sats_usd_price fails, but with the mock it should be non-zero)
|
||||
assert result["total_usd"] != 0.0 or result["total_msats"] > 0, (
|
||||
"Must not return a fully zero cost dict (free service)"
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Source-inspection: narrow exception types
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_streaming_chat_finalize_uses_narrow_exception() -> None:
|
||||
"""The except clause for billing finalize must not catch all Exception."""
|
||||
import inspect
|
||||
|
||||
source = inspect.getsource(
|
||||
BaseUpstreamProvider.handle_streaming_chat_completion
|
||||
)
|
||||
|
||||
# Find the billing finalize block: the area between
|
||||
# adjust_payment_for_tokens and usage_finalized after it
|
||||
idx = source.find("adjust_payment_for_tokens")
|
||||
# Look at the next 500 chars after the call to find the except clause
|
||||
finalize_area = source[idx : idx + 500]
|
||||
|
||||
# Should use a narrow exception tuple, not bare `except Exception:`
|
||||
assert "except Exception:" not in finalize_area, (
|
||||
"Streaming billing finalize must use a narrow exception type, "
|
||||
"not catch-all Exception"
|
||||
)
|
||||
assert "SQLAlchemyError" in finalize_area or "OSError" in finalize_area, (
|
||||
"Streaming billing finalize should catch specific DB/upstream errors"
|
||||
)
|
||||
|
||||
|
||||
def test_streaming_messages_finalize_uses_narrow_exception() -> None:
|
||||
"""The messages streaming finalize must not catch all Exception."""
|
||||
import inspect
|
||||
|
||||
source = inspect.getsource(
|
||||
BaseUpstreamProvider.handle_streaming_messages_completion
|
||||
)
|
||||
|
||||
idx = source.find("adjust_payment_for_tokens")
|
||||
finalize_area = source[idx : idx + 500]
|
||||
|
||||
assert "except Exception:" not in finalize_area, (
|
||||
"Messages streaming billing finalize must use a narrow exception type"
|
||||
)
|
||||
assert "SQLAlchemyError" in finalize_area or "OSError" in finalize_area, (
|
||||
"Messages streaming finalize should catch specific DB/upstream errors"
|
||||
)
|
||||
|
||||
|
||||
def test_safe_finalize_billing_method_exists() -> None:
|
||||
"""The _safe_finalize_billing helper method must exist on the class."""
|
||||
assert hasattr(BaseUpstreamProvider, "_safe_finalize_billing"), (
|
||||
"_safe_finalize_billing helper must exist on BaseUpstreamProvider"
|
||||
)
|
||||
import inspect
|
||||
|
||||
sig = inspect.signature(BaseUpstreamProvider._safe_finalize_billing)
|
||||
params = list(sig.parameters.keys())
|
||||
assert "deducted_max_cost" in params, "Helper must accept deducted_max_cost"
|
||||
assert "error" in params, "Helper must accept the original error"
|
||||
assert "context" in params, "Helper must accept a context string"
|
||||
Reference in New Issue
Block a user