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

Recuring Interval Maintenances display wrong dates in the maintenance list #4634

Open
1 task done
eakteam opened this issue Mar 31, 2024 · 7 comments
Open
1 task done
Labels
area:maintenance related to the maintenance mode blocked-upstream upstream (i.e. a dependency we depend on will have to do this work) bug Something isn't working dependencies Pull requests that update a dependency file

Comments

@eakteam
Copy link

eakteam commented Mar 31, 2024

📑 I have found these related issues/pull requests

NONE

🛡️ Security Policy

Description

Trying to add a Recuring Interval Maintenance and it seems that it gets the wrong dates.

E.g. trying to add this maintenance on 04/07/2024
uptime1

It shows wrong data here.
uptime2

👟 Reproduction steps

Add a recuring interval maintenance e.g. running once every 7 days starting from a specific date and after it ads it, it displays the wrong data in the Maintenance list.

👀 Expected behavior

It should show the right date which we have added in the first step.

😓 Actual Behavior

It shows the wrong date.

🐻 Uptime-Kuma Version

1.23.11

💻 Operating System and Arch

Ubuntu 22.04 LTS

🌐 Browser

Google Chrome Version 123.0.6312.86 (Official Build) (64-bit)

🖥️ Deployment Environment

  • Runtime: NodeJS v20.12.0
  • Database: sqlite/embedded
  • Filesystem used to store the database on: ext4 on SSD
  • number of monitors: 7

📝 Relevant log output

No response

@eakteam eakteam added the bug Something isn't working label Mar 31, 2024
@CommanderStorm CommanderStorm changed the title Maintenance Recuring Interval Maintenances display wrong dates in the maintenance list Apr 1, 2024
@CommanderStorm CommanderStorm added help wanted May need your help to test or answer good first issue Good for newcomers area:maintenance related to the maintenance mode labels Apr 1, 2024
@buzzinJohnnyBoi
Copy link

Hello, is anyone currently working on this issue? If not, then I would like to try my hand at it.
Thanks.

@CommanderStorm
Copy link
Collaborator

Nobody is currently working on this to my knowledge ^^

@buzzinJohnnyBoi
Copy link

buzzinJohnnyBoi commented Apr 13, 2024

I have made a little bit of progress, I was able to reproduce the error and have isolated it to this line of code:

let start = dayjs(this.beanMeta.job.nextRun(dayjs().add(-this.duration, "second").toDate()));

If the interval is 5 or 7, this evaluates incorrectly (the dayjs obj element $d = Tue Apr 16 2024 06:00:00 GMT-0400 (Eastern Daylight Saving Time), instead of the correct value, Sat Apr 13 2024 06:00:00 GMT-0400 (Eastern Daylight Saving Time))
But, if the value is 6, or 4-1, then it evaluates correctly to Sat Apr 13 2024 06:00:00 GMT-0400 (Eastern Daylight Saving Time).

My question is, what exactly is the this.beanMeta.job.nextRun function doing? Is it part of the redbean-node lib somehow?
I am new to contributing, so any help would be much appreciated.

@CommanderStorm
Copy link
Collaborator

Not quite. beanMeta.job is the croner instance for said maintenace period.

Said variable comes from here:

this.beanMeta.job = new Cron(this.cron, {
timezone: await this.getTimezone(),
}, startEvent);

To your question:

what exactly is the nextRun function doing

Sadly, croners docs is not great, but hey..
https://github.com/Hexagon/croner/blob/aa932979a949258af2c8093e8af88f15d743342e/src/croner.js#L195-L204
=> the line finds the next runtime as dayjs, based on the current time-duration. I would need to look into if timezones are correctly passed here.

The duration is initialised here:

