Skip to content

fix: stop hour-step loop early on spring-forward DST days to prevent day skip#400

Merged
harrisiirak merged 2 commits intoharrisiirak:masterfrom
malenkov:fix/dst-spring-forward-day-skip
Mar 10, 2026
Merged

fix: stop hour-step loop early on spring-forward DST days to prevent day skip#400
harrisiirak merged 2 commits intoharrisiirak:masterfrom
malenkov:fix/dst-spring-forward-day-skip

Conversation

@malenkov
Copy link
Copy Markdown
Contributor

@malenkov malenkov commented Mar 8, 2026

Bug

On spring-forward DST transition days, next() skips the entire day when the cron expression targets that day via day-of-week, returning the following week instead.

Minimal repro (US spring-forward, March 8 2026):

const { CronExpressionParser } = require('cron-parser');

const interval = CronExpressionParser.parse('0 8 * * 0', {
  tz: 'America/Chicago',
  currentDate: new Date('2026-03-08T05:45:00.000Z'), // 11:45 PM CST Sat Mar 7
});

console.log(interval.next().toISOString());
// Expected: '2026-03-08T13:00:00.000Z'  (8am CDT Sun Mar 8)
// Got:      '2026-03-15T13:00:00.000Z'  (skips DST day, returns following Sunday)

Root Cause

In #matchHour (CronExpression.ts), when the current date falls on a DST transition day, the code steps hour-by-hour instead of directly setting the target hour. The number of steps is computed as nextHour - currentHour in local wall-clock time.

On a spring-forward day, one applyDateOperation(Add, Hour) call jumps 2 wall-clock hours (e.g. 01:00 → 03:00), so the loop overshoots the target by 1 hour:

  • Steps calculated: 8 - 0 = 8
  • i=0: 00:00 → 01:00
  • i=1: 01:00 → 03:00 ← DST jump (2 hours)
  • i=2..6: 03→04→05→06→07→08
  • i=7: 08:00 → 09:00 ← overshoot!

Now at 09:00, findNearestValue(9) on [8] returns null → the code jumps a full day → repeats until the next Sunday.

Fix

Break out of the stepping loop as soon as the current hour reaches or passes the target:

for (let i = 0; i < steps; i++) {
  currentDate.applyDateOperation(dateMathVerb, TimeUnit.Hour, hours.length);
  // On spring-forward days, one step can jump 2 wall-clock hours and overshoot.
  if (!reverse && currentDate.getHours() >= nextHour) break;
  if (reverse && currentDate.getHours() <= nextHour) break;
}

Tests

Added a regression test in CronExpressionParser.test.ts covering this exact scenario. All 264 existing tests continue to pass.

…overshooting

On spring-forward DST transition days, one applyDateOperation(Add, Hour)
call jumps 2 wall-clock hours (e.g. 01:00 → 03:00). The hour-stepping
loop in #matchHour computed steps as `nextHour - currentHour`, which
caused it to overshoot the target by 1 hour on any day that includes
a spring-forward transition.

For example, with '0 8 * * 0' (Sundays at 8am, America/Chicago) and
currentDate on Saturday night before the US spring-forward day (Mar 8
2026), next() returned the following Sunday (Mar 15) instead of Mar 8,
skipping the DST day entirely.

Fix: break out of the stepping loop as soon as the current hour reaches
or passes the target, so the DST jump cannot cause an overshoot.

Adds a regression test covering this exact scenario.
@harrisiirak harrisiirak self-requested a review March 8, 2026 19:47
@harrisiirak harrisiirak added the bug label Mar 9, 2026
Copy link
Copy Markdown
Owner

@harrisiirak harrisiirak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @malenkov!

I've left some comments, but the PR looks good to me in general.

Comment thread src/CronExpression.ts
currentDate.applyDateOperation(dateMathVerb, TimeUnit.Hour, hours.length);
// On spring-forward days, one step jumps 2 wall-clock hours (e.g. 01:00 → 03:00),
// which can overshoot the target hour. Stop as soon as we reach or pass it.
if (!reverse && currentDate.getHours() >= nextHour) break;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpic: Adding comment overshoot protection is probably fine enough

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, shortened to a one-liner.

Comment thread tests/CronExpressionParser.test.ts Outdated
expect(prev.getMinutes()).toEqual(0);
});

test('does not skip spring-forward DST day when targeted by day-of-week', () => {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: Let's also add reverse direction test case to ensure that it works in both ways.
suggestion: Rename it to "does not overshoot target hour on spring-forward DST day" or something similar

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, renamed both tests and added a reverse direction case.

Comment thread tests/CronExpressionParser.test.ts Outdated
});

test('does not skip spring-forward DST day when targeted by day-of-week', () => {
// March 8, 2026 is the US spring-forward day (America/Chicago clocks jump 01:00→03:00).
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: Probably don't need that long comment about the behavior, it's rather self explaining already.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, trimmed.

@harrisiirak harrisiirak merged commit 04e300a into harrisiirak:master Mar 10, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants