Exception in periodic task stops this task queue

[dasagan]

Name of Issue

Exception in periodic task stops this task queue

• ActionHero Version: 19.1.4
• Node.js Version: 8.11.3
• Operating System: (Windows 10)

Steps to reproduce your error

To create test-task.js with content:

const {api, Task} = require('actionhero')

exports.SayHello = class SayHello extends Task {
  constructor () {
    super()
    this.name = 'sayHello'
    this.description = 'I say hello'
    this.frequency = 1000
    this.queue = 'low'
    this.middleware = []
  }

  async run () {
    api.log("hello",'info');
    throw 'exception from SayHello periodic task';
  }
}

Steps to fix error

Old snippet in actionhero/initializers/tasks.js:

      return {
        plugins: plugins,
        pluginOptions: pluginOptions,
        perform: async function () {
          let combinedArgs = [].concat(Array.prototype.slice.call(arguments))
          combinedArgs.push(this)
          let response = await task.run.apply(task, combinedArgs)
          await api.tasks.enqueueRecurrentTask(taskName)
          return response
        }
      }
    }

to replace by new:

      return {
        plugins: plugins,
        pluginOptions: pluginOptions,
        perform: async function () {
          let combinedArgs = [].concat(Array.prototype.slice.call(arguments))
          combinedArgs.push(this)
          let response = null;
          try{
          	response = await task.run.apply(task, combinedArgs)
          } catch (error) {
          	throw error
          }
          finally {
          	await api.tasks.enqueueRecurrentTask(taskName)
          }
          return response
        }
      }
    }

[evantahler]

Thanks!

I don't know if this is a bug per-se, but was a design choice to not keep re-running a crashing job. That said, perhaps this belongs as an option to re-run crashing jobs or not? Are you able to make a Pull request with your changes we can discuss?

[evantahler]

Amending my note above, it might be cool to have the new api.config.tasks.reEnqueuePeriodicTasksIfException (or whatever you call the option) as default enabled for the development environment, and then off for all the others?

[samking314]

I feel like this is more useful for very specific jobs. For instance, if I have a job that I know might throw an error initially but will return successfully at some point over a bounded period of time then it might be useful for that job to keep crashing until it returns successfully(although now that I write that down it seems like a very unlikely scenario).
As for development, wouldn't you want the job to crash and throw an exception only once so you could fix it? I don't know what its use in development would be.

[evantahler]

That's a good hint that perhaps the option should be on the task itself!

[samking314]

Going off of what I said about a bounded interval, you might also add a variable for the number of allowed retries or amount of time it's allowed to retry until it finally returns the error.

[dasagan]

add a variable for the number of allowed retries or amount of time it's allowed to retry until it finally returns the error

It seems to me that this is an excessive complication. I think that reEnqueuePeriodicTaskIfException option in the task will be more than enough.