Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lastDate() value for cron intervals > 25 days is sometimes wrong #710

Closed
sheerlox opened this issue Oct 2, 2023 · 0 comments · Fixed by #711
Closed

lastDate() value for cron intervals > 25 days is sometimes wrong #710

sheerlox opened this issue Oct 2, 2023 · 0 comments · Fixed by #711
Labels
status:ready Ready to start implementation type:bug Bug reports and bug fixes

Comments

@sheerlox
Copy link
Collaborator

sheerlox commented Oct 2, 2023

Description

lastExecution (which is returned by lastDate()) is not about the last time we ran the onTick function, but about the last time we checked if we should run it (which will differ only when the time between executions is > ~ 24.85 days).

Expected Behavior

lastExecution should be updated only when we run the onTick function.

Actual Behavior

lastExecution is updated when we check if we should run the onTick function.

Possible Fix

Move lastExecution update from line 227 to line 239.

Steps to Reproduce

test case

it('should give the correct last execution date for monthly crons', () => {
	const callback = jest.fn();
	const clock = sinon.useFakeTimers();

	// At 00:00 on day-of-month 1.
	const job = new CronJob('0 0 0 1 * *', callback);
	job.start();

	// tick 25 days (MAX_TIMEOUT == 2147483647 ms == ~ 24,85 days)
	clock.tick(1000 * 60 * 60 * 24 * 25);

	expect(callback).toHaveBeenCalledTimes(0);
	expect(job.lastDate()?.getTime()).toBeUndefined();

	job.stop();
	clock.restore();
});

result

image

observations

callback was never called, but we still have a lastExecution (which equals the MAX_DELAY value).

Context

N/A

Your Environment

  • cron version: v3.0.0
  • NodeJS version: v18.17.1
  • Operating System and version: Ubuntu 22.04.3 (jammy)
  • TypeScript version (if applicable):
  • Link to your project (if applicable):
@sheerlox sheerlox added type:bug Bug reports and bug fixes status:ready Ready to start implementation labels Oct 2, 2023
@sheerlox sheerlox changed the title lastDate() value for cron intervals > ~ 24.85 days is sometimes wrong lastDate() value for cron intervals > 25 days is sometimes wrong Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:ready Ready to start implementation type:bug Bug reports and bug fixes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant