dachev / node-crontab

A module for creating, reading, updating, and deleting system cron jobs
189 stars 35 forks source link

invalid syntax does not give an error, it just doesn't create the cronjob. #17

Closed toymachiner62 closed 8 years ago

toymachiner62 commented 8 years ago

If i have this array:

[
  {
    command: '<command to execute',
    when: '<when to execute',
    comment: '<comment>'
  }
]

And this method to create a cron job from that array:

var crontab = require('crontab');

function createCronjobs(cronjobs) {
  return Q.promise(function(resolve, reject) {

    crontab.load(function(err, tab) {

      if(err) {
        return reject(err);
      }

      console.log('tab before = ', tab.jobs());

      cronjobs.forEach(function(cronjob) {
        tab.create(cronjob.command, cronjob.when, cronjob.comment);  // Produces nothing in the crontab
        //tab.create('ls -al', '* * * * *', cronjob.comment);  // When uncommented, this successfully updates the crontab

        // save the cronjobs
        tab.save(function(err, tab) {
          if(err) {  // err is null even if the above cronjob was not created b/c of the syntax
            return reject(err);
          }

          console.log('tab after = ', tab.jobs().toString());
        });
      });

      return resolve();
    });
  });
}

If i have an invalid syntax the crontab does NOT get updated and the err param in the save callback is null.

Shouldn't there be error handling to handle this and appropriately return an error message if the cronjob that was tried to be created did not have the correct syntax?

toymachiner62 commented 8 years ago

I think the problem lies here and here.

Those should probably not return null but rather throw an error or something. Returning null leaves the consumer wondering why their cronjob did not get created and no error was given.