async generateCron() {
log.info("maintenance", "Generate cron for maintenance id: " + this.id);
if (this.strategy === "cron") {
// Do nothing for cron
} else if (!this.strategy.startsWith("recurring-")) {
this.cron = "";
} else if (this.strategy === "recurring-interval") {
let array = this.start_time.split(":");
let hour = parseInt(array[0]);
let minute = parseInt(array[1]);
this.cron = minute + " " + hour + " */" + this.interval_day + " * *";
this.duration = this.calcDuration();
log.debug("maintenance", "Cron: " + this.cron);
log.debug("maintenance", "Duration: " + this.duration);
} else if (this.strategy === "recurring-weekday") {
let list = this.getDayOfWeekList();
let array = this.start_time.split(":");
let hour = parseInt(array[0]);
let minute = parseInt(array[1]);
this.cron = minute + " " + hour + " * * " + list.join(",");
this.duration = this.calcDuration();
} else if (this.strategy === "recurring-day-of-month") {
let list = this.getDayOfMonthList();
let array = this.start_time.split(":");
let hour = parseInt(array[0]);
let minute = parseInt(array[1]);
let dayList = [];
for (let day of list) {
if (typeof day === "string" && day.startsWith("lastDay")) {
if (day === "lastDay1") {
dayList.push("L");
}
// Unfortunately, lastDay2-4 is not supported by cron
} else {
dayList.push(day);
}
}
// Remove duplicate
dayList = [ ...new Set(dayList) ];
this.cron = minute + " " + hour + " " + dayList.join(",") + " * *";
this.duration = this.calcDuration();
}

@buzzinJohnnyBoi
Copy link

Thank you for that info, @CommanderStorm, I think I can solve the issue now, here is my approach:
The task will run at the selected time, every possible day, but in the args for the new Cron() I will include the interval param, which will be the time for 5, 7, or whatever interval of days. I will also include the startAt in the args.
This will work for any date that is in the future, as it will start on the start date and then continue on every interval after that.
However, if the date is in the past, then it will start as soon as possible, so today or tomorrow.
This problem is solved if the date for the next maintenance start date/time is calculated.
E.g. if the start day is Apr. 14, 2024, (4 days ago) for a 6 day interval, then it will recalculate the start date for the cron to be Apr 20th.
Below is some sample code to demonstrate this.
Please let me know if this approach makes sense, and if it does, I will incorporate these changes and hopefully commit the changes soon :)

import { Cron } from "croner";

const intervalInDays = 6;

let startDate = new Date('2024-04-14T07:00:00Z');

const hour = startDate.getUTCHours();
const minute = startDate.getUTCMinutes();

const cronString = `${minute} ${hour} * * *`;

const task = () => {
    console.log("Running task");
};

const now = new Date();
if (startDate < now) {
    const differenceInDays = Math.ceil((now - startDate) / (1000 * 60 * 60 * 24));
    const numberIntervalsSinceStartDate = Math.ceil(differenceInDays / intervalInDays);
    startDate.setDate(startDate.getDate() + numberIntervalsSinceStartDate * intervalInDays);
}

console.log(`This will run every ${intervalInDays} days, starting from ${startDate}`);

const job = new Cron(cronString, { interval: intervalInDays * 24 * 60 * 60, startAt: startDate.toISOString() });

job.schedule(task);

console.log(job.nextRuns(5));

@CommanderStorm
Copy link
Collaborator

Could you provide a PR instead? Revieingg what has to be changed is more error prown than nessesary via comments..

@buzzinJohnnyBoi
Copy link

Hi, @CommanderStorm, the PR should be ready for review. I did not use the strategy described above, instead I had to re-calculate the Cron every time the maintenance task is run. While it is not optimal because code needs to be added for only that strategy, it works for every use case.

@chakflying chakflying added dependencies Pull requests that update a dependency file blocked-upstream upstream (i.e. a dependency we depend on will have to do this work) and removed help wanted May need your help to test or answer good first issue Good for newcomers labels May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:maintenance related to the maintenance mode blocked-upstream upstream (i.e. a dependency we depend on will have to do this work) bug Something isn't working dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants