Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Remove or Make Cronos' Year 2499 Upper Limit Configurable #78

Open
DevJasperNL opened this issue Mar 19, 2025 · 4 comments
Open

Comments

@DevJasperNL
Copy link
Contributor

Currently, Cronos enforces a hardcoded upper limit for the year 2499.

As discussed in this issue, the purpose of this limit appears to be to prevent long looping for invalid expressions. From my understanding, the intention is not to safeguard against incorrect dates being returned.

However, since Cronos is intended to be a flexible, general-purpose library, I believe that in some cases, this limit might be too restrictive. If a user inputs a cron expression that wouldn't return a valid result, the user would likely adjust their cron expression accordingly. If a user wants to use an upper bound, there already is the GetOccurrences(DateTime fromUtc, DateTime toUtc) method that can be used.

I’m building a general-purpose library on top of Cronos, and I’d like to offer my users the flexibility to calculate dates as far ahead as the year 5000, for example, without running into this restriction.

Ideally, I’d propose removing the limit entirely. Alternatively, and in a way that’s also backward-compatible, we could introduce an optional DateTime? toUtc parameter to GetNextOccurrence. This would allow the user to opt out of the limit by passing a null value if they prefer not to apply it.

I’d love to hear your thoughts on this suggestion. If you agree, I’d be happy to submit a PR for either of these ideas.

@odinserj
Copy link
Member

The year limit was introduced to avoid looping through 9999 year for unreachable cron expressions (as they can be more difficult than 30th of February). I have another idea to solve this problem though – to set a relative limitation, and not the absolute one. For example, we can limit the GetOccurrence method to stop iterations when nothing was found within the 100 years, starting from the from date. What do you think about this solution?

P. S. Initially commented on a wrong issue

@DevJasperNL
Copy link
Contributor Author

My first question would be: how problematic is it really if someone has to loop through 9999 years to find out that their cron expression is invalid? Cronos would simply be executing what it was asked to do. The fact that the result may not align with a user's expectations shouldn’t necessarily be a concern. Introducing "invisible" safeguards could actually make Cronos less predictable, potentially leading to unexpected behavior and longer debugging times. Personally, I’d prefer allowing developers to impose their own upper bound rather than enforcing a hardcoded limit.

My second question is whether it's possible for a valid cron expression to have an interval greater than 100 years. I can't think of one, but given how complex cron expressions can be, I’m not entirely sure (for example, I wasn’t able to fully grasp DeBruijnPositions in the code). If we can be 100% certain that such an expression doesn't exist, then I think your proposed solution is a good one. However, if such an expression is possible, then my first question applies.

That said, I do think your proposed solution is an improvement over the current approach. It still provides developers with a workaround for the limit by allowing multiple calls to GetNextOccurrence with 100-year intervals.

What do you think?

@odinserj
Copy link
Member

We can measure how long it takes to iterate an unreachable expression through the year 9999, for the following cases:

  • February 30th (for example)
  • One or two other cases available in tests

As far as I remember, the numbers are too high.

Yes, we must ensure first whether limiting iterations to N year 100% reliable. The current limitation is better in case of any doubts, because it is explicit.

@DevJasperNL
Copy link
Contributor Author

I just set MaxYear to 9999 and executed GetNextOccurrence_ReturnsNull_WhenCronExpressionIsUnreachable. It still executed pretty much instantly. I didn't measure it though and whatever is acceptable is up to you.

Instead of a relative limit however, it could also be an idea to just make MaxYear a property of CronExpression (with a default of 2499) instead of a constant. That way it is backwards compatable and people can just set it to 10000 if they don't want a limit. It would also be easier to implement than a relative limit.

What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants