Skip to content

Commit a3e70ad

Browse files
committed
fix: replace loop timeout by max match date (kelektiv#686)
1 parent b52b5ca commit a3e70ad

File tree

2 files changed

+42
-9
lines changed

2 files changed

+42
-9
lines changed

lib/time.js

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -260,19 +260,23 @@ function CronTime(luxon) {
260260
throw new Error('ERROR: You specified an invalid date.');
261261
}
262262

263-
// it shouldn't take more than 5 seconds to find the next execution time
264-
// being very generous with this. Throw error if it takes too long to find the next time to protect from
265-
// infinite loop.
266-
const timeout = Date.now() + 5000;
263+
/**
264+
* maximum match interval is 8 years:
265+
* crontab has '* * 29 2 *' and we are on 1 March 2096:
266+
* next matching time will be 29 February 2104
267+
* source: https://github.com/cronie-crond/cronie/blob/0d669551680f733a4bdd6bab082a0b3d6d7f089c/src/cronnext.c#L401-L403
268+
*/
269+
const maxMatch = luxon.DateTime.now().plus({ years: 8 });
270+
267271
// determine next date
268272
while (true) {
269273
const diff = date - start;
270274

271-
// hard stop if the current date is after the expected execution
272-
if (Date.now() > timeout) {
275+
// hard stop if the current date is after the maximum match interval
276+
if (date > maxMatch) {
273277
throw new Error(
274-
`Something went wrong. It took over five seconds to find the next execution time for the cron job.
275-
Please refer to the canonical issue (https://github.com/kelektiv/node-cron/issues/467) and provide the following string if you would like to help debug:
278+
`Something went wrong. No execution date was found in the next 8 years.
279+
Please provide the following string if you would like to help debug:
276280
Time Zone: ${zone || '""'} - Cron String: ${this} - UTC offset: ${date.offset}
277281
- current Date: ${luxon.DateTime.local().toString()}`
278282
);

tests/cron.test.js

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,7 @@ describe('cron', () => {
379379
const clock = sinon.useFakeTimers();
380380
const callback = jest.fn();
381381

382-
var job = cron.job({
382+
const job = cron.job({
383383
cronTime: '* * * * * *',
384384
onTick: callback,
385385
runOnInit: true
@@ -768,6 +768,35 @@ describe('cron', () => {
768768
expect(callback).toHaveBeenCalledTimes(1);
769769
});
770770

771+
/**
772+
* maximum match interval is 8 years:
773+
* crontab has '* * 29 2 *' and we are on 1 March 2096:
774+
* next matching time will be 29 February 2104
775+
* source: https://github.com/cronie-crond/cronie/blob/0d669551680f733a4bdd6bab082a0b3d6d7f089c/src/cronnext.c#L401-L403
776+
*/
777+
it('should work correctly for max match interval', () => {
778+
const callback = jest.fn();
779+
const d = new Date(2096, 2, 1);
780+
const clock = sinon.useFakeTimers(d.getTime());
781+
782+
const job = new cron.CronJob({
783+
cronTime: ' * * 29 1 *',
784+
onTick: callback,
785+
start: true
786+
});
787+
788+
// 7 years, 11 months and 27 days
789+
const almostEightYears = 2919 * 24 * 60 * 60 * 1000;
790+
clock.tick(almostEightYears);
791+
expect(callback).toHaveBeenCalledTimes(0);
792+
793+
// tick by 1 day
794+
clock.tick(24 * 60 * 60 * 1000);
795+
clock.restore();
796+
job.stop();
797+
expect(callback).toHaveBeenCalledTimes(1);
798+
});
799+
771800
describe('with utcOffset', () => {
772801
it('should run a job using cron syntax with number format utcOffset', () => {
773802
const clock = sinon.useFakeTimers();

0 commit comments

Comments
 (0)