Partially Fixes #32264
Some tests are failing when bitcoin-cli returns an empty string. This change treats an empty response as None. See #32264 (comment)
This fixes the error for:
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32286.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | maflcko, pablomartin4btc, mzumsande, achow101 |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
lgtm ACK a4041c77f0e20d004524868e70ff12508832c9eb
I haven't tested this, but it makes sense to recover an empty string as json-none for legacy RPCs. Obviously, this could lead to errors when an empty string is returned for real, but this should never happen: I checked that all unwrapped json-strings returned are never empty (txid, block hash, hex encodings, addresses, ...). I haven't checked getblocktemplate rejection strings, but this seems fine either way, given that it is only the test suite.
For new RPCs, they are recommended to return json dicts, always. So this won't be an issue going forward.
918 | @@ -919,6 +919,8 @@ def send_cli(self, clicommand=None, *args, **kwargs): 919 | # Ignore cli_stdout, raise with cli_stderr 920 | raise subprocess.CalledProcessError(returncode, p_args, output=cli_stderr) 921 | try: 922 | + if not cli_stdout.strip(): 923 | + return None 924 | return json.loads(cli_stdout, parse_float=decimal.Decimal) 925 | except (json.JSONDecodeError, decimal.InvalidOperation): 926 | return cli_stdout.rstrip("\n")
nit: checking for cli_stdout shouldn't be outside the try? also cos it shouldn't fail and if it does the except is also accessing it...
raise subprocess.CalledProcessError(returncode, p_args, output=cli_stderr)
if not cli_stdout.strip():
return None
try:
return json.loads(cli_stdout, parse_float=decimal.Decimal)
except (json.JSONDecodeError, decimal.InvalidOperation):
return cli_stdout.rstrip("\n")
I think this was more to keep the logic in one place
ACK a4041c77f0e20d004524868e70ff12508832c9eb
ACK a4041c77f0e20d004524868e70ff12508832c9eb
ACK a4041c77f0e20d004524868e70ff12508832c9eb
