cli: improve bitcoin-cli error when not connected

willcl-ark commented at 10:31 am on March 21, 2024: contributor

Simply adds an additional suggestion to check bitcoin-cli -help.

Adds a string suggestion `bitcoin-cli -help` as an additional source of
information.

69d6fd676e

DrahtBot commented at 10:31 am on March 21, 2024: contributor

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	maflcko, tdb3, itornaza

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

DrahtBot added the label Scripts and tools on Mar 21, 2024

maflcko commented at 10:57 am on March 21, 2024: member

lgtm ACK 69d6fd676e9c15ef41a03722d51ed0b13c3e4320

in src/bitcoin-cli.cpp:833 in 69d6fd676e

826@@ -827,7 +827,10 @@ static UniValue CallRPC(BaseRequestHandler* rh, const std::string& strMethod, co
827         if (response.error != -1) {
828             responseErrorMessage = strprintf(" (error code %d - \"%s\")", response.error, http_errorstring(response.error));
829         }
830-        throw CConnectionFailed(strprintf("Could not connect to the server %s:%d%s\n\nMake sure the bitcoind server is running and that you are connecting to the correct RPC port.", host, port, responseErrorMessage));
831+        throw CConnectionFailed(strprintf("Could not connect to the server %s:%d%s\n\n"
832+                    "Make sure the bitcoind server is running and that you are connecting to the correct RPC port.\n"
833+                    "Use \"bitcoin-cli -help\" for more info.",
834+                    host, port, responseErrorMessage));

cbergqvist commented at 11:48 am on March 21, 2024:

3 tabs eh? :) I suggest conforming with the throw-statement following the one being changed - putting the initial string literal on a new line and keep to one tab indentation.

0        throw CConnectionFailed(strprintf(
1            "Could not connect to the server %s:%d%s\n\n"
2            "Make sure the bitcoind server is running and that you are connecting to the correct RPC port.\n"
3            "Use \"bitcoin-cli -help\" for more info.",
4            host, port, responseErrorMessage));

willcl-ark commented at 3:38 pm on March 21, 2024:

clang-format-diff has other ideas:

0        throw CConnectionFailed(strprintf("Could not connect to the server %s:%d%s\n\n"
1                                          "Make sure the bitcoind server is running and that you are connecting to the correct RPC port.\n"
2                                          "Use \"bitcoin-cli -help\" for more info.",
3                                          host, port, responseErrorMessage));

Not sure where the 3 tabs came from, as my check script is supposed to run that… I guess was my editor default somehow though.

maflcko commented at 4:29 pm on March 21, 2024:

@cbergqvist Generally, the style is left to the pull request author and usually not changed, unless someone else touches it. Also, tabs \t are not allowed in C++ code in this repo. If there were any, the CI should have failed.

cbergqvist commented at 6:02 pm on March 21, 2024:

I regret the initial tone of my comment but I’m happy clang-format came up with something reasonable.

maflcko commented at 6:51 pm on March 21, 2024:

Sure, no worries :)

in src/bitcoin-cli.cpp:832 in 69d6fd676e

826@@ -827,7 +827,10 @@ static UniValue CallRPC(BaseRequestHandler* rh, const std::string& strMethod, co
827         if (response.error != -1) {
828             responseErrorMessage = strprintf(" (error code %d - \"%s\")", response.error, http_errorstring(response.error));
829         }
830-        throw CConnectionFailed(strprintf("Could not connect to the server %s:%d%s\n\nMake sure the bitcoind server is running and that you are connecting to the correct RPC port.", host, port, responseErrorMessage));
831+        throw CConnectionFailed(strprintf("Could not connect to the server %s:%d%s\n\n"
832+                    "Make sure the bitcoind server is running and that you are connecting to the correct RPC port.\n"
833+                    "Use \"bitcoin-cli -help\" for more info.",

cbergqvist commented at 2:59 pm on March 21, 2024:

nit: ++specificity

0                    "Use \"bitcoin-cli -help\" for more connection info.",

willcl-ark commented at 3:39 pm on March 21, 2024:

I think “connection info” might be less accurate here?

cbergqvist commented at 6:12 pm on March 21, 2024:

Was thinking the user might be missing -rpcport, -rpcconnect or -signet (different default port), but not -generate -netinfo or -stdin. Maybe -signet is not something one would classify as “connection info” though, making it less accurate as you say.

cbergqvist commented at 3:01 pm on March 21, 2024: none

This is one of the first errors users are likely to encounter, so I support making it a bit more helpful.

luke-jr referenced this in commit c8e1382472 on Mar 23, 2024

tdb3 commented at 3:34 pm on March 23, 2024: contributor

ACK for 69d6fd676e9c15ef41a03722d51ed0b13c3e4320

Good light-touch approach to address Issue #29555. Executed, and the output looks good:

0$ src/bitcoin-cli help
1error: timeout on transient error: Could not connect to the server 127.0.0.1:8332
2
3Make sure the bitcoind server is running and that you are connecting to the correct RPC port.
4Use "bitcoin-cli -help" for more info.

Just to be thorough: Pulled the pr branch, built, ran all unit and functional tests (all passed) before executing.

MarnixCroes approved

MarnixCroes commented at 7:58 am on March 26, 2024: contributor

ack 69d6fd676e9c15ef41a03722d51ed0b13c3e4320

itornaza commented at 6:46 pm on March 29, 2024: none

tested ACK 69d6fd676e9c15ef41a03722d51ed0b13c3e4320

This a reasonable way for both reporting the connection failure, plus guiding the user on how to get more help on bitcoin-cli.

Output

The output upon trying to connect without having the bitcoind already running is confirmed:

0% src/bitcoin-cli help
1error: timeout on transient error: Could not connect to the server 127.0.0.1:18332
2
3Make sure the bitcoind server is running and that you are connecting to the correct RPC port.
4Use "bitcoin-cli -help" for more info.

Tests

Configured with --with-incompatible-bdb and --enable-suppress-external-warnings
Run unit tests with make check and all tests pass
Run all functional tests at test/functional/test_runner.py --extended and all tests pass

fanquake merged this on Apr 2, 2024

fanquake closed this on Apr 2, 2024

cli: improve bitcoin-cli error when not connected #29687

Code Coverage

Reviews

Output

Tests