[bug] Rust panic in tauri_plugin_fs_extra from crash reporting in our installbase

[abose]

Describe the bug

We've recently integrated Bugsnag for crash reporting in our app phcode.io. This report summarizes a recurring crash issue observed in macOS.

Overview

• Platform Affected: macOS only
• Deployment Date: About 2 days ago, 12-Sep-2024
• Total Sessions Since Deployment: 13,000 sessions
• Total Crashes Reported: 173 incidents of the same crash type

Crash Details

• Crash Type: rust panic, probably at app start itself.
• macOS only
• Function Causing Crash: tauri_plugin_fs_extra::system_time_to_ms. The crashes are pointing to this single function in all stacks. See stack below.
• We do not capture or report the full file paths in the crash stack for privacy(potentially involving usernames in the paths). Sorry for not having more info on file paths.

Reproduction

This is crash reporting from our installbase.
This looks like the offending code: https://github.com/tauri-apps/tauri-plugin-fs-extra/blob/v1/src/lib.rs#L80

Expected behavior

no crash

Full tauri info output

N/A production crashlitics

Stack trace

Rust_Panic
Unknown panic message
Unknown panic message
Sept 12th 2024, 22:29:13 GMT+5:30

STACKTRACE

Rust_Panic Unknown panic message 
     (path redacted for privacy) phoenix_code_ide::bugsnag::handle::hf6fa730ab2ba5fdc
     (path redacted for privacy) phoenix_code_ide::main::{{closure}}::h63ef96d811aaafb2
     (path redacted for privacy) std::panicking::rust_panic_with_hook::h09e8a656f11e82b2
     (path redacted for privacy) std::panicking::begin_panic_handler::{{closure}}::h1230eb3cc91b241c
     (path redacted for privacy) std::sys::backtrace::__rust_end_short_backtrace::hc3491307aceda2c2
     (path redacted for privacy) _rust_begin_unwind
     (path redacted for privacy) core::panicking::panic_fmt::ha4b80a05b9fff47a
     (path redacted for privacy) core::result::unwrap_failed::h441932a0bca0dd7f
     (path redacted for privacy) tauri_plugin_fs_extra::system_time_to_ms::h397a5ccb275f176e
     (path redacted for privacy) <futures_util::future::future::map::Map<Fut,F> as core::future::future::Future>::poll::h37c3e63198041c1c
     (path redacted for privacy) <futures_util::future::future::map::Map<Fut,F> as core::future::future::Future>::poll::ha3a0836894dd9ab6
     (path redacted for privacy) tauri::hooks::InvokeResolver<R>::respond_async_serialized::{{closure}}::h3f627abb3f53c634
     (path redacted for privacy) tokio::runtime::task::core::Core<T,S>::poll::ha10d84077f79d4ae
     (path redacted for privacy) tokio::runtime::task::harness::Harness<T,S>::poll::h149b233a188d48fd
     (path redacted for privacy) tokio::runtime::scheduler::multi_thread::worker::Context::run_task::h9aa34f3a4661eb13
     (path redacted for privacy) tokio::runtime::scheduler::multi_thread::worker::Context::run::h787319f1d7277b47
     (path redacted for privacy) tokio::runtime::context::runtime::enter_runtime::h50bae69fda092374
     (path redacted for privacy) tokio::runtime::scheduler::multi_thread::worker::run::hcc941a10239ee947
     (path redacted for privacy) tokio::runtime::task::core::Core<T,S>::poll::h16d0024f82d9c144
     (path redacted for privacy) tokio::runtime::task::harness::Harness<T,S>::poll::hfda8362637abae26
     (path redacted for privacy) tokio::runtime::blocking::pool::Inner::run::hb8b3a7ae99ef937a
     (path redacted for privacy) std::sys::backtrace::__rust_begin_short_backtrace::h25d22e0eed28a485
     (path redacted for privacy) core::ops::function::FnOnce::call_once{{vtable.shim}}::h79d92fea277cd232
     (path redacted for privacy) std::sys::pal::unix::thread::Thread::new::thread_start::h1bd1b9c95010bf71
     (path redacted for privacy) __pthread_joiner_wake

Additional context

[abose]

