montrose icon indicating copy to clipboard operation
montrose copied to clipboard

Sub second precision is ignored and `.next` returns time in past for same second

Open doits opened this issue 2 years ago • 1 comments

According to this line in the specs ... https://github.com/rossta/montrose/blob/2487a31fb4b2703a82fd3ed3499e9fba3ea3849d/spec/montrose/frequency/secondly_spec.rb#L16 ... it seems to be intended, but just now it triggered a bug for me:

Montrose.every(:second).events.next de facto returns a time in past, because it will always return a time that has sub second time set to 0.

For example if you have a daily trigger Montrose.every(:day).at('18:00:00') and you run events.next at 15.01.2020 18:00:00.500 (half a second past 18:00:00), it returns 15.01.2020 18:00:00.000 which is in past (at least if you care about sub second precision).

Is this a bug? Shouldn't events.next never return a time in the past?

Edit: A workaround I just implemented is to do Montrose::Recurrence.new(recurrence).take(3).find { |time| time >= Time.current }.

doits avatar Sep 15 '23 11:09 doits

Montrose doesn't support sub-second recurrence. When a recurrence is initialized, the start time for the recurrence interval is zero'd out to the current second.

The issue you are describing will only exist when a recurrence is initialized at less than a second after a time which overlaps with the recurrence rules, as you have illustrated.

Is this a bug?

Probably, though I'd consider it minor. A simple patch with tests to address the issue is welcome.

Shouldn't events.next never return a time in the past?

We do generally want to support events.next returning time in the past (since the start timestamp may be explicitly set in the past) though I understand what you mean in the context of this issue.

A workaround...

Your workaround may lead to subtle bugs as well, since Time.current is evaluated anew each time your find block is run.

rossta avatar Oct 12 '23 04:10 rossta