From 3980ce0c6b18916221f47225e59bd947fa453aac Mon Sep 17 00:00:00 2001 From: fusion44 Date: Wed, 8 Jul 2026 13:16:10 +0200 Subject: [PATCH] fix(bitcoind): handle JSON-RPC errors returned with HTTP 200 Bitcoin Core returns the -28 warmup error ('Loading block index', 'Verifying blocks', 'Starting network threads') as an HTTP 200 response with a JSON-RPC error body. _process_response only classified errors on the non-200 path and passed a 200 body through unchanged, so the result had an 'error' key but no 'status' key. Callers doing 'raise HTTPException(result["status"], ...)' then crashed with KeyError: 'status' instead of HTTPException(425), so the startup warmup-retry loop in initialize_bitcoin_repo never engaged and the API (and login) failed while bitcoind was still warming up. Same root cause, different -28 warmup message. Fixes #287 Fixes #285 Normalize any JSON-RPC error to an {error, status} dict regardless of the HTTP status, mapping the warmup messages to 425 TOO_EARLY as before. Co-Authored-By: Claude Fable 5 --- app/bitcoind/utils.py | 93 +++++++++++++++++++------------- tests/test_bitcoin_rpc_warmup.py | 57 ++++++++++++++++++++ 2 files changed, 113 insertions(+), 37 deletions(-) create mode 100644 tests/test_bitcoin_rpc_warmup.py diff --git a/app/bitcoind/utils.py b/app/bitcoind/utils.py index 5ea6347..689bb72 100644 --- a/app/bitcoind/utils.py +++ b/app/bitcoind/utils.py @@ -90,10 +90,43 @@ async def bitcoin_rpc_async(method: str, params: list = []) -> coroutine: } -async def _process_response(resp: aiohttp.ClientResponse): - if resp.status == status.HTTP_200_OK: - return await resp.json() +def _classify_rpc_error(message: str, fallback_status: int, reason: str) -> dict: + """Map a Bitcoin Core JSON-RPC error message to an {error, status} dict.""" + if ( + "Loading block index" in message + or "Verifying blocks" in message + or "Starting network threads" in message + ): + return { + "error": ( + "Initializing Bitcoin Core (loading, verifying " + "blocks or starting network threads etc)" + ), + "status": status.HTTP_425_TOO_EARLY, + } + if "No such mempool or blockchain transaction." in message: + return { + "error": "No such mempool or blockchain transaction.", + "status": status.HTTP_404_NOT_FOUND, + } + if "parameter 1 must be of length 64" in message: + return { + "error": message, + "status": status.HTTP_400_BAD_REQUEST, + } + if "Use -txindex" in message: + return { + "error": "-txindex option for Bitcoin Core not enabled", + "status": status.HTTP_400_BAD_REQUEST, + } + return { + "error": f"Unknown answer from Bitcoin Core. Reason: {reason}", + "status": fallback_status, + } + + +async def _process_response(resp: aiohttp.ClientResponse): if resp.status == status.HTTP_401_UNAUTHORIZED: return { "error": ( @@ -112,39 +145,25 @@ async def _process_response(resp: aiohttp.ClientResponse): "status": status.HTTP_403_FORBIDDEN, } - e = await resp.json() - m = e["error"]["message"] + body = await resp.json() - if e["error"]: - if ( - "Loading block index" in m - or "Verifying blocks" in m - or "Starting network threads" in m - ): - return { - "error": ( - "Initializing Bitcoin Core (loading, verifying " - "blocks or starting network threads etc)" - ), - "status": status.HTTP_425_TOO_EARLY, - } - if "No such mempool or blockchain transaction." in m: - return { - "error": "No such mempool or blockchain transaction.", - "status": status.HTTP_404_NOT_FOUND, - } - if "parameter 1 must be of length 64" in m: - return { - "error": m, - "status": status.HTTP_400_BAD_REQUEST, - } - if "Use -txindex" in m: - return { - "error": "-txindex option for Bitcoin Core not enabled", - "status": status.HTTP_400_BAD_REQUEST, - } + # Bitcoin Core may return a JSON-RPC error either with a non-200 HTTP status + # or, during warmup (e.g. code -28 "Loading block index"), with HTTP 200. + # Normalize any JSON-RPC error to an {error, status} dict so callers can rely + # on a "status" key being present (previously a 200 + error body was passed + # through unchanged and crashed callers with KeyError: 'status'). + rpc_error = body.get("error") if isinstance(body, dict) else None + if rpc_error: + message = ( + rpc_error.get("message", "") + if isinstance(rpc_error, dict) + else str(rpc_error) + ) + fallback_status = ( + resp.status + if resp.status != status.HTTP_200_OK + else status.HTTP_500_INTERNAL_SERVER_ERROR + ) + return _classify_rpc_error(message, fallback_status, resp.reason) - return { - "error": f"Unknown answer from Bitcoin Core. Reason: {resp.reason}", - "status": resp.status, - } + return body diff --git a/tests/test_bitcoin_rpc_warmup.py b/tests/test_bitcoin_rpc_warmup.py new file mode 100644 index 0000000..e9f5e28 --- /dev/null +++ b/tests/test_bitcoin_rpc_warmup.py @@ -0,0 +1,57 @@ +""" +Regression test for blitz_api#287. + +Bitcoin Core returns the -28 "Loading block index" warmup error as an +HTTP 200 with a JSON-RPC error body. _process_response returned that body +unchanged, so it had an "error" key but no "status" key, and callers doing +`raise HTTPException(result["status"], ...)` crashed with KeyError: 'status' +instead of the HTTPException(425) the startup warmup loop expects. +""" + +import pytest +from starlette import status + +from app.bitcoind.utils import _process_response + + +class _FakeResp: + def __init__(self, http_status, body, reason="OK"): + self.status = http_status + self._body = body + self.reason = reason + + async def json(self): + return self._body + + +@pytest.mark.parametrize( + "message", + [ + "Loading block index…", # blitz_api#287 + "Verifying blocks…", # blitz_api#285 + "Starting network threads…", + ], +) +async def test_200_warmup_error_is_normalized_to_too_early(message): + resp = _FakeResp( + status.HTTP_200_OK, + { + "jsonrpc": "2.0", + "error": {"code": -28, "message": message}, + "id": 6, + }, + ) + + out = await _process_response(resp) + + assert out["status"] == status.HTTP_425_TOO_EARLY + assert out["error"] # a human-readable message, not the raw dict + + +async def test_200_success_body_is_passed_through(): + body = {"jsonrpc": "2.0", "result": {"blocks": 42}, "error": None, "id": 1} + resp = _FakeResp(status.HTTP_200_OK, body) + + out = await _process_response(resp) + + assert out == body