diff --git a/app/lightning/impl/lnd_grpc.py b/app/lightning/impl/lnd_grpc.py index f47e0b8..0b2dcb0 100644 --- a/app/lightning/impl/lnd_grpc.py +++ b/app/lightning/impl/lnd_grpc.py @@ -41,15 +41,35 @@ from app.lightning.utils import alias_or_empty @logger.catch(exclude=(HTTPException,)) -def _check_if_locked(error): - logger.debug("logger._check_if_locked()") +def _check_transient_ln_error(error): + """Map known transient LND gRPC errors to appropriate HTTP statuses. - if error.details() is not None and error.details().find("wallet locked") > -1: + Returns without raising for unknown errors, leaving the caller to turn + them into a 500. + """ + logger.debug("logger._check_transient_ln_error()") + + details = error.details() + if details is None: + return + + if details.find("wallet locked") > -1: raise HTTPException( status.HTTP_423_LOCKED, detail="Wallet is locked. Unlock via /lightning/unlock-wallet", ) + if "the RPC server is in the process of starting up" in details: + # LND is up but its RPC server isn't ready yet; signal a retryable + # status instead of a generic 500 (blitz_api#247) + raise HTTPException( + status.HTTP_425_TOO_EARLY, + detail=( + "The Lightning RPC server is starting up and not yet ready. " + "Please try again shortly." + ), + ) + # Due to updated ECDSA generated tls.cert we need to let gprc know that # we need to use that cipher suite otherwise there will be a handshake @@ -298,7 +318,7 @@ This will show more debug information. return WalletBalance.from_lnd_grpc(onchain, channel) except grpc.aio._call.AioRpcError as error: - _check_if_locked(error) + _check_transient_ln_error(error) raise HTTPException( status.HTTP_500_INTERNAL_SERVER_ERROR, detail=error.details() ) @@ -379,7 +399,7 @@ This will show more debug information. return tx[index_offset : index_offset + max_tx] except grpc.aio._call.AioRpcError as error: - _check_if_locked(error) + _check_transient_ln_error(error) raise HTTPException( status.HTTP_500_INTERNAL_SERVER_ERROR, detail=error.details() ) @@ -404,7 +424,7 @@ This will show more debug information. response = await self._lnd_stub.ListInvoices(req) return [Invoice.from_lnd_grpc(i) for i in response.invoices] except grpc.aio._call.AioRpcError as error: - _check_if_locked(error) + _check_transient_ln_error(error) raise HTTPException( status.HTTP_500_INTERNAL_SERVER_ERROR, detail=error.details() ) @@ -418,7 +438,7 @@ This will show more debug information. response = await self._lnd_stub.GetTransactions(req) return [OnChainTransaction.from_lnd_grpc(t) for t in response.transactions] except grpc.aio._call.AioRpcError as error: - _check_if_locked(error) + _check_transient_ln_error(error) raise HTTPException( status.HTTP_500_INTERNAL_SERVER_ERROR, detail=error.details() ) @@ -449,7 +469,7 @@ This will show more debug information. response = await self._lnd_stub.ListPayments(req) return [Payment.from_lnd_grpc(p) for p in response.payments] except grpc.aio._call.AioRpcError as error: - _check_if_locked(error) + _check_transient_ln_error(error) raise HTTPException( status.HTTP_500_INTERNAL_SERVER_ERROR, detail=error.details() ) @@ -495,7 +515,7 @@ This will show more debug information. return invoice except grpc.aio._call.AioRpcError as error: - _check_if_locked(error) + _check_transient_ln_error(error) raise HTTPException( status.HTTP_500_INTERNAL_SERVER_ERROR, detail=error.details() ) @@ -509,7 +529,7 @@ This will show more debug information. res = await self._lnd_stub.DecodePayReq(req) return PaymentRequest.from_lnd_grpc(res) except grpc.aio._call.AioRpcError as error: - _check_if_locked(error) + _check_transient_ln_error(error) if ( error.details() is not None and error.details().find("checksum failed.") > -1 @@ -540,7 +560,7 @@ This will show more debug information. response = await self._lnd_stub.NewAddress(req) return response.address except grpc.aio._call.AioRpcError as error: - _check_if_locked(error) + _check_transient_ln_error(error) raise HTTPException( status.HTTP_500_INTERNAL_SERVER_ERROR, detail=error.details() ) @@ -576,7 +596,7 @@ This will show more debug information. await broadcast_sse_msg(SSE.LN_ONCHAIN_PAYMENT_STATUS, r.model_dump()) return r except grpc.aio._call.AioRpcError as error: - _check_if_locked(error) + _check_transient_ln_error(error) details = error.details() if details and details.find("invalid bech32 string") > -1: raise HTTPException( @@ -623,7 +643,7 @@ This will show more debug information. await broadcast_sse_msg(SSE.LN_PAYMENT_STATUS, p.model_dump()) return p except grpc.aio._call.AioRpcError as error: - _check_if_locked(error) + _check_transient_ln_error(error) if ( error.details() is not None and error.details().find("invalid bech32 string") > -1 @@ -692,7 +712,7 @@ This will show more debug information. response = await self._lnd_stub.GetInfo(req) return LnInfo.from_lnd_grpc(self.get_implementation_name(), response) except grpc.aio._call.AioRpcError as error: - _check_if_locked(error) + _check_transient_ln_error(error) raise HTTPException( status.HTTP_500_INTERNAL_SERVER_ERROR, detail=error.details() ) @@ -771,7 +791,7 @@ This will show more debug information. async for r in self._lnd_stub.SubscribeInvoices(request): yield Invoice.from_lnd_grpc(r) except grpc.aio._call.AioRpcError as error: - _check_if_locked(error) + _check_transient_ln_error(error) raise HTTPException( status.HTTP_500_INTERNAL_SERVER_ERROR, detail=error.details() ) @@ -811,7 +831,7 @@ This will show more debug information. del _fwd_cache[e.incoming_htlc_id] except grpc.aio._call.AioRpcError as error: - _check_if_locked(error) + _check_transient_ln_error(error) raise HTTPException( status.HTTP_500_INTERNAL_SERVER_ERROR, detail=error.details() ) diff --git a/tests/test_lnd_transient_errors.py b/tests/test_lnd_transient_errors.py new file mode 100644 index 0000000..89b458c --- /dev/null +++ b/tests/test_lnd_transient_errors.py @@ -0,0 +1,50 @@ +""" +Regression test for blitz_api#247 (original symptom). + +While the LND RPC server is still starting up it answers gRPC calls with +"the RPC server is in the process of starting up, but not yet ready to accept +calls". The API mapped only the "wallet locked" case to a proper status and +returned a bare 500 for everything else, so list-all-tx (and friends) 500'd +during LND startup. The startup case should map to a transient status the +client can retry. +""" + +import pytest +from fastapi import HTTPException +from starlette import status + +from app.lightning.impl import lnd_grpc + + +class _FakeGrpcError: + def __init__(self, details): + self._details = details + + def details(self): + return self._details + + +def test_wallet_locked_maps_to_423(): + err = _FakeGrpcError("wallet locked, unlock it to enable full RPC access") + with pytest.raises(HTTPException) as exc: + lnd_grpc._check_transient_ln_error(err) + assert exc.value.status_code == status.HTTP_423_LOCKED + + +def test_rpc_starting_up_maps_to_425(): + err = _FakeGrpcError( + "the RPC server is in the process of starting up, but not yet " + "ready to accept calls" + ) + with pytest.raises(HTTPException) as exc: + lnd_grpc._check_transient_ln_error(err) + assert exc.value.status_code == status.HTTP_425_TOO_EARLY + + +def test_unrelated_error_does_not_raise(): + # unknown errors are left for the caller to turn into a 500 + lnd_grpc._check_transient_ln_error(_FakeGrpcError("some unrelated error")) + + +def test_none_details_does_not_raise(): + lnd_grpc._check_transient_ln_error(_FakeGrpcError(None))