Rephrase error message for invalid value of `-peertimeout` #25506

pull ghost wants to merge 1 commits into bitcoin:master from changing 1 files +1 −1

ghost commented at 10:10 PM on June 29, 2022: none

Fixes #25505 (comment)
in src/init.cpp:971 in 1642256d2f outdated
```
 964 | @@ -965,8 +965,10 @@ bool AppInitParameterInteraction(const ArgsManager& args, bool use_syscall_sandb
 965 |      }
 966 |  
 967 |      peer_connect_timeout = args.GetIntArg("-peertimeout", DEFAULT_PEER_CONNECT_TIMEOUT);
 968 | -    if (peer_connect_timeout <= 0) {
 969 | +    if (peer_connect_timeout < 0) {
 970 |          return InitError(Untranslated("peertimeout cannot be configured with a negative value."));
 971 | +    } else if (peer_connect_timeout == 0) {
 972 | +        return InitError(Untranslated("invalid value for peertimeout."));
```
kristapsk commented at 10:24 PM on June 29, 2022:

return InitError(Untranslated("Invalid value for peertimeout."));
unknown commented at 10:40 PM on June 29, 2022:

Reasons for using lowercasel i instead of uppercase:

Error is printed with uppercase E in this format: Error: <error message>

Other error starts with lowercase p so maintain consistency
kristapsk commented at 10:49 PM on June 29, 2022:

Other error messages not starting with argument name does not start with lowercase. Lowercase is used above because peertimeout and Peertimeout will be different arguments. See https://github.com/bitcoin/bitcoin/blob/1642256d2ffb69b3a7386d9f99d584bf8c2df090/src/init.cpp#L1053, for example.

unknown commented at 11:01 PM on June 29, 2022:

Done in https://github.com/bitcoin/bitcoin/commit/6cd4bf35757d034556226eb9576d735c689c85ec
unknown force-pushed on Jun 29, 2022
kristapsk approved
kristapsk commented at 11:09 PM on June 29, 2022: contributor

ACK 6cd4bf35757d034556226eb9576d735c689c85ec
MarcoFalke commented at 6:12 AM on June 30, 2022: member

Not sure if we need two error messages to state a simple thing: A non-zero positive timeout is needed.
kristapsk commented at 7:35 AM on June 30, 2022: contributor

Not sure if we need two error messages to state a simple thing: A non-zero positive timeout is needed.

Single message is alternative approach, would then suggest "peertimeout must be a non-zero positive integer".
ghost commented at 7:51 AM on June 30, 2022: none

Not sure if we need two error messages to state a simple thing: A non-zero positive timeout is needed.

The error is incorrect and should be based on user input. It makes no sense to print negative value error when zero or non integer was entered.

Other approach is to replace old error with a new error that prints user input and calls it invalid similar to a few other errors and https://github.com/bitcoin/bitcoin/pull/22087/. Or remove =, just keep if (peer_connect_timeout < 0)and no error for other invalid values like -prune.
rephrase error for invalid timeout 748a10e896
unknown renamed this:
~~add error msg for invalid value of `-peertimeout`~~
Rephrase error message for invalid value of `-peertimeout`
on Jun 30, 2022
unknown force-pushed on Jun 30, 2022
ghost commented at 8:48 AM on June 30, 2022: none

Not sure if we need two error messages to state a simple thing: A non-zero positive timeout is needed.

Single message is alternative approach, would then suggest "peertimeout must be a non-zero positive integer". @kristapsk @MarcoFalke Changed the error message in last commit. No new errors.
kristapsk approved
kristapsk commented at 12:07 PM on June 30, 2022: contributor

re-ACK 748a10e896b84f084f573472428b76d49c3c9795
brunoerg approved
brunoerg commented at 12:33 PM on June 30, 2022: member

ACK 748a10e896b84f084f573472428b76d49c3c9795

After this get merged, I can change #25505.
w0xlt approved
w0xlt commented at 12:55 PM on June 30, 2022: contributor

ACK https://github.com/bitcoin/bitcoin/pull/25506/commits/748a10e896b84f084f573472428b76d49c3c9795
MarcoFalke added the label Docs on Jun 30, 2022
MarcoFalke merged this on Jun 30, 2022
MarcoFalke closed this on Jun 30, 2022
MarcoFalke referenced this in commit b6cf0f8848 on Jun 30, 2022
sidhujag referenced this in commit 97f981e659 on Jun 30, 2022
sidhujag referenced this in commit 8d61dab2e7 on Jun 30, 2022
DrahtBot locked this on Jun 30, 2023

Contributors

Labels

Linked (view graph)

#25505 test: passing a negative value to `-peertimeout` should throw an error #25511 test: non-positive integer value to `-peertimeout` should throw an error