fix: compensate DST transitions wider than one hour - #435
Conversation
`applyDateOperation` detects a spring-forward by comparing wall-clock hours and testing for a delta of exactly 2, which assumes the gap is one hour wide. `Antarctica/Troll` advances two hours on 2026-03-29, 00:00 to 03:00, so the delta is 3, the branch never fires, and `dstStart` stays null. A schedule inside that window is then dropped for the day rather than moved past the gap. Two changes are needed. Detection now derives the gap from the change in UTC offset, which is exact for any width. A sub-hour transition such as `Australia/Lord_Howe` skips no whole wall-clock hour and so still records nothing, as before. Matching now considers every skipped hour rather than only the one immediately before the current one, since Troll skips both 01:00 and 02:00. For a one-hour gap the loop runs exactly once and behaves as it did. That second change removes an accidental property of the old test: `dstStart === currentHour - 1` stopped matching by itself once the hour advanced, whereas a multi-hour window does not. Without closing the window explicitly, `30 1 * * *` in Troll would match 03:30, then 04:30, then 05:30. The window is closed on hour steps only, because the minutes within the compensating hour still have to match the skipped hour, which is what keeps `*/20 3 * * *` firing at 04:00, 04:20 and 04:40 in the existing Athens test. Result for `30 1 * * *` in `Antarctica/Troll`: before 2026-03-28 01:30 2026-03-30 01:30 <- 29 March never fires after 2026-03-28 01:30 2026-03-29 03:30 2026-03-30 01:30 which is the same compensation `America/New_York` already performs for its one-hour gap. Verified by sweeping 5,865 combinations of 23 expressions across 17 timezones and 15 starting instants, in both directions, against master. 23 cases change, all of them in `Antarctica/Troll`, the only zone in the set with a two-hour gap. Nothing moves in the other 16, including Athens, Lord Howe, Chatham, Havana, Tehran and the fall-back cases. Four regression tests are added in the style of the existing DST block. Three fail without this change; the fourth pins Lord Howe's sub-hour behaviour so a later change cannot start compensating a gap that skips no whole hour. Out of scope: `America/Santiago` transitions at midnight and drops an occurrence the same way, but through a different route, because day-granularity jumps return from `applyDateOperation` before any DST detection runs. That is left separate rather than widening this change. Fixes harrisiirak#419
Quality gates · commit
|
| Statements | Branches | Functions | Lines |
|---|---|---|---|
| 100% | 100% | 100% | 100% |
Benchmark — vs cron-parser@5.7.0 (npm latest)
10000 iterations × 5 samples per pattern. A positive change is faster than the baseline.
| Pattern | Baseline | This PR | Change |
|---|---|---|---|
* * * * * * |
67.34ms | 64.77ms | +3.82% |
0 15 */5 5 * |
610.98ms | 596.20ms | +2.42% |
0 * * 1,4-10,L * * |
116.35ms | 113.79ms | +2.20% |
0 H/3 * * * |
226.04ms | 221.15ms | +2.16% |
10-30/2 2 12 8 0 |
207.15ms | 205.64ms | +0.72% |
0 H * * * |
226.06ms | 224.46ms | +0.71% |
0 0 0 * * 5#3 |
1604.31ms | 1597.30ms | +0.44% |
0 0 0 8 * 5#3 |
882.27ms | 878.67ms | +0.41% |
0 0 6-20/2,L 2 * |
684.00ms | 681.38ms | +0.38% |
0 12 */5 6 * |
613.72ms | 611.49ms | +0.36% |
0 0 0 * * 1L,5L |
906.10ms | 903.25ms | +0.31% |
10 2 12 8 7 |
962.87ms | 960.21ms | +0.28% |
H H H(9-20)/3 1-11 * |
569.86ms | 568.62ms | +0.22% |
0 0 0 * * 4,6L |
454.81ms | 454.55ms | +0.06% |
0 0 0 15 * 5#3 |
934.25ms | 934.29ms | -0.00% |
Informational only. Runner timings are noisy and this check never fails.
harrisiirak
left a comment
There was a problem hiding this comment.
Thanks for the detailed writeup. The detection change is right, but the window close leaks through one path. dstStart is only cleared on hour-unit steps, while #moveToNextSecond rolls minute 59 into the next hour via a minute unit step (src/CronExpression.ts:163), so the marker survives that crossing and the acceptance loop, bounded by the live current hour, re-accepts every later hour of the day.
Repro in a plain one-hour zone:
const interval = CronExpressionParser.parse('30 59 2 * * *', {
currentDate: new Date('2026-03-07T12:00:00Z'),
tz: 'America/New_York',
});
// master: 2026-03-08T07:59:30Z (03:59:30 EDT, compensated), then Mar 9
// this branch: 07:59:30Z, 08:59:30Z, 09:59:30Z, ... hourly for the rest of the daySuggestion is to record the landing hour next to dstStart (cleared and copied together with it) and gate acceptance on currentHour === landingHour instead of closing the window via hour steps.
| const diff = currentHour - previousHour; | ||
| if (diff === 2) { | ||
|
|
||
| // Spring-forward is detected from the change in UTC offset rather than from |
There was a problem hiding this comment.
Let's keep this and other comments a bit more codensed if possible.
There was a problem hiding this comment.
Condensed. That block is down from eleven lines to three, and I trimmed the surrounding ones in CronExpression.ts and the new tests the same way.
Closing the skipped-hour window on hour-unit steps leaked through `#moveToNextSecond`, which rolls minute 59 into the next hour with a minute-unit step. The marker survived that crossing, so the acceptance loop re-matched every later hour of the day. `CronDate` now records the hour the clock landed on alongside `dstStart`, copied and cleared with it, and `#matchHour` accepts a skipped hour only while the clock is still on that landing hour. The window is then self limiting again, as `dstStart === currentHour - 1` was before, so the explicit close on hour steps is gone. Adds a regression test for the reported case, `30 59 2 * * *` in America/New_York, which now yields one occurrence per day again.
|
Good catch, that path was open. Went with your suggestion. Your repro now gives one occurrence per day again, matching master: Added it as a regression test. Reverting just the landing hour check makes it fail, and it takes the Troll test with it. Troll still fires on 29 March, and the existing Athens |
Fixes #419. Opening this as invited in the issue.
Problem
applyDateOperationdetects a spring-forward by comparing wall-clock hours and testing for a delta of exactly 2:That assumes the gap is one hour wide.
Antarctica/Trolladvances two hours on 2026-03-29, 00:00 to 03:00, so the delta is 3, the branch never fires, anddstStartstaysnull. A schedule inside the skipped window is then dropped for the day rather than moved past the gap:For a daily job that is a silently missed execution rather than a late one.
Changes
Detection derives the gap from the change in UTC offset, which is exact for any width:
A sub-hour transition such as
Australia/Lord_Howeskips no whole wall-clock hour, so it still records nothing, as before.Matching considers every skipped hour rather than only the one immediately before the current one, since Troll skips both 01:00 and 02:00. For a one-hour gap the loop runs exactly once and behaves as it did.
That second change removes an accidental property of the old test:
dstStart === currentHour - 1stopped matching by itself once the hour advanced, whereas a multi-hour window does not. Without closing the window explicitly,30 1 * * *in Troll matches 03:30, then 04:30, then 05:30. I close it on hour steps only, because the minutes within the compensating hour still have to match the skipped hour, which is what keeps*/20 3 * * *firing at 04:00, 04:20 and 04:40 in the existing Athens test.Result
which is the same compensation
America/New_Yorkalready performs for its one-hour gap.Verification
The existing suite passes unchanged. Four regression tests are added in the style of the existing DST block: three fail without this change, and the fourth pins Lord Howe's sub-hour behaviour so a later change cannot start compensating a gap that skips no whole hour. 303 of 303 pass with the change.
Because this touches
#matchHour, I swept 5,865 combinations of 23 expressions across 17 timezones and 15 starting instants, in both directions, comparingmasteragainst this branch. 23 cases change, all of them inAntarctica/Troll, the only zone in the set with a two-hour gap. Nothing moves in the other 16, including Athens, Lord Howe, Chatham, Havana, Tehran and the fall-back cases.Out of scope
America/Santiagotransitions at midnight and drops an occurrence the same way, but through a different route: day-granularity jumps return fromapplyDateOperationbefore any DST detection runs. I have left that separate rather than widening this change.Found while porting this library to Go and comparing the two implementations on generated input. Happy to adjust the approach if you would rather solve it another way.