mirror of
https://github.com/Routstr/routstr-core.git
synced 2026-10-05 12:28:22 +00:00
fix(refund): give refund refusals machine-readable error codes
/v1/wallet/refund answered three distinct refusals with a bare `detail`
string: "No balance to refund", "Balance too small to refund" and "Cannot
refund key. There are ongoing requests for this api key." They are
indistinguishable to a client, and the consequences differ: the first proves
the key holds nothing and its stored copy can be dropped, the second is dust
no retry can pay out, and the third is a transient race whose balance is
still on the key and must be kept.
The SDK tried to tell them apart by matching the whole error string, but the
refund error it compares against is built as "API key refund failed:
<detail>", so its no-balance branch never matched. Every dead key stayed in
storage and was re-swept every five minutes, forever.
All three now carry the structured envelope the other refund errors already
use ({"error": {"type", "code", "message"}}), with codes
`no_balance_to_refund`, `balance_too_small_to_refund` and
`refund_ongoing_requests`. Messages and status codes are unchanged, so
message-matching clients (including SDK <= 0.4.8) behave exactly as before.
This commit is contained in:
+9
-5
@@ -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
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user