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

Infinite loop with day_of_week #469

Closed
localhostdotdev opened this issue Mar 15, 2019 · 5 comments · Fixed by #554
Closed

Infinite loop with day_of_week #469

localhostdotdev opened this issue Mar 15, 2019 · 5 comments · Fixed by #554

Comments

@localhostdotdev
Copy link

Essentially doing this makes an infinite loop: day_of_week(monday: [0])

Full reproduction test case:

schedule = IceCube::Schedule.new(Time.zone.now)
schedule.add_recurrence_rule(IceCube::Rule.daily.day_of_week(monday: [0]))
schedule.first
@localhostdotdev localhostdotdev changed the title Infinite loops with day_of_week Infinite loop with day_of_week Mar 15, 2019
@arpadlukacs
Copy link

Similar issue with another schedule:

schedule = IceCube::Schedule.new(Time.zone.now)
schedule.add_recurrence_rule(IceCube::Rule.monthly.day_of_week(monday: [7]))
schedule.first

It seems when the number is greater than 5, it freezes.

@JamesPlayer
Copy link

It seems that setting any invalid values and calling schedule.first or schedule.next_occurrence causes it to freeze. In my case it was setting hour_of_day to 24 (should be between 0 and 23):

sched = IceCube::Schedule.new do |s|
  s.add_recurrence_rule(IceCube::Rule.weekly(1).day(:friday).hour_of_day(24))
end

sched.next_occurrence

Must be stuck in a loop somehow.

@aarona
Copy link

aarona commented Sep 16, 2021

Getting the same issue. I had some code that misunderstands how day_of_week validations should be created. A test rule hash was created for the purpose of having an occurrence for every day of the second week. The hash generated was this:

{
  "rule_type": "IceCube::MonthlyRule",
  "interval": 1,
  "validations": {
    "day_of_week": {
      "1": [],
      "2": [
        0,
        1,
        2,
        3,
        4,
        5,
        6
      ],
      "3": [],
      "4": []
    },
    "hour_of_day": 7,
    "minute_of_hour": 19
  }
}

The correct way to create this hash is like so:

{
  "rule_type": "IceCube::MonthlyRule",
  "interval": 1,
  "validations": {
    "day_of_week": {
      "0": [2],
      "1": [2],
      "2": [2],
      "3": [2],
      "4": [2],
      "5": [2],
      "6": [2]
    },
    "hour_of_day": 7,
    "minute_of_hour": 19
  }
}

The first hash is actually asking for occurrences for every 0th, 1st, 2nd, 3rd, 4th, 5th, and 6th Tuesday of every month. There is no 0th or 6th Tuesday of any month. When calling IceCube::Rule.from_hash and passing the first hash, this should cause some kind of ArgumentError (Invalid rule type) much like calling IceCube::Rule.from_hash({}) does.

@pacso
Copy link
Collaborator

pacso commented May 11, 2024

The problem is that there is no protection against rules which don't result in any occurrences within a schedule.

PR #554 will add validations to catch most cases, but I expect there will always be edge cases involving combining multiple rules, such as picking months with less than 31 days and asking for the 31st day of the month.

For that we will need to add some kind of infinite-loop protection, but raising an error doesn't feel right. Probably better to just return an empty schedule.

@aarona
Copy link

aarona commented May 15, 2024

The problem is that there is no protection against rules which don't result in any occurrences within a schedule.

PR #554 will add validations to catch most cases, but I expect there will always be edge cases involving combining multiple rules, such as picking months with less than 31 days and asking for the 31st day of the month.

For that we will need to add some kind of infinite-loop protection, but raising an error doesn't feel right. Probably better to just return an empty schedule.

I like this idea because it feels very "rubyish". My work around if I recall -- this was a few years ago -- was to take the hash and sanitize it. Remove any keys/indices that don't belong like a 0th Friday or 6th Sunday or something like that and then pass it along to whatever reads the hash.

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

Successfully merging a pull request may close this issue.

5 participants