From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id B17D31FF164 for ; Fri, 22 Nov 2024 10:01:31 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A6D65D911; Fri, 22 Nov 2024 10:01:39 +0100 (CET) MIME-Version: 1.0 In-Reply-To: <20241121154337.471425-4-c.ebner@proxmox.com> References: <20241121154337.471425-1-c.ebner@proxmox.com> <20241121154337.471425-4-c.ebner@proxmox.com> From: Fabian =?utf-8?q?Gr=C3=BCnbichler?= To: Christian Ebner , pbs-devel@lists.proxmox.com Date: Fri, 22 Nov 2024 10:01:28 +0100 Message-ID: <173226608868.2118190.15465009393024345476@yuna.proxmox.com> User-Agent: alot/0.10 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.047 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pbs-devel] [PATCH proxmox-backup 3/4] server: push: add error context to all target api calls 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: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" Quoting Christian Ebner (2024-11-21 16:43:36) > Make it clear from the context that these error messages stem from > the response of an api call rather than a local error. > > Signed-off-by: Christian Ebner > --- > src/server/push.rs | 24 +++++++++++++++++------- > 1 file changed, 17 insertions(+), 7 deletions(-) > > diff --git a/src/server/push.rs b/src/server/push.rs > index 86cef5520..fe2e11220 100644 > --- a/src/server/push.rs > +++ b/src/server/push.rs > @@ -219,7 +219,9 @@ async fn remove_target_namespace( > if params.target.supports_prune_delete_stats { > let data = result["data"].take(); > serde_json::from_value(data).map_err(|err| { > - format_err!("removing target namespace {target_namespace} failed - {err}") > + format_err!( > + "Failed to remove remote namespace {target_namespace}, remote returned: {err}" > + ) this is attached to the wrong error - it should be attached to the client.delete call right above.. this here should instead add the context that we failed to parse the returned value (which should never happen, that means we missed some API breakage..) > }) > } else { > Ok(BackupGroupDeleteStats::default()) > @@ -236,7 +238,8 @@ async fn fetch_target_groups( > let args = Some(serde_json::json!({ "ns": target_namespace.name() })); > > let mut result = params.target.client.get(&api_path, args).await?; > - let groups: Vec = serde_json::from_value(result["data"].take())?; > + let groups: Vec = serde_json::from_value(result["data"].take()) > + .map_err(|err| format_err!("Failed to fetch remote groups, remote returned: {err}"))?; same here, just with get instead of delete ;) > > let (mut owned, not_owned) = groups.into_iter().fold( > (Vec::new(), HashSet::new()), > @@ -277,8 +280,9 @@ async fn remove_target_group( > > if params.target.supports_prune_delete_stats { > let data = result["data"].take(); > - serde_json::from_value(data) > - .map_err(|err| format_err!("removing target group {backup_group} failed - {err}")) > + serde_json::from_value(data).map_err(|err| { > + format_err!("Failed to remove remote group {backup_group}, remote returned: {err}") > + }) here as well > } else { > Ok(BackupGroupDeleteStats::default()) > } > @@ -313,7 +317,7 @@ async fn check_or_create_target_namespace( > match params.target.client.post(&api_path, Some(args)).await { > Ok(_) => info!("Successfully created new namespace {current} on remote"), > Err(err) => { > - bail!("Remote creation of namespace {current} failed, remote returned: {err}") > + bail!("Creation of remote namespace {current} failed, remote returned: {err}") > } > } > existing_target_namespaces.push(current.clone()); > @@ -585,7 +589,8 @@ async fn fetch_target_snapshots( > args["ns"] = serde_json::to_value(target_namespace)?; > } > let mut result = params.target.client.get(&api_path, Some(args)).await?; > - let snapshots: Vec = serde_json::from_value(result["data"].take())?; > + let snapshots: Vec = serde_json::from_value(result["data"].take()) > + .map_err(|err| format_err!("Failed to fetch remote snapshots, remote returned: {err}"))?; here as well > > Ok(snapshots) > } > @@ -603,7 +608,12 @@ async fn forget_target_snapshot( > if !target_namespace.is_root() { > args["ns"] = serde_json::to_value(target_namespace)?; > } > - params.target.client.delete(&api_path, Some(args)).await?; > + params > + .target > + .client > + .delete(&api_path, Some(args)) > + .await > + .map_err(|err| format_err!("Failed to remove remote snapshot, remote returned: {err}"))?; this should probably be just "Request to remote returned {err}", since the call site already logs the snapshot name and the fact that this is removal failing ;) > > Ok(()) > } > -- > 2.39.5 > > > > _______________________________________________ > pbs-devel mailing list > pbs-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel > > _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel