fix(sentry-types): Switch to checked version of from_secs_f64
#554
Conversation
This change make use of `try_from_secs_f64` (the checked version of `from_secs_f64`), avoding panics if the incoming seconds is negative, overflows Duration or not finite. Only one gotcha here, is that this function was added only in rust `1.66.0`, which is above current MSRV.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #554 +/- ##
==========================================
+ Coverage 70.59% 70.62% +0.03%
==========================================
Files 66 66
Lines 6658 6665 +7
==========================================
+ Hits 4700 4707 +7
Misses 1958 1958 |
I’m surprised that CI does not complain though, its using |
|
It was stabilized in 1.66, not added, so I wonder if we have incorrect compilation flags set somewhere |
That's what I meant 😅 thanks! How do you want to proceed with this? Maybe change the implementation a bit and for now just use previous function with additional checks on top of it to avoid panics? Since this one caused an incident for us getsentry/relay#1872 |
Before converting an intoming `f64` to `Duration` using `from_secs_f64`, check if if the value fits into `Duration` type and the function won't panic. Starting from Rust `1.66.0` it is possible to use `Duration::try_from_secs_f64` which returns Option and does not panic.
|
How about fix from c987539 which does not use the new function, but just has additional checks in-place to make sure we do not panic. |
|
@olksdr feel free to revert it back to your original fix, we bumped MSRV :) |
This change make use of
try_from_secs_f64(the checked version offrom_secs_f64), avoiding panics if the incoming seconds is negative, overflows Duration or not finite.Only one gotcha here, is that this function was added only in rust
1.66.0, which is above current MSRV.