From c685c63a6d7fa86d6c8afca29b536b9da24e824b Mon Sep 17 00:00:00 2001 From: Pierre Cavin Date: Tue, 15 Aug 2023 00:15:49 +0200 Subject: [PATCH] fix: replace loop timeout by max match date (#686) --- lib/time.js | 20 ++++++++++++-------- tests/cron.test.js | 31 ++++++++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/lib/time.js b/lib/time.js index b236ac94..3b9e54c8 100644 --- a/lib/time.js +++ b/lib/time.js @@ -259,19 +259,23 @@ function CronTime(luxon) { throw new Error('ERROR: You specified an invalid date.'); } - // it shouldn't take more than 5 seconds to find the next execution time - // being very generous with this. Throw error if it takes too long to find the next time to protect from - // infinite loop. - const timeout = Date.now() + 5000; + /** + * maximum match interval is 8 years: + * crontab has '* * 29 2 *' and we are on 1 March 2096: + * next matching time will be 29 February 2104 + * source: https://github.com/cronie-crond/cronie/blob/0d669551680f733a4bdd6bab082a0b3d6d7f089c/src/cronnext.c#L401-L403 + */ + const maxMatch = luxon.DateTime.now().plus({ years: 8 }); + // determine next date while (true) { const diff = date - start; - // hard stop if the current date is after the expected execution - if (Date.now() > timeout) { + // hard stop if the current date is after the maximum match interval + if (date > maxMatch) { throw new Error( - `Something went wrong. It took over five seconds to find the next execution time for the cron job. - 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: + `Something went wrong. No execution date was found in the next 8 years. + Please provide the following string if you would like to help debug: Time Zone: ${zone || '""'} - Cron String: ${this} - UTC offset: ${date.offset} - current Date: ${luxon.DateTime.local().toString()}` ); diff --git a/tests/cron.test.js b/tests/cron.test.js index 4d515e38..4953e156 100644 --- a/tests/cron.test.js +++ b/tests/cron.test.js @@ -379,7 +379,7 @@ describe('cron', () => { const clock = sinon.useFakeTimers(); const callback = jest.fn(); - var job = cron.job({ + const job = cron.job({ cronTime: '* * * * * *', onTick: callback, runOnInit: true @@ -768,6 +768,35 @@ describe('cron', () => { expect(callback).toHaveBeenCalledTimes(1); }); + /** + * maximum match interval is 8 years: + * crontab has '* * 29 2 *' and we are on 1 March 2096: + * next matching time will be 29 February 2104 + * source: https://github.com/cronie-crond/cronie/blob/0d669551680f733a4bdd6bab082a0b3d6d7f089c/src/cronnext.c#L401-L403 + */ + it('should work correctly for max match interval', () => { + const callback = jest.fn(); + const d = new Date(2096, 2, 1); + const clock = sinon.useFakeTimers(d.getTime()); + + const job = new cron.CronJob({ + cronTime: ' * * 29 1 *', + onTick: callback, + start: true + }); + + // 7 years, 11 months and 27 days + const almostEightYears = 2919 * 24 * 60 * 60 * 1000; + clock.tick(almostEightYears); + expect(callback).toHaveBeenCalledTimes(0); + + // tick by 1 day + clock.tick(24 * 60 * 60 * 1000); + clock.restore(); + job.stop(); + expect(callback).toHaveBeenCalledTimes(1); + }); + describe('with utcOffset', () => { it('should run a job using cron syntax with number format utcOffset', () => { const clock = sinon.useFakeTimers();