fix: phoenixd pending payments (#3075)

Co-authored-by: Vlad Stan <stan.v.vlad@gmail.com>

lnbits/wallets/phoenixd.py

@@ -7,6 +7,7 @@ from collections.abc import AsyncGenerator
 from typing import Any, Optional
 
 import httpx
+from httpx import RequestError, TimeoutException
 from loguru import logger
 from websockets.legacy.client import connect
 
@@ -15,6 +16,7 @@ from lnbits.settings import settings
 
 from .base import (
     InvoiceResponse,
+    PaymentFailedStatus,
     PaymentPendingStatus,
     PaymentResponse,
     PaymentStatus,
@@ -173,13 +175,29 @@ class PhoenixdWallet(Wallet):
             )
 
             r.raise_for_status()
+
+        except TimeoutException:
+            # be safe and return pending on timeouts
+            msg = f"Timeout connecting to {self.endpoint}. keep pending..."
+            logger.warning(msg)
+            return PaymentResponse(ok=None, error_message=msg)
+        except RequestError as exc:
+            # RequestError is raised when the request never hit the destination server
+            msg = f"Unable to connect to {self.endpoint}."
+            logger.warning(msg)
+            logger.warning(exc)
+            return PaymentResponse(ok=False, error_message=msg)
+        except Exception as exc:
+            logger.warning(exc)
+            return PaymentResponse(
+                ok=None, error_message=f"Unable to connect to {self.endpoint}."
+            )
+
+        try:
             data = r.json()
 
-            if "routingFeeSat" not in data and "reason" in data:
+            if "routingFeeSat" not in data and ("reason" in data or "message" in data):
-                return PaymentResponse(error_message=data["reason"])
+                error_message = data.get("reason", data.get("message", "Unknown error"))
-
-            if r.is_error or "paymentHash" not in data:
-                error_message = data["message"] if "message" in data else r.text
                 return PaymentResponse(error_message=error_message)
 
             checking_id = data["paymentHash"]
@@ -208,38 +226,73 @@ class PhoenixdWallet(Wallet):
             )
 
     async def get_invoice_status(self, checking_id: str) -> PaymentStatus:
-        try:
+        r = await self.client.get(
-            r = await self.client.get(f"/payments/incoming/{checking_id}")
+            f"/payments/incoming/{checking_id}?all=true&limit=1000"
+        )
         if r.is_error:
+            if r.status_code == 404:
+                # invoice does not exist in phoenixd, so it was never paid
+                return PaymentFailedStatus()
+            else:
+                # otherwise something unexpected happened, and we keep it pending
+                logger.warning(
+                    f"Error getting invoice status: {r.text}, keeping pending"
+                )
                 return PaymentPendingStatus()
+        try:
             data = r.json()
+        except json.JSONDecodeError:
+            # should never return invalid json, but just in case we keep it pending
+            logger.warning(
+                f"Phoenixd: invalid json response: {r.text}, keeping pending"
+            )
+            return PaymentPendingStatus()
 
-            if data["isPaid"]:
+        if "isPaid" not in data or "fees" not in data or "preimage" not in data:
+            # should never return missing fields, but just in case we keep it pending
+            return PaymentPendingStatus()
+
+        if data["isPaid"] is True:
             fee_msat = data["fees"]
             preimage = data["preimage"]
             return PaymentSuccessStatus(fee_msat=fee_msat, preimage=preimage)
 
         return PaymentPendingStatus()
-        except Exception as e:
-            logger.error(f"Error getting invoice status: {e}")
-            return PaymentPendingStatus()
 
     async def get_payment_status(self, checking_id: str) -> PaymentStatus:
-        # TODO: how can we detect a failed payment?
+        r = await self.client.get(
-        try:
+            f"/payments/outgoing/{checking_id}?all=true&limit=1000"
-            r = await self.client.get(f"/payments/outgoing/{checking_id}")
+        )
         if r.is_error:
+            if r.status_code == 404:
+                # payment does not exist in phoenixd, so it was never paid
+                return PaymentFailedStatus()
+            else:
+                # otherwise something unexpected happened, and we keep it pending
+                logger.warning(
+                    f"Error getting payment status: {r.text}, keeping pending"
+                )
                 return PaymentPendingStatus()
+        try:
             data = r.json()
+        except json.JSONDecodeError:
+            # should never return invalid json, but just in case we keep it pending
+            logger.warning(
+                f"Phoenixd: invalid json response: {r.text}, keeping pending"
+            )
+            return PaymentPendingStatus()
 
-            if data["isPaid"]:
+        if "isPaid" not in data or "fees" not in data or "preimage" not in data:
+            # should never happen, but just in case we keep it pending
+            logger.warning(
+                f"Phoenixd: missing required fields: {data}, keeping pending"
+            )
+            return PaymentPendingStatus()
+
+        if data["isPaid"] is True:
             fee_msat = data["fees"]
             preimage = data["preimage"]
             return PaymentSuccessStatus(fee_msat=fee_msat, preimage=preimage)
-
-            return PaymentPendingStatus()
-        except Exception as e:
-            logger.error(f"Error getting invoice status: {e}")
         return PaymentPendingStatus()
 
     async def paid_invoices_stream(self) -> AsyncGenerator[str, None]:
@@ -263,7 +316,7 @@ class PhoenixdWallet(Wallet):
                             yield message_json["paymentHash"]
 
             except Exception as exc:
-                logger.error(
+                logger.warning(
                     f"lost connection to phoenixd invoices stream: '{exc}'"
                     "retrying in 5 seconds"
                 )

tests/wallets/fixtures/json/fixtures_rest.json

@@ -2182,8 +2182,16 @@
               "get_invoice_status_endpoint": []
             },
             "phoenixd": {
-              "description": "phoenixd.py doesn't handle the 'failed' status for `get_invoice_status`",
+              "get_invoice_status_endpoint": [
-              "get_invoice_status_endpoint": []
+                {
+                  "description": "http 404",
+                  "response_type": "response",
+                  "response": {
+                    "response": "Not Found",
+                    "status": 404
+                  }
+                }
+              ]
             }
           }
         },
@@ -2389,14 +2397,6 @@
                   "description": "bad json",
                   "response_type": "data",
                   "response": "data-not-json"
-                },
-                {
-                  "description": "http 404",
-                  "response_type": "response",
-                  "response": {
-                    "response": "Not Found",
-                    "status": 404
-                  }
                 }
               ]
             }
@@ -2674,8 +2674,16 @@
               ]
             },
             "phoenixd": {
-              "description": "phoenixd.py doesn't handle the 'failed' status for `get_invoice_status`",
+              "get_payment_status_endpoint": [
-              "get_payment_status_endpoint": []
+                {
+                  "description": "http 404",
+                  "response_type": "response",
+                  "response": {
+                    "response": "Not Found",
+                    "status": 404
+                  }
+                }
+              ]
             }
           }
         },
@@ -2932,14 +2940,6 @@
                   "description": "bad json",
                   "response_type": "data",
                   "response": "data-not-json"
-                },
-                {
-                  "description": "http 404",
-                  "response_type": "response",
-                  "response": {
-                    "response": "Not Found",
-                    "status": 404
-                  }
                 }
               ]
             }