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

Jobs don't fire due to leap year #225

Closed
albertdatui opened this Issue Jan 8, 2016 · 69 comments

Comments

Projects
None yet
@albertdatui
Contributor

albertdatui commented Jan 8, 2016

There is a bug in cron-parser library.
I suspect that it is due to the leap year.

Here is the snippet of the error code.

screen shot 2016-01-08 at 2 37 46 pm

@santigimeno

This comment has been minimized.

Member

santigimeno commented Jan 8, 2016

I can confirm these errors are happening. Will look into them. Thanks for reporting!

@JosMarRivera

This comment has been minimized.

JosMarRivera commented Jan 28, 2016

+1

@fenwick67

This comment has been minimized.

fenwick67 commented Jan 29, 2016

This bit me today (well, sometime within the last week or so).

This may be a duplicate of #172.

@JamCode

This comment has been minimized.

JamCode commented Jan 29, 2016

image

+1

@zaikin-andrew

This comment has been minimized.

zaikin-andrew commented Jan 29, 2016

+1

@ericsandine

This comment has been minimized.

ericsandine commented Jan 29, 2016

👍
screenshot 2016-01-29 11 45 46

var time = '00 41 11 * * 1-5';
Node: 0.12.7

@santigimeno

This comment has been minimized.

Member

santigimeno commented Jan 30, 2016

Hey guys, can you check if the changes in #233 fix the issue for you? Thanks!

@adityabansod

This comment has been minimized.

adityabansod commented Jan 31, 2016

+1, ran in to this today

@corbanb

This comment has been minimized.

corbanb commented Jan 31, 2016

IS there a fix for today then? I updated to 0.6.0.

@GermanBluefox

This comment has been minimized.

GermanBluefox commented Jan 31, 2016

I use node-schedule in my project. It is about 1000 people who use it. They all can't control heating, light and so on. I am interesting in solution too.

@tejasmanohar

This comment has been minimized.

Member

tejasmanohar commented Jan 31, 2016

@santigimeno

This comment has been minimized.

Member

santigimeno commented Jan 31, 2016

Hi all. I understand this issue is important for all. I plan releasing a 1.0.0 release soon (soon meaning when I/we can). In the meantime, it would really help if you could try the great fix from @albertdatui in #233 and provide feedback. Also, as @tejasmanohar said here #233 (comment), you can always add the branch from @albertdatui to your package.json to include the fix in your project. Thanks

@fenwick67

This comment has been minimized.

fenwick67 commented Jan 31, 2016

I'm trying out the @albertdatui branch... it seems to be working for me.

@KevinC0

This comment has been minimized.

KevinC0 commented Feb 1, 2016

@albertdatui branch is also working properly for me. In package.json: "node-schedule": "git://github.com/albertdatui/node-schedule.git#bugFixLeapYear"

@wwgc

This comment has been minimized.

wwgc commented Feb 1, 2016

+1

@nmors

This comment has been minimized.

nmors commented Feb 1, 2016

+1

same issue here

@vwatson

This comment has been minimized.

vwatson commented Feb 1, 2016

+1

@ivands

This comment has been minimized.

ivands commented Feb 1, 2016

+1
i'm getting this error once a day.
Please fix this.
It's unusable right now.

@tejasmanohar

This comment has been minimized.

Member

tejasmanohar commented Feb 1, 2016

this is a semver major change. @santigimeno suggests grouping it w/ the other 1.0 changes, which I stand by. in the meantime, see-

and definitely do not comment here or open additional issues unless you've read the above

@holm

This comment has been minimized.

holm commented Feb 1, 2016

I deployed the branch from the pull request last night, and while there are no errors now, it also doesn't seem like it is firing any crons now.

@tejasmanohar tejasmanohar added the bug label Feb 1, 2016

@tejasmanohar tejasmanohar changed the title from Invalid explicit day of month definition to Jobs don't fire due to leap year Feb 1, 2016

@tejasmanohar

This comment has been minimized.

Member

tejasmanohar commented Feb 1, 2016

@holm Can you provide an example we can run to replicate? Maybe you can use sinon fake timers? Small, isolated- separate from all your business logic and so forth.

@holm

This comment has been minimized.

holm commented Feb 1, 2016

Absolutely. Let me look into a bit more and see what I can find and come up with a testcase.

@tejasmanohar

This comment has been minimized.

Member

tejasmanohar commented Feb 1, 2016

Thanks! Next few days are super busy 😓 so no promises, but a reproducible test case makes debugging this 100x easier (and more reasonable) for anyone who'd like to attempt it.

@santigimeno

This comment has been minimized.

Member

santigimeno commented Feb 1, 2016

@doronMendelson That's great news :)

@KevinC0

This comment has been minimized.

KevinC0 commented Feb 1, 2016

I have been using the @albertdatui branch for 11 hours now. I thought it tackled 100% of my issue, however, I discovered this morning a strange behavior every hour between :45 and :00.

