parseTimeOrError: no parse of "2020-02-24 11:60" (since LTS-15.1 migration) about cron

[rdelgatte]

FYI this happened since we migrated from lts-14.16 to resolver: lts-15.0

[sir4ur0n]

After some investigation:
I think the problem is in System.Cron.Schedule.findNextMinuteDelay, in particular on this line:

m     = (read (formatTime defaultTimeLocale fmtMinutes now) :: Int) + 1

If the current minute is 59, then this returns 60 (thanks Captain Obvious!).

Now the interesting bit:

-- With time-1.8.0.2
λ> parseTimeOrError True defaultTimeLocale "%F %H:%M" "2020-02-26 17:60" :: UTCTime
2020-02-26 18:00:00 UTC
it :: UTCTime
-- With time-1.9.3
λ> parseTimeOrError True defaultTimeLocale "%F %H:%M" "2020-02-26 17:60" :: UTCTime
*** Exception: parseTimeOrError: no parse of "2020-02-26 17:60"
CallStack (from HasCallStack):
  error, called at libraries/time/lib/Data/Time/Format/Parse.hs:82:12 in time-1.9.3:Data.Time.Format.Parse

The behavior changed in time library!

Proposal:

• Extract the pure function so that it's testable in isolation (and actually add tests 😆 )

findNextMinuteDelay :: IO (UTCTime, Int)
findNextMinuteDelay = getCurrentTime >>= findNextMinuteDelay'

findNextMinuteDelay' :: UTCTime -> (UTCTime, Int)
findNextMinuteDelay' now = ...

• Simplify this function by:
  • Relying on addUTCTime to first add a minute to now. This will be simpler and correctly handle the "off by 1" problem for 60 minutes
  • Manipulating seconds (numbers) instead of parsing

oneMinute :: NominalDiffTime
oneMinute = 60

addUTCTime oneMinute now

How does that sound?

Thank you!

If you (@MichaelXavier) are ok with this proposal, we (me or someone in my team) can try to propose a PR to apply those changes (but it would still be your duty to publish a new release to fix this bug).

Please let us know!

[MichaelXavier]

@sir4ur0n I'd take that PR. Thanks for planning that out and stepping up to fix 😁

[paolino]

good, maybe we can have it in for lts-15.3 ?
Nice work anyway removing partial parsing!