diff --git a/routstr/balance.py b/routstr/balance.py index f887f5ae..33760ceb 100644 --- a/routstr/balance.py +++ b/routstr/balance.py @@ -384,9 +384,9 @@ async def refund_wallet_endpoint( ) await session.refresh(key) if key.reserved_balance > 0: - raise HTTPException( - status_code=400, - detail="Cannot refund key. There are ongoing requests for this api key.", + raise refund.refund_failure_error( + "Cannot refund key. There are ongoing requests for this api key.", + refund.REFUND_ONGOING_REQUESTS, ) logger.warning( "refund_wallet_endpoint: released stale reservation before refund", @@ -401,9 +401,13 @@ async def refund_wallet_endpoint( remaining_balance = refund.amount_in_unit(remaining_balance_msats, unit) if remaining_balance_msats > 0 and remaining_balance <= 0: - raise HTTPException(status_code=400, detail="Balance too small to refund") + raise refund.refund_failure_error( + "Balance too small to refund", refund.REFUND_BALANCE_TOO_SMALL + ) elif remaining_balance <= 0: - raise HTTPException(status_code=400, detail="No balance to refund") + raise refund.refund_failure_error( + "No balance to refund", refund.REFUND_NO_BALANCE + ) requested = refund_request.lightning_address if refund_request else None destination = requested or key.refund_address diff --git a/routstr/refund.py b/routstr/refund.py index 2aee20e8..babe146d 100644 --- a/routstr/refund.py +++ b/routstr/refund.py @@ -252,6 +252,38 @@ async def hold( await session.commit() +# Machine-readable reasons a refund cannot proceed. Clients act on these +# codes, so they must not be reworded: `no_balance_to_refund` proves the key +# holds nothing and its local copy can be dropped, while +# `refund_ongoing_requests` is a transient race whose balance is still there. +REFUND_NO_BALANCE = "no_balance_to_refund" +REFUND_BALANCE_TOO_SMALL = "balance_too_small_to_refund" +REFUND_ONGOING_REQUESTS = "refund_ongoing_requests" + + +def refund_failure_error( + message: str, code: str, status_code: int = 400 +) -> HTTPException: + """A refund refusal carrying a structured `code`, not a bare `detail` string. + + These used to be plain-string 400s, which are indistinguishable to a + client: the SDK could not tell "No balance to refund" (the key is dead, + drop it) from "Cannot refund key. There are ongoing requests for this api + key." (a transient race, keep the key). It kept every key and re-swept it + forever, re-reading the same 400 every five minutes. + """ + return HTTPException( + status_code=status_code, + detail={ + "error": { + "message": message, + "type": "invalid_request_error", + "code": code, + } + }, + ) + + def refund_in_progress_error(refund: Refund | None = None) -> HTTPException: """The 409 raised when a key already has an unresolved refund claim.""" stuck = refund is not None and refund.status == "stuck" diff --git a/tests/integration/test_database_consistency.py b/tests/integration/test_database_consistency.py index 5c2bbe68..3ab9aa19 100644 --- a/tests/integration/test_database_consistency.py +++ b/tests/integration/test_database_consistency.py @@ -409,7 +409,9 @@ class TestDataIntegrity: # Should fail assert response.status_code == 400 - assert "Balance too small to refund" in response.json()["detail"] + error = response.json()["detail"]["error"] + assert error["code"] == "balance_too_small_to_refund" + assert "Balance too small to refund" in error["message"] # Verify balance unchanged await integration_session.refresh(api_key) diff --git a/tests/integration/test_wallet_refund.py b/tests/integration/test_wallet_refund.py index 1029d073..7aeacf36 100644 --- a/tests/integration/test_wallet_refund.py +++ b/tests/integration/test_wallet_refund.py @@ -134,7 +134,10 @@ async def test_zero_balance_refund_handling( response = await integration_client.post("/v1/wallet/refund") assert response.status_code == 400 - assert response.json()["detail"] == "No balance to refund" + error = response.json()["detail"]["error"] + assert error["code"] == "no_balance_to_refund" + assert error["message"] == "No balance to refund" + assert error["type"] == "invalid_request_error" # Key should still exist result = await integration_session.execute( @@ -585,7 +588,9 @@ async def test_refund_error_handling( response = await integration_client.post("/v1/wallet/refund") assert response.status_code == 400 - assert response.json()["detail"] == "No balance to refund" + error = response.json()["detail"]["error"] + assert error["code"] == "no_balance_to_refund" + assert error["message"] == "No balance to refund" @pytest.mark.integration diff --git a/tests/unit/test_refund_failure_error.py b/tests/unit/test_refund_failure_error.py new file mode 100644 index 00000000..357f0d99 --- /dev/null +++ b/tests/unit/test_refund_failure_error.py @@ -0,0 +1,51 @@ +"""The refund endpoint's refusal reasons must stay machine-readable. + +Clients act on these codes. ``no_balance_to_refund`` proves the key holds +nothing and its stored copy can be dropped; ``refund_ongoing_requests`` is a +transient race whose balance is still on the key and must be kept; +``balance_too_small_to_refund`` is dust that no retry can fix. They used to be +bare ``detail`` strings, which are indistinguishable to a client, so the SDK +kept every dead key and re-swept it forever. +""" + +import pytest +from fastapi import HTTPException + +from routstr import refund + + +@pytest.mark.parametrize( + ("code", "message"), + [ + (refund.REFUND_NO_BALANCE, "No balance to refund"), + (refund.REFUND_BALANCE_TOO_SMALL, "Balance too small to refund"), + ( + refund.REFUND_ONGOING_REQUESTS, + "Cannot refund key. There are ongoing requests for this api key.", + ), + ], +) +def test_refund_failure_error_carries_its_code(code: str, message: str) -> None: + error = refund.refund_failure_error(message, code) + + assert isinstance(error, HTTPException) + assert error.status_code == 400 + assert error.detail == { + "error": { + "message": message, + "type": "invalid_request_error", + "code": code, + } + } + + +def test_refund_failure_codes_are_distinct_lowercase_slugs() -> None: + codes = { + refund.REFUND_NO_BALANCE, + refund.REFUND_BALANCE_TOO_SMALL, + refund.REFUND_ONGOING_REQUESTS, + } + + assert len(codes) == 3 + assert all(code == code.lower() for code in codes) + assert all(code.strip() == code for code in codes) diff --git a/tests/unit/test_stale_reservations.py b/tests/unit/test_stale_reservations.py index 61a3da5a..eedffada 100644 --- a/tests/unit/test_stale_reservations.py +++ b/tests/unit/test_stale_reservations.py @@ -377,7 +377,9 @@ async def test_refund_rejects_recent_reservation(session: AsyncSession) -> None: ) assert exc_info.value.status_code == 400 - assert "ongoing requests" in exc_info.value.detail + error = exc_info.value.detail["error"] + assert error["code"] == "refund_ongoing_requests" + assert "ongoing requests" in error["message"] @pytest.mark.asyncio