Le chatgpt O1 says:
It appears that you're encountering a panic in the system_time_to_ms function of the tauri-plugin-fs-extra crate. The panic is likely caused by unwrapping a Result without handling the potential Err case when converting a SystemTime to milliseconds since the UNIX epoch.

Here's the relevant portion of the code from line 80 in lib.rs:

fn system_time_to_ms(time: SystemTime) -> u128 {
    time.duration_since(SystemTime::UNIX_EPOCH).unwrap().as_millis()
}

Issue:

• The duration_since method returns a Result<Duration, SystemTimeError>.
• If time is before the UNIX_EPOCH, duration_since will return an Err.
• Calling .unwrap() on an Err value causes a panic.

Solution:

Modify the function to handle the Result properly instead of unwrapping it. You have two main options:

1. Handle the Error Inside the Function

   Return a default value (e.g., 0) or handle the error appropriately within the function.

   fn system_time_to_ms(time: SystemTime) -> u128 {
       match time.duration_since(SystemTime::UNIX_EPOCH) {
           Ok(duration) => duration.as_millis(),
           Err(_) => {
               // Handle the error (e.g., return 0 or log the error)
               0
           }
       }
   }

2. Propagate the Error to the Caller

   Change the function signature to return a Result<u128, SystemTimeError> so that the caller can handle the error.

   use std::time::{SystemTime, SystemTimeError};
   
   fn system_time_to_ms(time: SystemTime) -> Result<u128, SystemTimeError> {
       time.duration_since(SystemTime::UNIX_EPOCH).map(|duration| duration.as_millis())
   }

   Usage:

   match system_time_to_ms(some_system_time) {
       Ok(ms) => {
           // Use the milliseconds
           println!("Milliseconds since UNIX_EPOCH: {}", ms);
       }
       Err(e) => {
           // Handle the error
           eprintln!("Error converting time: {:?}", e);
       }
   }

Recommendation:

Option 2 is generally better practice as it doesn't hide potential errors and allows the caller to decide how to handle them.

Explanation:

• duration_since Method: Calculates the duration between the given SystemTime and UNIX_EPOCH.
• Error Handling: By using match or Result propagation, you ensure that your program can handle times before the UNIX epoch without panicking.
• as_millis Method: Converts the Duration to milliseconds, returning a u128.

Additional Steps:

1. Update the Function Signature (if propagating the error):

   Ensure all places where system_time_to_ms is called are updated to handle the Result.

2. Test with Different Times:

   Test the function with SystemTime::now() and with times before the UNIX epoch to ensure it behaves as expected.

   // Time after UNIX_EPOCH
   let now = SystemTime::now();
   println!("{:?}", system_time_to_ms(now));
   
   // Time before UNIX_EPOCH
   let before_epoch = UNIX_EPOCH - Duration::from_secs(1000);
   println!("{:?}", system_time_to_ms(before_epoch));

Why This Error Occurred:

• SystemTime Before UNIX_EPOCH: If any file or system time you're working with is dated before January 1, 1970 (the UNIX epoch), duration_since will fail.
• Possible Causes: Incorrect system clock, files with incorrect timestamps, or using UNIX_EPOCH - duration without handling.

Preventing Future Errors:

• Validate Inputs: Ensure that the SystemTime values you're working with are valid and expected.
• Robust Error Handling: Always handle Result types properly to prevent panics.
• Logging: Implement logging for error cases to aid in debugging.

Example with Error Logging:

use std::time::{SystemTime, SystemTimeError};

fn system_time_to_ms(time: SystemTime) -> Result<u128, SystemTimeError> {
    match time.duration_since(SystemTime::UNIX_EPOCH) {
        Ok(duration) => Ok(duration.as_millis()),
        Err(e) => {
            eprintln!("Error converting time: {:?}", e);
            Err(e)
        }
    }
}

Conclusion:

By properly handling the Result from duration_since, you can prevent the panic and make your code more robust. Choose the error handling strategy that best fits your application's needs.

---

Note: Always ensure that you're handling time-related functions carefully, especially when dealing with files or systems that might have timestamps before the UNIX epoch.

[FabianLars]

Should be fixed with the linked pr above (non default branch so no auto closing...)