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

waitForCompletion not working as expected #923

Closed
cduff opened this issue Dec 11, 2024 · 4 comments · Fixed by #924
Closed

waitForCompletion not working as expected #923

cduff opened this issue Dec 11, 2024 · 4 comments · Fixed by #924
Labels
released type:bug Bug reports and bug fixes

Comments

@cduff
Copy link

cduff commented Dec 11, 2024

Description

In my testing I can't get waitForCompletion to stop another instance of a job running concurrently as expected based on changes made and documented in 3.3.0.

Expected Behavior

waitForCompletion should stop multiple instances running concurrently, as documented.

Actual Behavior

Setting waitForCompletion true or false doesn't seem to make any difference.

Possible Fix

No response

Steps to Reproduce

import { CronJob } from "cron";
let c = 0;
CronJob.from({
  cronTime: "*/1 * * * * *",
  onTick: async () => {
    console.log(++c);
    await new Promise((r) => setTimeout(r, 1100));
    c--;
  },
  waitForCompletion: true,
  start: true,
});

Output

1
2
2
...

Expect

1
1
1
...

Context

I had previously implemented my own solution to stop multiple instances of a job running concurrently. I thought with 3.3.0 I could simplify my code and instead use the new feature provided by cron.

Your Environment

  • cron version: 3.3.0
  • NodeJS version: 22.12.0
  • Operating System and version: Windows 11 Pro
@intcreator
Copy link
Collaborator

tagging @Zamoca42 who worked on the PR. I'll add your test case as well and see if I can figure out what's going on

@intcreator
Copy link
Collaborator

does this test case illustrate the issue you're experiencing?

@intcreator intcreator added the type:bug Bug reports and bug fixes label Dec 11, 2024
@cduff
Copy link
Author

cduff commented Dec 12, 2024

does this test case illustrate the issue you're experiencing?

Yes, that test asserts an incorrect result. I added a comment below yours: #894 (comment)

@ncb000gt
Copy link
Member

🎉 This issue has been resolved in version 3.3.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

JosephVoid pushed a commit to JosephVoid/node-cron that referenced this issue Dec 14, 2024
## [3.3.1](kelektiv/node-cron@v3.3.0...v3.3.1) (2024-12-12)

### 🐛 Bug Fixes

* correct waitForCompletion behavior ([kelektiv#924](kelektiv#924)) ([f6270f8](kelektiv@f6270f8)), closes [kelektiv#923](kelektiv#923) [kelektiv#923](kelektiv#923) [kelektiv#894](kelektiv#894)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released type:bug Bug reports and bug fixes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants