Merge remote-tracking branch 'origin/master' into ens/sdk-1940-dfx-ne…

…w-streamline-output

CHANGELOG.md

@@ -162,6 +162,8 @@ Total query response payload size: 0 Bytes
 Log visibility: controllers
 ```
 
+### feat!: Print error traces only in verbose (`-v`) mode or if no proper error message is available
+
 ## Dependencies
 
 ### Frontend canister

e2e/tests-dfx/basic-project.bash

@@ -79,7 +79,8 @@ teardown() {
   assert_eq "4449444c00017d02"
 
   assert_command_fail dfx canister call --query hello_backend inc
-  assert_match "Not a query method."
+  assert_match "inc is an update method, not a query method."
+  assert_match "Run the command without '--query'."
 
 
   dfx canister call hello_backend inc

e2e/tests-dfx/call.bash

@@ -311,7 +311,7 @@ function impersonate_sender() {
 
     # updating settings now fails because the default identity does not control the canister anymore
     assert_command_fail dfx canister update-settings hello_backend --freezing-threshold 0
-    assert_contains "Only controllers of canister $CANISTER_ID can call ic00 method update_settings"
+    assert_contains "The principal you are using to call a management function is not part of the controllers."
 
     # updating settings succeeds when impersonating the management canister as the sender
     assert_command dfx canister update-settings hello_backend --freezing-threshold 0 --impersonate "${IDENTITY_PRINCIPAL}"
@@ -322,7 +322,7 @@ function impersonate_sender() {
 
     # canister status fails because the default identity does not control the canister anymore
     assert_command_fail dfx canister status hello_backend
-    assert_contains "Only controllers of canister $CANISTER_ID can call ic00 method canister_status"
+    assert_contains "The principal you are using to call a management function is not part of the controllers."
 
     # canister status succeeds when impersonating the management canister as the sender
     assert_command dfx canister status hello_backend --impersonate "${IDENTITY_PRINCIPAL}"

src/dfx/src/commands/canister/call.rs

@@ -337,8 +337,11 @@ To figure out the id of your wallet, run 'dfx identity get-wallet (--network ic)
     let cycles = opts.with_cycles.unwrap_or(0);
 
     if call_sender == &CallSender::SelectedId && cycles != 0 {
-        return Err(DiagnosedError::new("It is only possible to send cycles from a canister.".to_string(), "To send the same function call from your wallet (a canister), run the command using 'dfx canister call <other arguments> (--network ic) --wallet <wallet id>'.\n\
-        To figure out the id of your wallet, run 'dfx identity get-wallet (--network ic)'.".to_string())).context("Function caller is not a canister.");
+        let explanation = "It is only possible to send cycles from a canister.";
+        let action_suggestion = "To send the same function call from your wallet (a canister), run the command using 'dfx canister call <other arguments> (--network ic) --wallet <wallet id>'.\n\
+        To figure out the id of your wallet, run 'dfx identity get-wallet (--network ic)'.";
+        return Err(DiagnosedError::new(explanation, action_suggestion))
+            .context("Function caller is not a canister.");
     }
 
     if is_query {

src/dfx/src/commands/canister/update_settings.rs

@@ -137,8 +137,8 @@ pub async fn exec(
         if threshold_in_seconds > 50_000_000 /* ~1.5 years */ && !opts.confirm_very_long_freezing_threshold
         {
             return Err(DiagnosedError::new(
-                "The freezing threshold is defined in SECONDS before the canister would run out of cycles, not in cycles.".to_string(),
-                "If you truly want to set a freezing threshold that is longer than a year, please run the same command, but with the flag --confirm-very-long-freezing-threshold to confirm you want to do this.".to_string(),
+                "The freezing threshold is defined in SECONDS before the canister would run out of cycles, not in cycles.",
+                "If you truly want to set a freezing threshold that is longer than a year, please run the same command, but with the flag --confirm-very-long-freezing-threshold to confirm you want to do this.",
             )).context("Misunderstanding is very likely.");
         }
     }

src/dfx/src/lib/diagnosis.rs

@@ -10,18 +10,12 @@ use regex::Regex;
 use std::path::Path;
 use thiserror::Error as ThisError;
 
-/// Contains two Option<Strings> that can be displayed to the user:
-///   - Error explanation: Goes into a bit of detail on what the error is and/or where the user can find out more about it.
-///   - Action suggestion: Tells the user how to move forward to resolve the error.
-pub type Diagnosis = (Option<String>, Option<String>);
-pub const NULL_DIAGNOSIS: Diagnosis = (None, None);
-
-#[derive(ThisError, Debug)]
+#[derive(ThisError, Debug, Default, Clone)]
 /// If you do not need the generic error diagnosis to run, you can add a DiagnosedError with .context(err: DiagnosedError).
 /// In that case, no extra diagnosis is attempted and the last-added explanation and suggestion are printed out.
 pub struct DiagnosedError {
     /// A user-friendly explanation of what went wrong.
-    pub error_explanation: Option<String>,
+    pub explanation: Option<String>,
 
     /// Suggestions for the user on how to move forward to recover from the error.
     pub action_suggestion: Option<String>,
@@ -34,21 +28,22 @@ impl std::fmt::Display for DiagnosedError {
 }
 
 impl DiagnosedError {
-    pub fn new(error_explanation: String, action_suggestion: String) -> Self {
+    pub fn new(explanation: impl Into<String>, action_suggestion: impl Into<String>) -> Self {
         Self {
-            error_explanation: Some(error_explanation),
-            action_suggestion: Some(action_suggestion),
+            explanation: Some(explanation.into()),
+            action_suggestion: Some(action_suggestion.into()),
         }
     }
+
+    pub fn contains_diagnosis(&self) -> bool {
+        self.action_suggestion.is_some() || self.explanation.is_some()
+    }
 }
 
 /// Attempts to give helpful suggestions on how to resolve errors.
-pub fn diagnose(err: &AnyhowError) -> Diagnosis {
+pub fn diagnose(err: &AnyhowError) -> DiagnosedError {
     if let Some(diagnosed_error) = err.downcast_ref::<DiagnosedError>() {
-        return (
-            diagnosed_error.error_explanation.clone(),
-            diagnosed_error.action_suggestion.clone(),
-        );
+        return diagnosed_error.clone();
     }
 
     if let Some(agent_err) = err.downcast_ref::<AgentError>() {
@@ -84,7 +79,7 @@ pub fn diagnose(err: &AnyhowError) -> Diagnosis {
         }
     }
 
-    NULL_DIAGNOSIS
+    DiagnosedError::default()
 }
 
 fn local_replica_not_running(err: &AnyhowError) -> bool {
@@ -127,11 +122,12 @@ fn not_a_controller(err: &AgentError) -> bool {
     }
 
     // Older replicas do not include the error code in the reject response.
-    // differing behavior between replica and ic-ref:
-    // replica gives HTTP403, ic-ref gives HTTP400 with message "Wrong sender"
-    matches!(err, AgentError::HttpError(payload) if payload.status == 403)
-        || matches!(err, AgentError::HttpError(payload) if payload.status == 400 &&
-            matches!(std::str::from_utf8(payload.content.as_slice()), Ok("Wrong sender")))
+    // replica gives HTTP403, message looks like "Only controllers of canister bkyz2-fmaaa-aaaaa-qaaaq-cai can call ic00 method canister_status"
+    matches!(err, AgentError::HttpError(payload) if payload.status == 403
+    && matches!(
+        std::str::from_utf8(payload.content.as_slice()),
+        Ok("Only controllers of canister")
+    ))
 }
 
 fn wallet_method_not_found(err: &AgentError) -> bool {
@@ -150,8 +146,8 @@ fn wallet_method_not_found(err: &AgentError) -> bool {
     }
 }
 
-fn diagnose_http_403() -> Diagnosis {
-    let error_explanation = "Each canister has a set of controllers. Only those controllers have access to the canister's management functions (like install_code or stop_canister).\n\
+fn diagnose_http_403() -> DiagnosedError {
+    let explanation = "Each canister has a set of controllers. Only those controllers have access to the canister's management functions (like install_code or stop_canister).\n\
         The principal you are using to call a management function is not part of the controllers.";
     let action_suggestion = "To make the management function call succeed, you have to make sure the principal that calls the function is a controller.
 To see the current controllers of a canister, use the 'dfx canister info (--network ic)' command.
@@ -162,26 +158,21 @@ To add a principal to the list of controllers, one of the existing controllers h
 If your wallet is a controller, but not your own principal, then you have to make your wallet perform the call by adding '--wallet <your wallet id>' to the command.
 
 The most common way this error is solved is by running 'dfx canister update-settings --network ic --wallet \"$(dfx identity get-wallet)\" --all --add-controller \"$(dfx identity get-principal)\"'.";
-    (
-        Some(error_explanation.to_string()),
-        Some(action_suggestion.to_string()),
-    )
+    DiagnosedError::new(explanation, action_suggestion)
 }
 
-fn diagnose_local_replica_not_running() -> Diagnosis {
-    let error_explanation =
+fn diagnose_local_replica_not_running() -> DiagnosedError {
+    let explanation =
         "You are trying to connect to the local replica but dfx cannot connect to it.";
     let action_suggestion =
         "Target a different network or run 'dfx start' to start the local replica.";
-    (
-        Some(error_explanation.to_string()),
-        Some(action_suggestion.to_string()),
-    )
+    DiagnosedError::new(explanation, action_suggestion)
 }
 
-fn subnet_not_authorized() -> Diagnosis {
+fn subnet_not_authorized() -> DiagnosedError {
+    let explanation = "Subnet is not authorized to respond for the requested canister id.";
     let action_suggestion = "If you are connecting to a node directly instead of a boundary node, try using --provisional-create-canister-effective-canister-id with a canister id in the subnet's canister range. First non-root subnet: 5v3p4-iyaaa-aaaaa-qaaaa-cai, second non-root subnet: jrlun-jiaaa-aaaab-aaaaa-cai";
-    (None, Some(action_suggestion.to_string()))
+    DiagnosedError::new(explanation, action_suggestion)
 }
 
 fn duplicate_asset_key_dist_and_src(sync_error: &SyncError) -> bool {
@@ -208,7 +199,7 @@ fn duplicate_asset_key_dist_and_src(sync_error: &SyncError) -> bool {
     )
 }
 
-fn diagnose_duplicate_asset_key_dist_and_src() -> Diagnosis {
+fn diagnose_duplicate_asset_key_dist_and_src() -> DiagnosedError {
     let explanation = "An asset key was found in both the dist and src directories.
 One or both of the following are a likely explanation:
     - webpack.config.js is configured to copy assets from the src directory to the dist/ directory.
@@ -227,10 +218,10 @@ One or both of the following are a likely explanation:
 
 See also release notes: https://forum.dfinity.org/t/dfx-0-11-0-is-promoted-with-breaking-changes/14327"#;
 
-    (Some(explanation.to_string()), Some(suggestion.to_string()))
+    DiagnosedError::new(explanation, suggestion)
 }
 
-fn diagnose_bad_wallet() -> Diagnosis {
+fn diagnose_bad_wallet() -> DiagnosedError {
     let explanation = "\
 A wallet has been previously configured (e.g. via `dfx identity set-wallet`).
 However, it did not contain a function that dfx was looking for.
@@ -245,41 +236,41 @@ wrong, you can set a new wallet with
 `dfx identity set-wallet <PRINCIPAL> --identity <IDENTITY>`.
 If you're using a local replica and configuring a wallet was a mistake, you can
 recreate the replica with `dfx stop && dfx start --clean` to start over.";
-    (Some(explanation.to_string()), Some(suggestion.to_string()))
+    DiagnosedError::new(explanation, suggestion)
 }
 
 fn cycles_ledger_not_found(err: &AnyhowError) -> bool {
     err.to_string()
         .contains("Canister um5iw-rqaaa-aaaaq-qaaba-cai not found")
 }
 
-fn diagnose_cycles_ledger_not_found() -> Diagnosis {
+fn diagnose_cycles_ledger_not_found() -> DiagnosedError {
     let explanation =
         "Cycles ledger with canister ID 'um5iw-rqaaa-aaaaq-qaaba-cai' is not installed.";
     let suggestion =
         "Run the command with '--ic' flag if you want to manage the cycles on the mainnet.";
 
-    (Some(explanation.to_string()), Some(suggestion.to_string()))
+    DiagnosedError::new(explanation, suggestion)
 }
 
 fn ledger_not_found(err: &AnyhowError) -> bool {
     err.to_string()
         .contains("Canister ryjl3-tyaaa-aaaaa-aaaba-cai not found")
 }
 
-fn diagnose_ledger_not_found() -> Diagnosis {
+fn diagnose_ledger_not_found() -> DiagnosedError {
     let explanation = "ICP Ledger with canister ID 'ryjl3-tyaaa-aaaaa-aaaba-cai' is not installed.";
     let suggestion =
         "Run the command with '--ic' flag if you want to manage the ICP on the mainnet.";
 
-    (Some(explanation.to_string()), Some(suggestion.to_string()))
+    DiagnosedError::new(explanation, suggestion)
 }
 
 fn insufficient_cycles(err: &CreateCanisterError) -> bool {
     matches!(err, CreateCanisterError::InsufficientFunds { balance: _ })
 }
 
-fn diagnose_insufficient_cycles() -> Diagnosis {
+fn diagnose_insufficient_cycles() -> DiagnosedError {
     let network = match get_network_context() {
         Ok(value) => {
             if value == "local" {
@@ -297,5 +288,5 @@ fn diagnose_insufficient_cycles() -> Diagnosis {
 'dfx cycles convert --amount=0.123{}'",
         network
     );
-    (Some(explanation.to_string()), Some(suggestion))
+    DiagnosedError::new(explanation, suggestion)
 }

src/dfx/src/lib/operations/canister/motoko_playground.rs

@@ -228,10 +228,10 @@ pub async fn playground_install_code(
         .await
         .map_err(|err| {
             if is_asset_canister && err.to_string().contains("Wasm is not whitelisted") {
-                anyhow!(DiagnosedError {
-                    error_explanation: Some("The frontend canister wasm needs to be allowlisted in the playground but it isn't. This is a mistake in the release process.".to_string()),
-                    action_suggestion: Some("Please report this on forum.dfinity.org and mention your dfx version. You can get the version with 'dfx --version'.".to_string()),
-                })
+                anyhow!(DiagnosedError::new(
+                    "The frontend canister wasm needs to be allowlisted in the playground but it isn't. This is a mistake in the release process.",
+                    "Please report this on forum.dfinity.org and mention your dfx version. You can get the version with 'dfx --version'."
+                ))
             } else {
                 anyhow!(err)
             }

src/dfx/src/lib/operations/ledger.rs

@@ -191,7 +191,7 @@ Your principal for ICP wallets and decentralized exchanges: {}
         principal.to_text()
     );
 
-    DiagnosedError::new(explanation.to_string(), suggestion)
+    DiagnosedError::new(explanation, suggestion)
 }
 
 fn retryable(agent_error: &AgentError) -> bool {