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

Incorrect Behavior in Setting Up a Cron Job for First Monday of the Month #890

Open
mnkasikci opened this issue Jul 17, 2024 · 2 comments
Open
Labels
type:bug Bug reports and bug fixes

Comments

@mnkasikci
Copy link

mnkasikci commented Jul 17, 2024

Description

The cron expression ("5 12 */100,1-7 * 1") for running a cron job on every first Monday of the month at 12:05 PM appears to be having unintended behavior. The linkage between the month and weekday in the given expression is "OR" , but it should be "AND".
(It works every monday + first 7 days of the month, but it should work only if it's monday AND if it's first seven days of the month)
Sources: a blogpost
Crontab.guru

Expected Behavior

The cron job should execute only on the first Monday of the month at 12:05 PM.

Actual Behavior

Instead of running exclusively on the first Monday of the month, the cron job is executing on every Monday and the first seven days of any month.

Possible Fix

No response

Steps to Reproduce

  1. Set the cron expression in your node-cron configuration to "5 12 */100,1-7 * 1".
  2. Create a cron job that runs the function which logs a message.
  3. Run the cron job.
  4. Check the output of the next 10 execution dates:
const cronExpression = "5 12 */100,1-7 * 1";
const job = new CronJob(
  cronExpression,
  function () {
    console.log("You probably won't wait this long, but if you do, you should see that it runs every monday + first seven days of the month.");
    console.log(`Run on : ${new Date().toISOString()}`);
      }, // onTick
  null, // onComplete
  true,
);
for (const nextDate of job.nextDates(10))
  console.log(nextDate.toString());

Context

I was trying to send a certain report to people every first monday of the month. As a temporary solution, i set the cron expression to run every monday , and the onTick() function checks if it's in the first 7 days of the month, and returns without doing nothing otherwise.

Your Environment

I don't think these are relevant, but still providing them

  • cron version3.1.7:
  • NodeJS version: v20.11.1
  • Operating System and version: macOS Sonoma 14.5
  • TypeScript version (if applicable): 5.2.2
  • Link to your project (if applicable): N/A
@sheerlox
Copy link
Collaborator

Hi, thanks for reporting!

I must admit my knowledge stopped here:

This expression translates to “Run at the midnight if it’s the first day of the month OR Monday”. An expression like this could be handy for a job that sends weekly and monthly reports as it probably needs to run at the start of every week, and the start of every month. However, if either field is unrestricted, the logical relationship between the fields changes to “AND”.

I wasn't aware that any of these fields starting with a * would still be considered unrestricted. This is super interesting.

However, this feels like a hack (or trick as the author names it). We have a feature request (as well as a draft PR) to implement the # character that would fit this purpose more properly, although not available in the standard cron syntax.

I'm all for keeping aligned with the UNIX standard. Still, I wonder if this particular use case, because it's so hacky, wouldn't be more fit to be disclosed as unsupported in the README, in favor of the # character.

What do you think?

@sheerlox sheerlox added the type:bug Bug reports and bug fixes label Jul 17, 2024
@mnkasikci
Copy link
Author

That would apparently fit better to the use case, easier to read and less prone to user error.

Telling cron to run every 100th day of the month is indeed really hacky. I hated every moment of writing it to my code, but that would be the lesser of two evils :) (the other being, checking if it's the proper time for running in the function )

All in all, I agree that the problems this feature would bring outweights the benefits, considering the draft pr you mentioned.

Thanks for your time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Bug reports and bug fixes
Projects
None yet
Development

No branches or pull requests

2 participants