test: restore JSONRPCException error format #35385

pull rkrux wants to merge 1 commits into bitcoin:master from rkrux:jsonrpcexception changing 1 files +8 −1

rkrux commented at 3:03 PM on May 26, 2026: contributor

This is a follow-up to PR #34575.
DrahtBot added the label Tests on May 26, 2026
DrahtBot commented at 3:04 PM on May 26, 2026: contributor



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



Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35385.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK maflcko

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.
in test/functional/test_framework/util.py:38 in 57758b02cc
```
  30 | @@ -30,10 +31,16 @@
  31 |  
  32 |  class JSONRPCException(Exception):
  33 |      def __init__(self, rpc_error, http_status=None):
  34 | -        super().__init__(f"{rpc_error} [http_status={http_status}]")
  35 |          self.error = rpc_error
  36 |          self.http_status = http_status
  37 |  
  38 | +        # throw KeyError if any required fields are missing
  39 | +        copied_error = deepcopy(rpc_error)
```
maflcko commented at 7:53 PM on May 26, 2026:

copied_error = copy.copy(rpc_error)

nit: Haven't tried, but a shallow copy should be enough?
mercie-ux commented at 7:30 AM on May 29, 2026:

deepcopy(rpc_error) seems to be doing recursive work here while there isnt much to recurse into. I agree a shallow copy would work better.

nit: Haven't tried, but a shallow copy should be enough?

rkrux commented at 1:41 PM on May 29, 2026:

Switched to using shallow copy instead, should be fine I suppose because the extra is used as-is without any modification.
maflcko approved

Type	Reviewers
ACK	maflcko

test: restore JSONRPCException error format

This is a follow-up to PR 34575.

Copy is done so that checking of error["code"] in test_node.py
while handling this exception doesn't fail.

Co-authored-by: maflcko <6399679+maflcko@users.noreply.github.com>

ac09260982

rkrux force-pushed on May 29, 2026

maflcko commented at 1:07 PM on May 29, 2026: member

lgtm ACK ac09260982acbe1e9cdfb78eae289bbaf335f3e4

This asserts that required Keys must be present, which makes sense.

Also, the output looks a bit nicer now:

Before:

test_framework.util.JSONRPCException: {'code': -1, 'message': 'Internal bug detected: Unreachable code reached (non-fatal)\nrpc/blockchain.cpp:1618 (auto getchaintips()::(anonymous class)::operator()(const RPCMethod &, const JSONRPCRequest &) const)\nBitcoin Core v31.99.0-13b7fffc5e06-dirty\nPlease report this issue here: https://github.com/bitcoin/bitcoin/issues\n'} [http_status=200]

After:

test_framework.util.JSONRPCException: Internal bug detected: Unreachable code reached (non-fatal)
rpc/blockchain.cpp:1618 (auto getchaintips()::(anonymous class)::operator()(const RPCMethod &, const JSONRPCRequest &) const)
Bitcoin Core v31.99.0-13b7fffc5e06-dirty
Please report this issue here: https://github.com/bitcoin/bitcoin/issues
 (-1)  [http_status=200]

DrahtBot added the label CI failed on May 29, 2026
rkrux commented at 1:36 PM on May 29, 2026: contributor

I don't know why the Alpine musl job fails here.

It fails to download the log archive and the raw logs don't load fully in my browser.
maflcko commented at 1:39 PM on May 29, 2026: member

Fixed in #35379. Can be ignored here.
fanquake merged this on May 29, 2026
fanquake closed this on May 29, 2026

Contributors

Labels

Linked (view graph)

#34575 test: Avoid empty errmsg in JSONRPCException

test: restore JSONRPCException error format #35385

Code Coverage & Benchmarks

Reviews