var j = schedule.scheduleJob('*/2 * * * *', function(){

https://www.dropbox.com/s/vxpy5die0btlqhc/Capture%20d%27%C3%A9cran%202016-02-01%2010.23.31.png?dl=0

I still have to investigate but it seems related.

@santigimeno

This comment has been minimized.

Member

santigimeno commented Feb 1, 2016

@KevinC0 Ok just let us know if you find something. FYI, I've run some tests using your cron expression and sinon and it works correctly.

@zettam

This comment has been minimized.

zettam commented Feb 1, 2016

Any solutions so far?

@holm

This comment has been minimized.

holm commented Feb 1, 2016

I have tried to reproduce the problem on my mac, but I can only reproduce on our servers. On our servers all the jobs fire immediately when started, and then never after that.

@santigimeno

This comment has been minimized.

Member

santigimeno commented Feb 1, 2016

@holm Thanks for the info. Can you reproduce it consistently on your servers? If that's the case, can you add these lines:

  console.log('runOnDate >>>>>>>>>>>>>>>>>>>>>>>>>');
  console.log('now: ' + new Date(now));
  console.log('then: ' + new Date(then));
  console.log('runOnDate <<<<<<<<<<<<<<<<<<<<<<<<<');

To the runOnDate function here: https://github.com/albertdatui/node-schedule/blob/bugFixLeapYear/lib/schedule.js#L366, just before

if (then < now) {
    process.nextTick(job);
    return null;
  }

And paste the output?

@holm

This comment has been minimized.

holm commented Feb 1, 2016

Alright, it seems the node-schedule package didn't update properly on the servers, even though the shrinkwrap file is fine and it works on Travis. I suspect it is a problem with npm@3.5, which has been giving us a bit of grief.

Sorry for the smoke on this, I think you should go ahead and release.

@santigimeno

This comment has been minimized.

Member

santigimeno commented Feb 1, 2016

@holm Thanks for the feedback! Regarding the npm issue. We have avoided upgrading to npm@3 because of some weird behaviour and are using npm@2 everywhere.

@santigimeno

This comment has been minimized.

Member

santigimeno commented Feb 1, 2016

Update regarding 1.0.0. Here https://github.com/node-schedule/node-schedule/tree/v1.0-pre is the code I hopefully will be publishing this evening as 1.0.0. It's just the code from #233 plus removing a functionality that was scheduled some time ago for 1.0.0: #157. Any feedback would be appreciated. Thanks

@cybertim

This comment has been minimized.

cybertim commented Feb 1, 2016

It's working again, thank goodness 👍

wget https://github.com/node-schedule/node-schedule/archive/v1.0-pre.tar.gz
npm uninstall node-schedule
npm install v1.0-pre.tar.gz
@yossi-vexigo

This comment has been minimized.

yossi-vexigo commented Feb 1, 2016

When the V1.0.0 need to released?
Thanks

@GermanBluefox

This comment has been minimized.

GermanBluefox commented Feb 1, 2016

npm install https://github.com/node-schedule/node-schedule/tarball/v1.0-pre

Works! Thank you.

@Jean-PhilippeD

This comment has been minimized.

Jean-PhilippeD commented Feb 1, 2016

👍

@DefunctExodus

This comment has been minimized.

DefunctExodus commented Feb 1, 2016

👍

Thanks for the fix

@crissmoldovan

This comment has been minimized.

crissmoldovan commented Feb 1, 2016

Thanks guys :)

@gruzilla

This comment has been minimized.

gruzilla commented Feb 1, 2016

we changed our package.json to use the v1.0-pre repository and now everything works as it did 2015:

"node-schedule": "node-schedule/node-schedule#v1.0-pre"

thanks for the fix! 👍

@iwozzy

This comment has been minimized.

iwozzy commented Feb 1, 2016

+1

1 similar comment
@andrewpuch

This comment has been minimized.

andrewpuch commented Feb 1, 2016

+1

@ericsandine

This comment has been minimized.

ericsandine commented Feb 1, 2016

👍 Looks like it's working here

@pgkehle

This comment has been minimized.

pgkehle commented Feb 1, 2016

+1

@santigimeno

This comment has been minimized.

Member

santigimeno commented Feb 1, 2016

Fixed in 2468fb7 and released as version 1.0.0.
Major props to @albertdatui for the excellent fix.

@santigimeno santigimeno closed this Feb 1, 2016

@ryanprinsloo

This comment has been minimized.

ryanprinsloo commented Feb 2, 2016

Thanks for getting this sorted so quickly. You guys rock!

@mohitkhanna

This comment has been minimized.

mohitkhanna commented Feb 2, 2016

+1

@NiViktor

This comment has been minimized.

NiViktor commented Feb 2, 2016

Great work! Thanks!

@tejasmanohar

This comment has been minimized.

Member

tejasmanohar commented Feb 2, 2016

@corbanb

This comment has been minimized.

corbanb commented Feb 2, 2016

👍

@stephent

This comment has been minimized.

stephent commented Feb 3, 2016

I updated to 1.0.0, deployed and restarted my app, but my scheduled jobs are no longer firing (last execution was at 09:00 Jan 31 (Mountain Time). The schedule is as follows:

var job = schedule.scheduleJob('0 0,4,8,12,16,20 * * *', function(){...});

Is there something else I need to do to get things running again? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment