Skip to content

Conversation

@jnkien
Copy link

@jnkien jnkien commented Oct 15, 2014

When setting a scheduled job that occurs just one time, the function
was returning an error because next[1] is then undefined and .getTime()
is not defined. Adding a test to bypass this error.

When setting a scheduled job that occurs just one time, the function
was returning an error because next[1] is then undefined and .getTime()
is not defined. Adding a test to bypass this error.
@zol
Copy link
Contributor

zol commented Oct 15, 2014

@jnkien - thanks for your PR. The code you've changed comes from https://github.com/bunkat/later/blob/master/src/core/settimeout.js, unmodified except for adding the intendedAt parameter. I would prefer to leave the logic in here as is. Can you reproduce the issue in plain later.js? If not, could you please make the fix outside of SyncedCron._laterSetTimeout.

@jnkien
Copy link
Author

jnkien commented Oct 16, 2014

@zol - thank you for your feedback. I understand the fact that you want to keep the logic of included parts of code from later.js as you made it so far.

I investigate the possibility of a fix outside of SyncedCron._laterSetTimeout. Unfortunately, after doing some test and up to my knowledge, it cannot be done and I don't see how to do it because the error is inside a callback of a later.js function.

Then, I reproduced the issue in plain later.js as follows (still under a Meteor project):

Later = Meteor.npmRequire('later');
var schedule = function(parser) {
    var cronDate = '30 12 16 10 * 2014';// just change the first 4 parameters to trigger the job asap
    return parser.cron(cronDate);
};
var s = schedule(Later.parse);
Later.setInterval(function(){
    console.log("This is a one time scheduled job.");
    return true;
}, s);
console.log('One time scheduled job @' + Later.schedule(s).next(1));

I think I can simply use my own fork in my project (it will work but...) or submit a pull request to the later.jsgithub repository. Then afterwards, you could modified your code and recompile the meteor package?

What do you suggest?

Enable adding scheduled jobs on-the-fly. Jobs can be saved and loaded
between server restarts. A job can be run back if the date is passed.
Display the next schedule time after running a job.
@zol
Copy link
Contributor

zol commented Oct 24, 2014

Hey @jnkien . Your cron format looks incorrect there, see here. You have 6 entries in the format and don't specify hasSeconds.

In any case, please submit your pull request to the later.js repo and I will update to use the new dependency once the author fixes the issue. Thanks!

@dalgard
Copy link

dalgard commented Feb 23, 2015

Huh. Didn't see you pull request, @jnkien. You might take a look at my pull requests here, I think we've set out to achieve the same thing:

#42

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants