public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Gabriel Goller <g.goller@proxmox.com>
To: "Laurențiu Leahu-Vlăducu" <l.leahu-vladucu@proxmox.com>
Cc: pbs-devel@lists.proxmox.com
Subject: Re: [pbs-devel] [PATCH proxmox-backup 1/1] backup-proxy and parallel_handler: Improved errors when panics occur and the panic message is a formatted (not static) string. This worked already for &str literals, but not for Strings.
Date: Tue, 21 Jan 2025 11:56:04 +0100	[thread overview]
Message-ID: <7kuacdx3nll4s2bbgskn4gmrxspex2kf3jxfowdm3lxk5qk2ua@72gbp2nsqiyo> (raw)
In-Reply-To: <20250121093612.43094-2-l.leahu-vladucu@proxmox.com>

On 21.01.2025 10:36, Laurențiu Leahu-Vlăducu wrote:
>diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
>index ce1be1c0..a7dab4ac 100644
>--- a/src/bin/proxmox-backup-proxy.rs
>+++ b/src/bin/proxmox-backup-proxy.rs
>@@ -424,7 +424,10 @@ async fn run_task_scheduler() {
>         match schedule_tasks().catch_unwind().await {
>             Err(panic) => match panic.downcast::<&str>() {
>                 Ok(msg) => eprintln!("task scheduler panic: {msg}"),
>-                Err(_) => eprintln!("task scheduler panic - unknown type"),
>+                Err(panic) => match panic.downcast::<String>() {
>+                    Ok(msg) => eprintln!("task scheduler panic: {msg}"),
>+                    Err(_) => eprintln!("task scheduler panic - cannot show error message due to unknown error type")

AFAIK this gets called after the logging init, so we can use
tracing::error for the log messages here.

>+                }
>             },
>             Ok(Err(err)) => eprintln!("task scheduler failed - {err:?}"),
>             Ok(Ok(_)) => {}
>diff --git a/src/tools/parallel_handler.rs b/src/tools/parallel_handler.rs
>index 17f70179..429a1f8b 100644
>--- a/src/tools/parallel_handler.rs
>+++ b/src/tools/parallel_handler.rs
>@@ -140,7 +140,13 @@ impl<I: Send + 'static> ParallelHandler<I> {
>                         "thread {} ({}) panicked: {}",
>                         self.name, i, panic_msg
>                     )),
>-                    Err(_) => msg_list.push(format!("thread {} ({}) panicked", self.name, i)),
>+                    Err(panic) => match panic.downcast::<String>() {
>+                        Ok(panic_msg) => msg_list.push(format!(
>+                            "thread {} ({}) panicked: {}",
>+                                self.name, i, panic_msg
>+                        )),
>+                        Err(_) => msg_list.push(format!("thread {} ({}) panicked", self.name, i))

A few variables can be be inlined here.

The downcast part looks ok to me.
Please keep the commit message title under 72 characters.

Thanks!


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

  reply	other threads:[~2025-01-21 10:56 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-21  9:36 [pbs-devel] [PATCH proxmox/proxmox-backup 0/1] Improve errors when panics occur and the panic message is a formatted (not static) string Laurențiu Leahu-Vlăducu
2025-01-21  9:36 ` [pbs-devel] [PATCH proxmox-backup 1/1] backup-proxy and parallel_handler: Improved errors when panics occur and the panic message is a formatted (not static) string. This worked already for &str literals, but not for Strings Laurențiu Leahu-Vlăducu
2025-01-21 10:56   ` Gabriel Goller [this message]
2025-01-21 13:14     ` Laurențiu Leahu-Vlăducu
2025-01-21 13:17     ` Shannon Sterz
2025-01-21  9:36 ` [pbs-devel] [PATCH proxmox 1/1] rest-server: " Laurențiu Leahu-Vlăducu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7kuacdx3nll4s2bbgskn4gmrxspex2kf3jxfowdm3lxk5qk2ua@72gbp2nsqiyo \
    --to=g.goller@proxmox.com \
    --cc=l.leahu-vladucu@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal