-
Notifications
You must be signed in to change notification settings - Fork 120
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
Add --output-json
for call
, instantiate
& upload
commands
#722
Conversation
Currently, JSON is only displayed when if the operation has been successful. Should JSON response be introduced for errors? |
That would be ideal, then third-party tooling can make better use of For context: The PR has conflicts with |
UpdateError are now partially json-rendered. Only Example
P.S.After short conversation with @ascjones It was agreed that RPC error does not have to and can not be serialised properly
|
UpdateModule and Generic errors are now serialised generically
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WIP review
* Convert subxt error to custom ErrorVariant * Refactor displaying of extrinsic result events
crates/cargo-contract/src/main.rs
Outdated
"ERROR:".bright_red().bold(), | ||
format!("{:?}", err).bright_red() | ||
); | ||
if !suppress_err { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we actually want to report the error as json rather than supressing it. At the moment for some errors we will get the pretty printed json at the call site, and all others will not be displayed at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If don't do this, then the output result can be like this:
{
"module_error": {
"pallet": "Contracts",
"error": "DuplicateContract",
"docs": [
"A contract with the same AccountId already exists."
]
}
}
{
"error": "Pre-submission dry-run failed. Use --skip-dry-run to skip this step."
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then we will need to rethink the approach. How about instead of printing the json error directly, we bubble it back up as an error?.
So instead of eprintnln!("{}", json_error); Ok(())
you could do (Err(anyhow!("{}", json_err))
. Then this would be printed correctly at the top level
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made the error propagate to the top level, all the way back to the main()
, then I check if an error is an json format and print out, otherwise I wrap the message into a serializable object.
crates/cargo-contract/src/main.rs
Outdated
// error message can be either plain text or json string | ||
// we need to check whether the error message is json object | ||
let json_result = | ||
serde_json::from_str::<serde_json::Value>(&err.to_string()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not great to serialize to json at the error site and then deserialize again at the top level. I think we can do better with a strongly typed approach see #736
--output-json
for call
and instantiate
commands--output-json
for call
, instantiate
& upload
commands
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Summary
As requested this PR closes #682, this PR adds support for printing out response from calling
cargo contract instantiate ...
. andcargo contract call ...
outputs in JSON format by introducing--output-json
flag.Previous human-readable format has been preserved.
Examples
Calling a contract with arg
Calling a contract with
--dry-run
flagInstantiating contract
Instantiating a contract with dry-run
Updates
StorageDeposit
changes