Skip to content
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

reEnqueuePeriodicTaskIfException #1309

Merged
merged 4 commits into from Jun 14, 2019
Merged

Conversation

dasagan
Copy link

@dasagan dasagan commented Jun 10, 2019

Fix: Exception in periodic task stops this task queue
New task option: reEnqueuePeriodicTaskIfException

Solves #1308

n.a.onishchuk and others added 2 commits June 10, 2019 17:39
Fix: Exception in periodic task stops this task queue
New task option: reEnqueuePeriodicTaskIfException
@evantahler evantahler changed the title Issue #1308 reEnqueuePeriodicTaskIfException Jun 12, 2019
Copy link
Member

@evantahler evantahler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for contributing!

Aside from the failing linter check, please also add a test to ensure that this behavior is "locked in" for the future. There are examples of writing tests for tasks here https://github.com/actionhero/actionhero/blob/master/__tests__/core/tasks.js

I suggest that the spec would:

  1. mock out a task that raises an exception
  2. runs the task
  3. checks that the task definition is still enqueued for another run in the delayed queue(s)

This is going to be a rather involved test, so please ask for help if you need it!

initializers/tasks.js Outdated Show resolved Hide resolved
@evantahler
Copy link
Member

evantahler commented Jun 14, 2019

Thank you for your work @dasagan! I've been asking a lot of folks for their opinion on this, and it turns out that we should make the default behavior to re-enqueue if there is a failure. It's more surprising that recurring tasks... aren't recurring.

I'm going to merge your work in, but change the option around so the default option is actually to re-enqueue failing tasks.

#1312

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.

None yet

3 participants