Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bitcoin-cli] improve error output #6387

Merged

Conversation

jonasschnelli
Copy link
Contributor

@jonasschnelli jonasschnelli commented Jul 7, 2015

[only affects bitcoin-cli]

At the moment, errors in bitcoin-cli are sent to stderr as compact json string. Example:

jonasschnelli$ ./src/bitcoin-cli -regtest sendtoaddress 276536543
error: {"code":-1,"message":"sendtoaddress \"bitcoinaddress\" amount ( \"comment\" \"comment-to\" 
subtractfeefromamount )\n\nSend an amount to a given address. The amount is a real and is rounded to the nearest 0.00000001\n\nArguments:\n1. \"bitcoinaddress\"  (string, required) The bitcoin address to 
send to.\n2. \"amount\"      (numeric, required) The amount in btc to send. eg 0.1\n3. \"comment\"     (string, optional) A comment used to store what the transaction is for. \n                             This is not part 
of the transaction, just kept in your wallet.\n4. \"comment-to\"  (string, optional) A comment to store the name of the person or organization \n                             to which you're sending the transaction. This is 
not part of the \n                             transaction, just kept in your wallet.\n5. subtractfeefromamount  (boolean, optional, default=false) The fee will be deducted from the amount being sent.\n                             
The recipient will receive less bitcoins than you enter in the amount field.\n\nResult:\n\"transactionid\"  (string) The transaction id.\n\nExamples:\n> bitcoin-cli sendtoaddress 
\"1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd\" 0.1\n> bitcoin-cli sendtoaddress \"1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd\" 0.1 \"donation\" \"seans outpost\"\n> bitcoin-cli 
sendtoaddress \"1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd\" 0.1 \"\" \"\" true\n> curl --user myusername --data-binary '{\"jsonrpc\": \"1.0\", \"id\":\"curltest\", \"method\": \"sendtoaddress\", \"params\": 
[\"1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd\", 0.1, \"donation\", \"seans outpost\"] }' -H 'content-type: text/plain;' http://127.0.0.1:8332/\n"}

This PR would format the error message and error code so that it's better readable. Example after this PR:

jonasschnelli$ ./src/bitcoin-cli --regtest sendtoaddress mvDv4Z
error code: -1
error message:
sendtoaddress "bitcoinaddress" amount ( "comment" "comment-to" subtractfeefromamount )

Send an amount to a given address. The amount is a real and is rounded to the nearest 0.00000001

Arguments:
1. "bitcoinaddress"  (string, required) The bitcoin address to send to.
2. "amount"      (numeric, required) The amount in btc to send. eg 0.1
3. "comment"     (string, optional) A comment used to store what the transaction is for. 
                             This is not part of the transaction, just kept in your wallet.
4. "comment-to"  (string, optional) A comment to store the name of the person or organization 
                             to which you're sending the transaction. This is not part of the 
                             transaction, just kept in your wallet.
5. subtractfeefromamount  (boolean, optional, default=false) The fee will be deducted from the amount being sent.
                             The recipient will receive less bitcoins than you enter in the amount field.

Result:
"transactionid"  (string) The transaction id.

Examples:
> bitcoin-cli sendtoaddress "1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd" 0.1
> bitcoin-cli sendtoaddress "1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd" 0.1 "donation" "seans outpost"
> bitcoin-cli sendtoaddress "1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd" 0.1 "" "" true
> curl --user myusername --data-binary '{"jsonrpc": "1.0", "id":"curltest", "method": "sendtoaddress", "params": ["1M72Sfpbz1BPpXFHz9m3CdqATR44Jvaydd", 0.1, "donation", "seans outpost"] }' -H 'content-type: text/plain;' http://127.0.0.1:8332/

@jonasschnelli jonasschnelli force-pushed the 2015/07/bitcoin-cli-error-output branch from e7c244c to 9c7d0dd Compare Jul 7, 2015
@jgarzik
Copy link
Contributor

jgarzik commented Jul 7, 2015

ACK

@@ -190,6 +190,15 @@ int CommandLineRPC(int argc, char *argv[])
throw CConnectionFailed("server in warmup");
strPrint = "error: " + error.write();
Copy link
Member

@laanwj laanwj Jul 7, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe not print the whole object, when we print the nicely formatted error?

What has always held me back from doing this (sensible) thing is that scripts may be relying on bitcoin-cli's output for errors to be as it is.

Copy link
Contributor Author

@jonasschnelli jonasschnelli Jul 7, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line strPrint = "error: " + error.write(); get's overwritten at L198 when error.isObject() (currently always).
Hmm... if someone relies on a/the output of a RPC client application like bitcoin-cli he should definitively rethink his API design and probably directly use the JSON RPC API.

Copy link
Member

@laanwj laanwj Jul 7, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, fair enough. utACK.

Copy link
Contributor

@jgarzik jgarzik Jul 7, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree / @jonasschnelli

@jonasschnelli jonasschnelli force-pushed the 2015/07/bitcoin-cli-error-output branch from 9c7d0dd to f3a004a Compare Jul 7, 2015
@jonasschnelli jonasschnelli force-pushed the 2015/07/bitcoin-cli-error-output branch from f3a004a to 65ce021 Compare Jul 7, 2015
@laanwj
Copy link
Member

laanwj commented Jul 7, 2015

Travis error was unrelated (see #6391)

@laanwj laanwj merged commit 65ce021 into bitcoin:master Jul 8, 2015
1 check passed
laanwj added a commit that referenced this issue Jul 8, 2015
65ce021 [bitcoin-cli] improve error output (Jonas Schnelli)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants