From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 4320992265 for ; Fri, 16 Feb 2024 16:33:52 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 23DB7557B for ; Fri, 16 Feb 2024 16:33:22 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Fri, 16 Feb 2024 16:33:21 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id EE08D48585 for ; Fri, 16 Feb 2024 16:33:20 +0100 (CET) From: Gabriel Goller To: pbs-devel@lists.proxmox.com Date: Fri, 16 Feb 2024 16:33:08 +0100 Message-ID: <20240216153317.323260-2-g.goller@proxmox.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240216153317.323260-1-g.goller@proxmox.com> References: <20240216153317.323260-1-g.goller@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.101 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox-offline-mirror.rs, pool.rs, command.rs] Subject: [pbs-devel] [PATCH proxmox 1/4] CLI: print fatal errors including causes X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 16 Feb 2024 15:33:52 -0000 From: Fabian Grünbichler as a first step of improving our error handling story, printing context and causes if the error contains them. For example, with two calls to `.with_context` when bubbling up errors in proxmox-offline-mirror: > ----8<---- > diff --git a/src/bin/proxmox-offline-mirror.rs b/src/bin/proxmox-offline-mirror.rs > index bec366a..403a2f5 100644 > --- a/src/bin/proxmox-offline-mirror.rs > +++ b/src/bin/proxmox-offline-mirror.rs > @@ -1,7 +1,7 @@ > use std::fmt::Display; > use std::path::Path; > > -use anyhow::{bail, Error}; > +use anyhow::{bail, format_err, Context, Error}; > use serde_json::Value; > > use proxmox_router::cli::{run_cli_command, CliCommand, CliCommandMap, CliEnvironment}; > @@ -676,7 +676,8 @@ async fn setup(config: Option, _param: Value) -> Result<(), Error> { > Action::AddMirror => { > for mirror_config in action_add_mirror(&config)? { > let id = mirror_config.id.clone(); > - mirror::init(&mirror_config)?; > + mirror::init(&mirror_config) > + .with_context(|| format!("Failed to initialize mirror '{id}'"))?; > config.set_data(&id, "mirror", mirror_config)?; > save_config(&config_file, &config)?; > println!("Config entry '{id}' added"); > diff --git a/src/pool.rs b/src/pool.rs > index 3da8c08..ecf3f6f 100644 > --- a/src/pool.rs > +++ b/src/pool.rs > @@ -7,7 +7,7 @@ use std::{ > path::{Path, PathBuf}, > }; > > -use anyhow::{bail, format_err, Error}; > +use anyhow::{bail, format_err, Context, Error}; > use nix::{unistd, NixPath}; > > use proxmox_apt::deb822::CheckSums; > @@ -45,10 +45,12 @@ impl Pool { > } > > if !pool.exists() { > - create_path(pool, None, None)?; > + create_path(pool, None, None) > + .with_context(|| format!("Failed to create pool dir {pool:?}"))?; > } > > - create_path(link_dir, None, None)?; > + create_path(link_dir, None, None) > + .with_context(|| format!("Failed to create link dir {link_dir:?}"))?; > > Ok(Self { > pool_dir: pool.to_path_buf(), > ---->8---- we'd get the following output: Error: Failed to initialize mirror 'debian_bullseye_main' Caused by: 0: Failed to create pool dir "/var/lib/proxmox-offline-mirror/mirrors//.pool" 1: EACCES: Permission denied instead of the original Error: EACCESS: Permission denied which is not really helpful without knowing the path. For non-fatal cases or logging inside tasks, `{:#}` could be used which just prints the causes/contexts in a single line like this: Failed to initialize mirror 'debian_bullseye_main': Failed to create pool dir "/var/lib/proxmox-offline-mirror/mirrors//.pool": EACCES: Permission denied but for that usage, the context should be kept short to avoid the line getting overly long. One downside to adding context is that the default Display implementation will *just* print the context, which hides the root cause: Error: Failed to initialize mirror 'debian_bullseye_main' When adding context to existing error handling (or when replacing manual context adding via format_err!), call sites need to be adapted to ensure the causes are not accidentally hidden. Signed-off-by: Fabian Grünbichler Signed-off-by: Gabriel Goller --- proxmox-router/src/cli/command.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/proxmox-router/src/cli/command.rs b/proxmox-router/src/cli/command.rs index 7a26ffb..d5522f1 100644 --- a/proxmox-router/src/cli/command.rs +++ b/proxmox-router/src/cli/command.rs @@ -83,7 +83,7 @@ async fn handle_simple_command_future( } } Err(err) => { - eprintln!("Error: {}", err); + eprintln!("Error: {:?}", err); return Err(err); } } @@ -135,7 +135,7 @@ fn handle_simple_command( } } Err(err) => { - eprintln!("Error: {}", err); + eprintln!("Error: {:?}", err); return Err(err); } } -- 2.43.0