test: Avoid empty errmsg in JSONRPCException

maflcko commented at 4:12 pm on February 12, 2026: member

It is unclear why the fallback should be an empty message, when it is better to include all rpc_error details that are available.

Also, include the http status.

This allows to revert commit 6354b4fd7fe819eb13274b212e426a7d10ca75d3, because it is no longer needed.

Can be tested by running this diff:

 0diff --git a/test/functional/wallet_disable.py b/test/functional/wallet_disable.py
 1index dbcccd4778..9717a2d248 100755
 2--- a/test/functional/wallet_disable.py
 3+++ b/test/functional/wallet_disable.py
 4@@ -18,9 +18,8 @@ class DisableWalletTest (BitcoinTestFramework):
 5         self.extra_args = [["-disablewallet"]]
 6         self.wallet_names = []
 7 
 8-    def run_test (self):
 9-        # Make sure wallet is really disabled
10-        assert_raises_rpc_error(-32601, 'Method not found', self.nodes[0].getwalletinfo)
11+    def run_test(self):
12+        self.nodes[0].getwalletinfo()
13         x = self.nodes[0].validateaddress('3J98t1WpEZ73CNmQviecrnyiWrnqRhWNLy')
14         assert x['isvalid'] == False
15         x = self.nodes[0].validateaddress('mneYUmWYsuk7kySiURxCi3AGxrAqZxLgPZ')

DrahtBot added the label Tests on Feb 12, 2026

DrahtBot commented at 4:12 pm on February 12, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	furszy, hodlinator, ismaelsadeeq

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

fanquake added this to the milestone 31.0 on Feb 12, 2026

in test/functional/test_framework/test_framework.py:155 in fa2ac3ee57 outdated

146@@ -147,9 +147,6 @@ def main(self):
147         except subprocess.CalledProcessError as e:
148             self.log.exception(f"Called Process failed with stdout='{e.stdout}'; stderr='{e.stderr}';")
149             self.success = TestStatus.FAILED
150-        except JSONRPCException as e:
151-            self.log.exception(f"Failure during setup: error={e.error}, http_status={e.http_status}")
152-            self.success = TestStatus.FAILED
153         except BaseException:
154             self.log.exception("Unexpected exception")
155             self.success = TestStatus.FAILED

furszy commented at 4:18 pm on February 13, 2026:

Shouldn’t we log the exception here now? Like:

0except BaseException as e:
1   self.log.exception(f"Unexpected exception: {e}")
2   self.success = TestStatus.FAILED

maflcko commented at 4:32 pm on February 13, 2026:

No, that’d log it twice, because Python already does the right thing internally :)

Happy to add a comment, if it helps

furszy commented at 5:40 pm on February 13, 2026:

oh, yeah. It would be helpful to mention it for python neophytes like me, as it is something that could come up in the future again.

maflcko commented at 5:56 pm on February 13, 2026:

thx, done

test: Avoid empty errmsg in JSONRPCException

It is unclear why the fallback should be an empty message, when it is
better to include all rpc_error details that are available.

Also, include the http status.

This allows to revert commit 6354b4fd7fe819eb13274b212e426a7d10ca75d3,
because it is no longer needed.

211111b804

maflcko force-pushed on Feb 13, 2026

furszy commented at 6:32 pm on February 13, 2026: member

utACK 211111b8048dad959a510150d80277bd9b3d0e25

hodlinator approved

hodlinator commented at 9:26 pm on February 13, 2026: contributor

ACK 211111b8048dad959a510150d80277bd9b3d0e25

Modified wallet_disable.py as suggested and got the following results:

Output on master

0₿ ./build/test/functional/wallet_disable.py
12026-02-13T21:08:07.396695Z TestFramework (ERROR): Failure during setup: error={'code': -32601, 'message': 'Method not found'}, http_status=200
2...
3test_framework.authproxy.JSONRPCException: Method not found (-32601)

Output with PR

0₿ ./build/test/functional/wallet_disable.py
12026-02-13T21:01:42.321047Z TestFramework (ERROR): Unexpected exception:
2...
3test_framework.authproxy.JSONRPCException: {'code': -32601, 'message': 'Method not found'} [http_status=200]

Conclusions

“Failure during setup” message on master is incorrect, the example makes us throw an exception from run_test(), which is included in the try-block along with setup().

https://github.com/bitcoin/bitcoin/blob/b65ff0e5a1fd4ea2ae75e204729b8008c4ebb9ab/test/functional/test_framework/test_framework.py#L137-L142
I think JSON-RPC 2 is meant to always use HTTP status 200 regardless of error but cannot find it in the specification, I guess adding it is okay, it’s being sent in everywhere after all.
The old output on master

Method not found (-32601)

was nicer than the PR’s

{‘code’: -32601, ‘message’: ‘Method not found’} [http_status=200]

ismaelsadeeq commented at 3:16 pm on February 16, 2026: member

utACK 211111b8048dad959a510150d80277bd9b3d0e25

maflcko commented at 3:43 pm on February 16, 2026: member

I think JSON-RPC 2 is meant to always use HTTP status 200 regardless of error but cannot find it in the specification, I guess adding it is okay, it’s being sent in everywhere after all.

See https://github.com/bitcoin/bitcoin/blob/35e6444fdc4068adc79082648f9889ad593e623b/doc/JSON-RPC-interface.md#json-rpc-11-vs-20

The old output on master

Method not found (-32601)

was nicer than the PR’s

{‘code’: -32601, ‘message’: ‘Method not found’} [http_status=200]

Yeah, happy to review a follow-up improving the wording. One could use something like (untested):

0 class JSONRPCException(Exception):
1    def __init__(self, rpc_error, http_status=None):
2        message = rpc_error.pop("message", "INTERNAL ERROR: message field missing in rpc error dict")
3        code = rpc_error.pop("code", "INTERNAL_ERROR: code field missing in rpc error dict")
4        extra = f'{rpc_error}' if rpc_error else ''
5        super().__init__(f"{message} ({code}) {extra} [http_status={http_status}]")

test: Avoid empty errmsg in JSONRPCException #34575

Reviews

Output on master

Output with PR

Conclusions