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.

["Shannon Sterz" <s.sterz@proxmox.com>]

On Tue Jan 21, 2025 at 11:56 AM CET, Gabriel Goller wrote:
> 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.

small note: for pbs this is actually 70 chars, guess there might have
been a wikipage that wasn't updated [1].

[1]: https://pbs.proxmox.com/wiki/index.php/Developer_Documentation#Commits_and_Commit_Messages

>
> Thanks!
>
>
> _______________________________________________
> 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