Skip to content

Commit

Permalink
fix: replace loop timeout by max match date (#686)
Browse files Browse the repository at this point in the history
  • Loading branch information
sheerlox authored Aug 14, 2023
1 parent 9dbe962 commit c685c63
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 9 deletions.
20 changes: 12 additions & 8 deletions lib/time.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()}`
);
Expand Down
31 changes: 30 additions & 1 deletion tests/cron.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit c685c63

Please sign in to comment.