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

Some cron expression will build incorrect intervals #156

Open
coolnk2001 opened this issue Apr 25, 2019 · 9 comments
Open

Some cron expression will build incorrect intervals #156

coolnk2001 opened this issue Apr 25, 2019 · 9 comments
Assignees

Comments

@coolnk2001
Copy link

(An easy to reproduce test case will dramatically decrease the resolution time.)
Example
Cron format | 0 */2 * ? * * work for every 2 minute start from 0 is ok
so the schedule is 11:00, 11:02, 11:04, 11:06 ....
Cron format | 0 1/2 * ? * * work for every 2 minute start from 1 is not ok,
the expected schedule is 11:01, 11:03, 11:05, 11:07 ....
but it give 11:01, 12:01, 13:01, 14:01....

Strange that this expression is working ... 0 1-40/2 * ? * *

I use the library test directly here is the result

*/2 * * ? * *
Date: Tue Apr 23 2019 09:47:26 GMT+0800
Date: Tue Apr 23 2019 09:47:28 GMT+0800
Date: Tue Apr 23 2019 09:47:30 GMT+0800
1/2 * * ? * *
Date: Tue Apr 23 2019 09:48:01 GMT+0800
Date: Tue Apr 23 2019 09:49:01 GMT+0800
Date: Tue Apr 23 2019 09:50:01 GMT+0800
1-59/2 * * ? * *
Date: Tue Apr 23 2019 09:47:25 GMT+0800
Date: Tue Apr 23 2019 09:47:27 GMT+0800
Date: Tue Apr 23 2019 09:47:29 GMT+0800
0 */2 * ? * *
Date: Tue Apr 23 2019 09:48:00 GMT+0800
Date: Tue Apr 23 2019 09:50:00 GMT+0800
Date: Tue Apr 23 2019 09:52:00 GMT+0800
0 1/2 * ? * *
Date: Tue Apr 23 2019 10:01:00 GMT+0800
Date: Tue Apr 23 2019 11:01:00 GMT+0800
Date: Tue Apr 23 2019 12:01:00 GMT+0800
0 1-59/2 * ? * *
Date: Tue Apr 23 2019 09:49:00 GMT+0800
Date: Tue Apr 23 2019 09:51:00 GMT+0800
Date: Tue Apr 23 2019 09:53:00 GMT+0800

@harrisiirak harrisiirak self-assigned this May 20, 2019
@harrisiirak
Copy link
Owner

@coolnk2001 thanks for reporting this! I'll look into this during the following days.

@xmabul
Copy link

xmabul commented Jun 29, 2019

@coolnk2001 HELP~ I have meet the same problem,do you solve it ?

@harrisiirak
Copy link
Owner

harrisiirak commented Jun 29, 2019

@coolnk2001 @xmabul can you provide any reference to other cron implementation that supports this behavior as described in the issue description?

What I'm seeing here:

  • 0 */2 * ? * * - this wildcard range which means that any minute value from 0 to 59
  • 0 1-40/2 * ? * * - this is a partial range (1 to 40) which is also acceptable and step modifier can be applied
  • 0 1/2 * ? * * - this is the explicit definition of minute value which cannot be used together with step modifier. The correct pattern here would be 0 1-59/2 * ? * *

Shortly, step modifier cannot work together with non-range values.

@kaos
Copy link

kaos commented Jul 10, 2019

I see your reasoning for this. I don't know if there's a "cron standard" that dictates what is right or wrong here, but could I suggest to translate a non-range value to a ranged one when used with a step modifier. So 0/n would be the same as */n and 10/2 would become 10-59/2 when used for the minute field, for example.

This to work more transparently with what we have with the cron support for the python library apscheduler (never mind that they went of the rails, by having dow 0 being Monday..)

@harrisiirak
Copy link
Owner

@kaos

So 0/n would be the same as */n and 10/2 would become 10-59/2 when used for the minute field, for example.

Yeah, it sounds to be the most logical behaviour that most people expect of this. However, 10/2 could also mean 0-10/2, therefore it may be somehow confusing for the user if this automagical expansion happens. Sure, this could be documented.

The other option, maybe even clearer/cleaner way to handle this, could be returning an error when an explicit range is not provided with step modifier. Technically this behaviour isn't currently supported and should yield an error. At least it gives some feedback to the user.

@oavanruiten
Copy link

Hi @harrisiirak

Today I ran into the issue discussed above.

I also think that yielding an error is the way to go in this case. I use cron-parser together with other packages and this is causing inconsistencies.

Do you have any idea how straight-forward it is to add this?

@oavanruiten
Copy link

Hi @harrisiirak

I am implementing an extra validation rule which checks each part of the cron expression against the following regex:

^\d{1,2}/\d{1,2}$

This should detect strings where specific, non-range values are used together with a step modifier, e.g. "0/5".

Do you see any immediate problems with this?

@EstebanBorai
Copy link

EstebanBorai commented Sep 26, 2020

Hi @oavanruiten !

I'm using this library too and also found myself in the same requirement.

I ended up using cron-validator npm package to validate the syntax.

I think its a good workaround for this issue while the behavior is being defined!

@HugoPoi
Copy link

HugoPoi commented Sep 20, 2021

0 0 7 1 1/1 * does this => 0 0 7 1 1 *

MichaelLeeHobbs added a commit to MichaelLeeHobbs/cron-parser that referenced this issue Apr 28, 2023
MichaelLeeHobbs added a commit to MichaelLeeHobbs/cron-parser that referenced this issue Apr 28, 2023
…fy as it'd be very difficult to make stringify work with these expressions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants