From 6b1382ab80ea6c9eab8290e4f1e9e0f2a81f1597 Mon Sep 17 00:00:00 2001 From: chatman-media Date: Tue, 23 Jun 2026 06:24:05 +0700 Subject: [PATCH] fix: don't advance lastDate() for ticks skipped by waitForCompletion When `waitForCompletion` is enabled and a callback is still running, the scheduled tick is skipped in `fireOnTick()`. However `lastExecution` was assigned right before calling `fireOnTick()` (and in `runOnInit`/threshold paths), so it advanced even when no callback actually ran. This made `lastDate()`/`lastExecution` report the time of the last tick check rather than the last real execution, contradicting their documented meaning. Move the `lastExecution` assignment into `fireOnTick()`, after the already-running guard, so it only updates when the callback truly fires. Closes #1048 --- src/job.ts | 8 ++++---- tests/cron.test.ts | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/src/job.ts b/src/job.ts index 4e301194..ab1482bc 100644 --- a/src/job.ts +++ b/src/job.ts @@ -127,7 +127,6 @@ export class CronJob { this.addCallback(this._fnWrap(onTick)); if (runOnInit) { - this.lastExecution = new Date(); void this.fireOnTick(); } @@ -245,6 +244,10 @@ export class CronJob { // skip job if previous callback is still running if (this.waitForCompletion && this._isCallbackRunning) return; + // record the moment the callback actually executes, so that lastDate() + // reflects the last real execution rather than the last tick check + // (ticks skipped above must not advance it). See #1048. + this.lastExecution = new Date(); this._isCallbackRunning = true; // handle errors in synchronous and asynchronous callbacks @@ -334,8 +337,6 @@ export class CronJob { setCronTimeout(timeout); } else { // we have arrived at the correct point in time. - this.lastExecution = new Date(); - this._isActive = false; // start before calling back so the callbacks have the ability to stop the cron job @@ -364,7 +365,6 @@ export class CronJob { // execute immediately if within threshold console.warn(`${message}. Executing immediately.`); - this.lastExecution = new Date(); void this.fireOnTick(); } else { // skip job if beyond threshold diff --git a/tests/cron.test.ts b/tests/cron.test.ts index 7c3e266d..c6a907c3 100644 --- a/tests/cron.test.ts +++ b/tests/cron.test.ts @@ -1436,6 +1436,45 @@ describe('cron', () => { expect(job.isCallbackRunning).toBe(false); expect(job.isActive).toBe(false); }); + + it('should not advance lastDate() for ticks skipped due to waitForCompletion (#1048)', async () => { + const clock = sinon.useFakeTimers(); + + const job = new CronJob( + '* * * * * *', + async () => { + await new Promise(resolve => { + setTimeout(resolve, 1500); + }); + }, + null, + true, + null, + null, + false, + null, + false, + true // waitForCompletion: true + ); + + // first execution starts at 1000ms + await clock.tickAsync(1000); + expect(job.isCallbackRunning).toBe(true); + expect(job.lastDate()?.getTime()).toBe(1000); + + // the tick at 2000ms is skipped because the callback is still + // running, so lastDate() must still reflect the 1000ms execution. + await clock.tickAsync(1000); + expect(job.isCallbackRunning).toBe(true); + expect(job.lastDate()?.getTime()).toBe(1000); + + // first callback completes at 2500ms, next execution starts at 3000ms + await clock.tickAsync(1000); + expect(job.isCallbackRunning).toBe(true); + expect(job.lastDate()?.getTime()).toBe(3000); + + job.stop(); + }); }); describe('Daylight Saving Time', () => {