chronoutil icon indicating copy to clipboard operation
chronoutil copied to clipboard

Panic when shifting to a DST transition

Open ebegumisa opened this issue 1 year ago • 5 comments

Firstly, thanks for the library.

Problem

I'm in a timezone which regrettably uses daylight saving tIme.

When taking a chrono DST timezone datetime and attempting to apply chronoutils shift utility functions (either directly or indirectly via a relative addition), chronoutils panics if the computed datetime lands on a DST transition.

Examples

    use chrono::{TimeZone, LocalResult};

    #[test] 
    fn test_shift_months_datetime_to_dst_backward_transition() {
        
        let dst_tz = &chrono_tz::Australia::Melbourne;
        
        // On Apr 5th 2020 after 02:59:59, clocks were wound back to 02:00:00 making 02:00::00 to
        // 02:59:59 ambiguous.
        // <https://www.timeanddate.com/time/change/australia/melbourne?year=2020>
        
        base = dst_tz.with_ymd_and_hms(2020, 3, 5, 2, 00, 0).single().unwrap();
        shift_months(base, 1); 
     // ^^^^^^^^^^^^^^^^^^^^^ panics due to shift to DST ambiguous datetime
    }

    #[test]
    fn test_shift_months_datetime_to_dst_forward_transition() {
        
        let dst_tz = &chrono_tz::Australia::Melbourne;

        // On Oct 4th 2020 after 01:59:59, clocks were advanced to 03:00:00 making 02:00:00 to 
        // 02:59:59 non-existent.
        // <https://www.timeanddate.com/time/change/australia/melbourne?year=2020>   
        
        let base = dst_tz.with_ymd_and_hms(2020, 9, 4, 2, 00, 0).single().unwrap();
        shift_months(base, 1); 
     // ^^^^^^^^^^^^^^^^^^^^^ panics due to shift to DST non-existent datetime
    }

Cause

These panics are due to the shift utils unwrapping calls to the chrono::Datelike::with_* functions, with the latter's doc currently stating:

Returns None when:
  • The resulting date does not exist (for example day(31) in April).
  • In case of DateTime<Tz> if the resulting date and time fall within a timezone transition such as from DST to standard time.
  • The value for day is out of range.
pub fn shift_months<D: Datelike>(date: D, months: i32) -> D {

...

   if day <= 28 {
        date.with_day(day)
            .unwrap() 
          // ^^^^^^^^ could panic due to DST
            .with_month(month as u32)
            .unwrap() // <<< Ditto
            .with_year(year)
            .unwrap() // <<< Ditto
    } else {
        date.with_day(1)
            .unwrap() // <<< Ditto
            .with_month(month as u32)
            .unwrap() // <<< Ditto
            .with_year(year)
            .unwrap() // <<< Ditto
            .with_day(day)
            .unwrap() // <<< Ditto
    }
...

Proposed Solution

It would be helpful to have *_opt versions of the shift utils which return an Option rather than a Datelike, for example

pub fn shift_months_opt( .. ) -> Option<Datelike>

This way, the caller can choose how to handle DST issues. A returned None won't help the caller distinguish nonsensical input args from args which result in a DST transition, but callers should be able to determine that for themselves.

ebegumisa avatar Sep 27 '23 07:09 ebegumisa

I am working an a pull request to resolve this issue.

ebegumisa avatar Sep 27 '23 07:09 ebegumisa

Hey, thanks for the comprehensive issue.

Ah, that's annoying that the fold is not handled by chrono (usually the way you resolve this with datetime types is by having a "fold" attribute which is either 0 or 1, depending on if it's the first or second 2am).

A shift_months_opt sounds like a good start. The larger solution will probably be to emulate the way that chrono handles the addition of regular duration to a datetime around the fold. If you can get shift_months_opt looking good I'm happy to add them and will investigate investigate improving the RelativeDuration for the 0.3 release

olliemath avatar Sep 27 '23 12:09 olliemath

Thanks @olliemath,

I've sent the PR #12 adding the shift_months_opt and related functions.

ebegumisa avatar Sep 27 '23 13:09 ebegumisa

shift_months_opt is now published under 0.2.6 - I'm going to keep this issue open to track progress on RelativeDelta - which I'll plan for 0.3.0

olliemath avatar Sep 29 '23 11:09 olliemath

shift_months_opt is now published under 0.2.6 - I'm going to keep this issue open to track progress on RelativeDelta - which I'll plan for 0.3.0

Great @olliemath . Much appreciated.

ebegumisa avatar Sep 29 '23 11:09 ebegumisa