croner icon indicating copy to clipboard operation
croner copied to clipboard

Published 20 hours ago • verified publisher icon Hexagon

[Bug] `interval` setting does seem to ignore `startAt`

Open CommanderStorm opened this issue 9 months ago • 1 comments

Describe the bug Hi, first thanks a ton for the library. We have been (ab)using it a lot.

I think we have come across what I think is a bug in the implementation of https://github.com/Hexagon/croner/issues/63: If startAt is in the past, interval does not calculate starting from startAt but is skippped instead (We think).

To Reproduce Steps to reproduce the behavior:

  1. enter startAt and an absurdly large interval (to make what I think is a bug more obvious ^^)
  2. Run code https://jsfiddle.net/ak85Lwrv/4/ image

Expected behavior

I would expect that the first execution of the cronjob is not "now" and then the interval comes into play, but rather that this respects startAt.

System:

  • OS: Manjraro/Linux
  • Runtime Chrome/JSfiddle
  • Version 124.0.6367.91

Additional context This issue is the outcome of the investigation of @buzzinJohnnyBoi in https://github.com/louislam/uptime-kuma/pull/4738

CommanderStorm avatar May 04 '24 19:05 CommanderStorm

Oh! Will have a look at this as soon as possible. Thanks for a great bug report!

Hexagon avatar May 04 '24 20:05 Hexagon

Hello, @Hexagon, I gave finding the bug a shot, and I think I have fixed it. In the /src/croner.js file (_next function), I just added a check !prev && options.startAt && options.interval, then calculate when prev (previous run) should have been. I have tested this with multiple different values, and it gets the correct value each time. All tests and linters pass, and I have run npm run build and the tests for dist pass as well. I also added a test in /src/suites/options.cjs with the startAt in the past and the excepted date in the future. Is it OK if I create a PR for this issue? My Commit

buzzinJohnnyBoi avatar May 11 '24 12:05 buzzinJohnnyBoi

@buzzinJohnnyBoi Awesome, send the PR 👍

Hexagon avatar May 11 '24 12:05 Hexagon

I sent the PR, wasn't sure if you wanted me to include the changes after running npm run build, so I looked at some of your previous PRs and it looks like you do that sometimes, so I included them :)

buzzinJohnnyBoi avatar May 11 '24 13:05 buzzinJohnnyBoi

@buzzinJohnnyBoi's fix available to test through npm i [email protected]

Hexagon avatar May 13 '24 19:05 Hexagon

Closing as https://github.com/Hexagon/croner/pull/248 was merged ^^ Thanks everyone ❤️

CommanderStorm avatar May 13 '24 22:05 CommanderStorm

An additional fix by @buzzinJohnnyBoi released through prerelease 8.0.3-dev.1

Hexagon avatar May 18 '24 18:05 Hexagon