HangfireIO / Cronos

A fully-featured .NET library for working with Cron expressions. Built with time zones in mind and intuitively handles daylight saving time transitions
MIT License
1.01k stars 118 forks source link

What's the logic behind the unreachable dates? #51

Closed theodorzoulias closed 10 months ago

theodorzoulias commented 2 years ago

Hi! I am trying to familiarize myself with the scenarios in which the GetNextOccurrence can return a DateTime? with null value, and I have a hard time understanding the logic behind the behavior. It seems that calculating the next occurrence for dates after the year 2100 is unsuccessful about half of the time. Here is a minimal example:

string expression = "0 0 15 6 *"; // June, 15
var cronExpression = CronExpression.Parse(expression);
Console.WriteLine($"Expression: {cronExpression}");
for (int i = 1; i <= 24; i++)
{
    var fromUtc = new DateTime(2150, 1, 1, 0, 0, 0, DateTimeKind.Utc).AddMonths(i);
    var nextUtc = cronExpression.GetNextOccurrence(fromUtc);
    Console.WriteLine($"Date: {fromUtc}, NextOccurrence: {(nextUtc.HasValue ? nextUtc.Value.ToString() : "-")}");
}

Output:

Expression: 0 0 0 15 6 *
Date: 02/01/2150 00:00:00, NextOccurrence: 06/15/2150 00:00:00
Date: 03/01/2150 00:00:00, NextOccurrence: 06/15/2150 00:00:00
Date: 04/01/2150 00:00:00, NextOccurrence: 06/15/2150 00:00:00
Date: 05/01/2150 00:00:00, NextOccurrence: 06/15/2150 00:00:00
Date: 06/01/2150 00:00:00, NextOccurrence: 06/15/2150 00:00:00
Date: 07/01/2150 00:00:00, NextOccurrence: -
Date: 08/01/2150 00:00:00, NextOccurrence: -
Date: 09/01/2150 00:00:00, NextOccurrence: -
Date: 10/01/2150 00:00:00, NextOccurrence: -
Date: 11/01/2150 00:00:00, NextOccurrence: -
Date: 12/01/2150 00:00:00, NextOccurrence: -
Date: 01/01/2151 00:00:00, NextOccurrence: 06/15/2151 00:00:00
Date: 02/01/2151 00:00:00, NextOccurrence: 06/15/2151 00:00:00
Date: 03/01/2151 00:00:00, NextOccurrence: 06/15/2151 00:00:00
Date: 04/01/2151 00:00:00, NextOccurrence: 06/15/2151 00:00:00
Date: 05/01/2151 00:00:00, NextOccurrence: 06/15/2151 00:00:00
Date: 06/01/2151 00:00:00, NextOccurrence: 06/15/2151 00:00:00
Date: 07/01/2151 00:00:00, NextOccurrence: -
Date: 08/01/2151 00:00:00, NextOccurrence: -
Date: 09/01/2151 00:00:00, NextOccurrence: -
Date: 10/01/2151 00:00:00, NextOccurrence: -
Date: 11/01/2151 00:00:00, NextOccurrence: -
Date: 12/01/2151 00:00:00, NextOccurrence: -
Date: 01/01/2152 00:00:00, NextOccurrence: 06/15/2152 00:00:00

Live demo.

Could you explain why the calculation fails when the month+day of the starting date is larger than the month+day in the Cron expression?

Thanks!

odinserj commented 2 years ago

Cronos wasn't planned to calculate occurrences after 2099 year, there's a hardcoded values, please see the following snippet. Such an upper bound is required, because it's not possible to know that some cron expression leads to an unreachable date in upfront, and we need to avoid the infinite loop in this case.

https://github.com/HangfireIO/Cronos/blob/b35c34692f74cf59630be8184d12742306ab2b32/src/Cronos/CronExpression.cs#L42

I think there should be ArgumentOutOfRangeException thrown when start date's year is more than this hardcoded value.

theodorzoulias commented 2 years ago

@odinserj so by using the Cronos, the effective range of the DateTime type is reduced from 7,978 years to 78 years. This should be documented more prominently IMHO. It has the smell of the Y2K bug all over again.

lonix1 commented 2 years ago

In 78 years when my system fails, I'm coming back here to complain. I'll be angry, and there will probably be swearing... :wink: :stuck_out_tongue:

AraHaan commented 2 years ago

In 78 years, most of us will either be gone, or in wheelchairs when we are 90+ waiting for that time. Preferably with 0 care about the problem as the successors would have to worry about it.

odinserj commented 2 years ago

As far as I remember the reason was to avoid iterating each second through 9999 year when something like 30th of February is specified. Case with February is more or less simple, but I'm afraid there can be other unreachable cases that's much harder to detect. May be we can increase the max date to 2200 or 2500 and throw an exception when start date is out of bounds.

AraHaan commented 2 years ago

or you could change it to where it selects which way to iterate based on if the chron expression is for iterating:

And then only do it based on that?