harrisiirak / cron-parser

Node.js library for parsing crontab instructions
MIT License
1.32k stars 156 forks source link

Incorrect next time in UTC for cron expression in America/New_York timezone #333

Closed shayonj closed 12 months ago

shayonj commented 1 year ago

Hello! I am sharing the following snippet where I am seeing that for 0 */3 * * * and 0 */4 * * * the next() time in UTC is not correct. I would have expected it to be Tue, 03 Oct 2023 21:00:00 GMT, instead of Tue, 03 Oct 2023 20:00:00 GMT for 0 */4 * * *.

Am I misunderstanding the use of the next and tz option or is perhaps this is a bug?

var parser = require("cron-parser");

var options = {
  currentDate: new Date("2023-10-03 18:17:37 UTC"),
  tz: "America/New_York"
};

var interval = parser.parseExpression("0 */1 * * *", options);
console.log("0 */1 * * * =>", interval.next().toDate().toUTCString());
// 0 */1 * * * => Tue, 03 Oct 2023 19:00:00 GMT 

var interval0 = parser.parseExpression("0 */2 * * *", options);
console.log("0 */2 * * * =>", interval0.next().toDate().toUTCString());
// 0 */2 * * * => Tue, 03 Oct 2023 20:00:00 GMT 

var interval1 = parser.parseExpression("0 */3 * * *", options);
console.log("0 */3 * * * =>", interval1.next().toDate().toUTCString());
// 0 */3 * * * => Tue, 03 Oct 2023 19:00:00 GMT 

var interval2 = parser.parseExpression("0 */4 * * *", options);
console.log("0 */4 * * * =>", interval2.next().toDate().toUTCString());
// 0 */4 * * * => Tue, 03 Oct 2023 20:00:00 GMT 
harrisiirak commented 1 year ago

Hi @shayonj!

Here's what happens:

In summary, date matching occurs against the localized date, not the original input date.

Let me know if I can provide any further help or missed something important.

shayonj commented 12 months ago

oh i see! Is it possible to make it happen against the original input date and the TZ?

harrisiirak commented 12 months ago

@shayonj one option is just to convert input date to UTC, and later, when you want to display it, just convert it back to the original TZ. But I'm not sure if it completely solves what you want to achieve.

shayonj commented 12 months ago

hi @harrisiirak , so unfortunately that doesn't, if I understand you correctly because the end result I am expecting is for the result to relative to the timezone that I pass tz. Is that basically a new feature request?

Because even if I don't convert it to UTC, the output (in the passed in tz) still seems incorrect, but ofc that is working based on how you described above:

var parser = require("cron-parser");

var options = {
  currentDate: new Date("2023-10-03 18:17:37 UTC"),
  tz: "America/New_York"
};

var interval = parser.parseExpression("0 */1 * * *", options);
console.log("0 */1 * * * =>", interval.next().toDate());
// 0 */1 * * * =>  Tue Oct 03 2023 15:00:00 GMT-0400 (Eastern Daylight Time)

var interval0 = parser.parseExpression("0 */2 * * *", options);
console.log("0 */2 * * * =>", interval0.next().toDate());
// 0 */2 * * * => Tue Oct 03 2023 16:00:00 GMT-0400 (Eastern Daylight Time)

var interval1 = parser.parseExpression("0 */3 * * *", options);
console.log("0 */3 * * * =>", interval1.next().toDate());
// 0 */3 * * * => Tue Oct 03 2023 15:00:00 GMT-0400 (Eastern Daylight Time)

var interval2 = parser.parseExpression("0 */4 * * *", options);
console.log("0 */4 * * * =>", interval2.next().toDate());
// 0 */4 * * * => Tue Oct 03 2023 16:00:00 GMT-0400 (Eastern Daylight Time)
harrisiirak commented 12 months ago

@shayonj

I was thinking if the input date is already in UTC, you can set tz = UTC also, and later apply to America/New_York tz conversion where needed.

Another option is that you already convert given currentDate to correct timezone (to America/New_York).

if I understand you correctly because the end result I am expecting is for the result to relative to the timezone that I pass tz.

But it does operate relative to the timezone defined in the options, as described in my first comment. And later when toUTCString is being called on the date object, UTC value is returned.

I would have expected it to be Tue, 03 Oct 2023 21:00:00 GMT, instead of Tue, 03 Oct 2023 20:00:00 GMT for 0 */4 * * *.

Even if you set tz = UTC, this would never will be true. Only following hours will match */4: 0, 4, 8, 12, 16, 20. Of course, how it is formatted later based on tz, is a separate topic.

shayonj commented 12 months ago

ah! i see. My bad in explaining, so the currentDate is only used as an example here to show a reproducible case. When I am using this live, the currentDate is nil and the issue still happens for the affect numerals. All I am doing is setting the tz to America/New_York.

shayonj commented 12 months ago

I was thinking if the input date is already in UTC, you can set tz = UTC also, and later apply to America/New_York tz conversion where needed.

Even if I keep the timezones in UTC, I still see the same issue 🤔

var parser = require("cron-parser");

var options = {
  currentDate: new Date("2023-10-03 18:17:37 UTC"),
  utc: true
};

var interval = parser.parseExpression("0 */1 * * *", options);
console.log("0 */1 * * * =>", interval.next().toDate().toUTCString());
// 0 */1 * * * => Tue, 03 Oct 2023 19:00:00 GMT 

var interval0 = parser.parseExpression("0 */2 * * *", options);
console.log("0 */2 * * * =>", interval0.next().toDate().toUTCString());
// 0 */2 * * * => Tue, 03 Oct 2023 20:00:00 GMT 

var interval1 = parser.parseExpression("0 */3 * * *", options);
console.log("0 */3 * * * =>", interval1.next().toDate().toUTCString());
// 0 */3 * * * => Tue, 03 Oct 2023 21:00:00 GMT 

var interval2 = parser.parseExpression("0 */4 * * *", options);
console.log("0 */4 * * * =>", interval2.next().toDate().toUTCString());
// 0 */4 * * * => Tue, 03 Oct 2023 20:00:00 GMT 

I would have expected 0 */4 * * * to be Tue, 03 Oct 2023 22:00:00 GMT, right?

harrisiirak commented 12 months ago

I would have expected 0 /4 to be Tue, 03 Oct 2023 22:00:00 GMT , right?

Actually no. As I've explained in my previous comment:

Even if you set tz = UTC, this would never will be true. Only following hours will match */4: 0, 4, 8, 12, 16, 20. Of course, how it is formatted later based on tz, is a separate topic.

22:00:00 can't be never matched under these conditions.

shayonj commented 12 months ago

ah! Ok i get it now, timezones oof. I wasn't thinking of 20:00 in UTC. Makes all the more sense, thanks so much for the examples @harrisiirak