Fix KeyError on malformed Bitcoin Core RPC responses (#2532)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: k9ert <117085+k9ert@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

src/cryptoadvance/specter/rpc.py

@@ -276,7 +276,12 @@ class RpcError(Exception):
             self.error_msg = error["error"]["message"]
         except Exception:
             self.error_code = -99
-            self.error_msg = str(self) + " - UNKNOWN API-ERROR:%s" % response.text
+            # Handle both Response objects and dicts
+            if hasattr(response, "text"):
+                response_text = response.text
+            else:
+                response_text = str(response)
+            self.error_msg = str(self) + " - UNKNOWN API-ERROR:%s" % response_text
 
 
 class BitcoinRPC:
@@ -508,21 +513,59 @@ class BitcoinRPC:
 
     def __getattr__(self, method):
         def fn(*args, **kwargs):
-            r = self.multi([(method, *args)], **kwargs)[0]
+            responses = self.multi([(method, *args)], **kwargs)
+            # Ensure multi() returned a non-empty list before indexing
+            if not isinstance(responses, list) or not responses:
+                raise RpcError(
+                    f"Request error for method {method}{args}: Invalid batch response (expected non-empty list, got {type(responses).__name__})",
+                    response=responses,
+                    error_msg=f"Invalid batch response from multi(): {responses}",
+                )
+            r = responses[0]
+            # Safely check if response is a dict and has error field
+            if not isinstance(r, dict):
+                raise RpcError(
+                    f"Request error for method {method}{args}: Invalid response format (expected dict, got {type(r).__name__})",
+                    response=None,
+                    error_msg=f"Invalid response format: {r}",
+                )
+
             error = r.get("error")
             if error is not None:
+                # Safely extract error message and code
                 if isinstance(error, dict):
+                    error_code = error.get("code", -99)
                     error_msg = error.get("message", str(error))
+                    # If error has both code and message, pass the response dict
+                    # Otherwise, pass explicit error_msg/error_code to avoid overwriting
+                    if "code" in error and "message" in error:
+                        raise RpcError(
+                            f"Request error for method {method}{args}: {error_msg}",
+                            r,
+                        )
+                    else:
+                        raise RpcError(
+                            f"Request error for method {method}{args}: {error_msg}",
+                            response=None,
+                            error_code=error_code,
+                            error_msg=error_msg,
+                        )
                 else:
+                    # Error is not a dict (e.g., plain string)
                     error_msg = str(error)
-                raise RpcError(
-                    f"Request error for method {method}{args}: {error_msg}",
-                    r,
-                )
+                    raise RpcError(
+                        f"Request error for method {method}{args}: {error_msg}",
+                        response=None,
+                        error_code=-99,
+                        error_msg=error_msg,
+                    )
+
             if "result" not in r:
+                error_msg_text = f"Unexpected RPC response for method {method}{args}: missing 'result' key"
                 raise RpcError(
-                    f"Unexpected RPC response for method {method}{args}: missing 'result' key",
-                    r,
+                    error_msg_text,
+                    response=None,
+                    error_msg=error_msg_text,
                 )
             return r["result"]
 

tests/test_rpc.py

@@ -3,6 +3,7 @@ import logging
 import pytest
 import requests
 from requests import Response
+from unittest.mock import MagicMock
 from cryptoadvance.specter.rpc import (
     BitcoinRPC,
     RpcError,
@@ -258,6 +259,56 @@ def test_BitcoinRpc_timeout(rpc, caplog):
         BitcoinRPC.default_timeout = None
 
 
+def test_BitcoinRpc_malformed_response():
+    """Test handling of malformed RPC responses"""
+    # Create a mock RPC instance
+    rpc = BitcoinRPC("user", "pass", "127.0.0.1", 8332)
+
+    # Test 1: Response is not a dict (e.g., a string)
+    rpc.multi = MagicMock(return_value=["not a dict"])
+    with pytest.raises(RpcError) as exc_info:
+        rpc.getblockchaininfo()
+    assert "Invalid response format" in str(exc_info.value)
+    assert "expected dict" in str(exc_info.value)
+    assert "Invalid response format" in exc_info.value.error_msg
+
+    # Test 2: Response dict has error but error is not a dict
+    rpc.multi = MagicMock(return_value=[{"error": "plain string error"}])
+    with pytest.raises(RpcError) as exc_info:
+        rpc.getblockchaininfo()
+    assert "plain string error" in str(exc_info.value)
+    # Ensure error_msg doesn't degrade to UNKNOWN API-ERROR
+    assert "plain string error" in exc_info.value.error_msg
+    assert "UNKNOWN API-ERROR" not in exc_info.value.error_msg
+
+    # Test 3: Response dict has error dict but missing 'message' key
+    rpc.multi = MagicMock(return_value=[{"error": {"code": -1}}])
+    with pytest.raises(RpcError) as exc_info:
+        rpc.getblockchaininfo()
+    # Should handle missing message gracefully
+    assert "getblockchaininfo" in str(exc_info.value)
+    # Ensure error_msg contains the error dict representation, not UNKNOWN API-ERROR
+    assert "UNKNOWN API-ERROR" not in exc_info.value.error_msg
+    assert exc_info.value.error_code == -1
+
+    # Test 4: Response dict missing both 'error' and 'result' keys
+    rpc.multi = MagicMock(return_value=[{}])
+    with pytest.raises(RpcError) as exc_info:
+        rpc.getblockchaininfo()
+    assert "missing 'result' key" in str(exc_info.value)
+    assert "missing 'result' key" in exc_info.value.error_msg
+
+    # Test 5: Valid response with error=None should work
+    rpc.multi = MagicMock(return_value=[{"error": None, "result": {"blocks": 100}}])
+    result = rpc.getblockchaininfo()
+    assert result == {"blocks": 100}
+
+    # Test 6: Valid response without error key should work
+    rpc.multi = MagicMock(return_value=[{"result": {"blocks": 200}}])
+    result = rpc.getblockchaininfo()
+    assert result == {"blocks": 200}
+
+
 @pytest.fixture
 def rpc(bitcoin_regtest):
     brt = bitcoin_regtest  # stupid long name