cron #previous_time loop breaker issue

bdarcet commented 3 months ago

Issue description

Follow up on https://github.com/floraison/fugit/issues/95 Looks like we've encountered other issues in our app after the last update

Same error when using Fugit.parse_cron with #previous_time on a specific Time zone it works for some (the one you've already fixed) but breaks for other ones it seems

How to reproduce

require 'fugit'

def test(x)
  puts
  puts "  --- #{x} ---"
  c = Fugit.parse_cron(x)
  begin
    puts c.previous_time.strftime('%F %T %:z %A')
  rescue => err
    p err
  end
  begin
    puts c.next_time.strftime('%F %T %:z %A')
  rescue => err
    p err
  end
end

test('20 0 * * 2%4 Europe/London') #fails
test('20 0 * * 2%4 Etc/UTC') # fails 
test('20 9 * * 2%4 Australia/Melbourne') # fails
test('0 7 * * 2%2 America/Bogota') # fails
test('21 0 * * 1%2 America/Sao_Paulo') # works
..

I can provide others if necessary I think

Error and error backtrace (if any)

/home/app/hivebrite/vendor/bundle/ruby/3.3.0/gems/fugit-1.9.0/lib/fugit/cron.rb:304:in `block in previous_time': too many loops for "0 5 * * 2%6 Europe/Berlin" #previous_time, breaking, cron expression most likely invalid (Feb 30th like?), please fill an issue at https://git.io/fjJCQ (RuntimeError)

Context

Please replace the content of this section with the output of the following commands:

uname -a
5.15.120+ #1 SMP Thu Aug 24 17:32:58 UTC 2023 x86_64 GNU/Linux
bundle exec ruby -v
bundle exec ruby -e "p [ :env_tz, ENV['TZ'] ]"
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-linux]
bundle exec ruby -r et-orbi -e "EtOrbi._make_info"
(secs:1711025258.1711426,utc~:"2024-03-21 12:47:38.171142578125",ltz~:"UTC")
(etz:nil,tnz:"UTC",tziv:"2.0.6",tzidv:"1.2023.4",rv:"3.3.0",rp:"x86_64-linux",win:false,rorv:nil,astz:nil,eov:"1.2.9",eotnz:#<TZInfo::DataTimezone: Etc/UTC>,eotnfz:"+0000",eotlzn:"Etc/UTC",eotnfZ:"UTC",debian:"Etc/UTC",centos:nil,osx:"Etc/UTC")
bundle exec ruby -r fugit -e "p [ :fugit, Fugit::VERSION ]"
[:fugit, "1.9.0"]
bundle exec ruby -e "p [ :now, Time.now, :zone, Time.now.zone ]"
[:now, 2024-03-21 12:48:36.736098929 +0000, :zone, "UTC"]

jmettraux commented 3 months ago

Hello,

I have further refined how et-orbi computes rweek which is used for the cron modulo. I think it solves your issue.

Please upgrade to et-orbi 1.2.10 and tell me how it goes.

Merci beaucoup !

bdarcet commented 3 months ago

Hello, @jmettraux ! Thanks again for your fast reply on this issue :) It looks like the upgrade to et-orbi 1.2.10 fixed all our errors 🎉

I've done a bit of testing to compare our last used versions 1.2.7 & 1.2.10 And it looks like the results are a bit different between the 2 (not for every cron but for some)

I just wanted your opinion on those (it looks like on 1.2.7, we had big jumps in the future on the next_time for example that we don't have anymore - I'm not sure if they were expected in the first place but .. - etc):

Version	1.2.7	1.2.10
cron	--- 0 5 2%6 Europe/Berlin ---	--- 0 5 2%6 Europe/Berlin ---
previous time	2024-02-27 04:00:00 +00:00 Tuesday	2024-02-27 04:00:00 +00:00 Tuesday
next time	2024-11-05 04:00:00 +00:00 Tuesday	2024-04-16 03:00:00 +00:00 Tuesday

Version	1.2.7	1.2.10
cron	--- 0 10 2%2 America/New_York ---	--- 0 10 2%2 America/New_York ---
previous time	2024-03-12 14:00:00 +00:00 Tuesday	2024-03-19 14:00:00 +00:00 Tuesday
next time	2024-11-05 15:00:00 +00:00 Tuesday	2024-04-02 14:00:00 +00:00 Tuesday

Version	1.2.7	1.2.10
cron	--- 20 0 2%2 America/Los_Angeles ---	--- 20 0 2%2 America/Los_Angeles ---
previous time	2024-02-27 08:20:00 +00:00 Tuesday	2024-03-19 07:20:00 +00:00 Tuesday
next time	2024-04-02 07:20:00 +00:00 Tuesday	2024-04-02 07:20:00 +00:00 Tuesday

Version	1.2.7	1.2.10
cron	--- 20 9 2%8 America/New_York ---	--- 20 9 2%8 America/New_York ---
previous time	2024-03-12 13:20:00 +00:00 Tuesday	2024-03-19 13:20:00 +00:00 Tuesday
next time	2024-12-17 14:20:00 +00:00 Tuesday	2024-05-14 13:20:00 +00:00 Tuesday

Let me know, thank you!

jmettraux commented 3 months ago

Hello @bdarcet,

thanks for the extra test cases. I much prefer the new 1.2.10 results. I've added some debug output to watch how #next_time and #previous_time zero on their target and it seems correct to me, it seems it does not skip now (it skipped too much and triggered the loop breaker previously, hence your reports).

The initial implementation of #rweek was too loose, sorry about that. I'm glad you spotted the issue.

Closing this one now. Please report if there is anything else.

Have a nice week-end!

floraison / fugit