Handle error handling for deleted tasks

[etiennebarrie]

If a Task is enqueued and is then deleted before being performed, or if a Task is interrupted and deleted, it results in a crash.

To repro:

$ bundle add sidekiq
$ $EDITOR test/dummy/config/application.rb
# add this line
# config.active_job.queue_adapter = :sidekiq

Start a server, visit http://localhost:3000/maintenance_tasks/tasks/Maintenance::UpdatePostsTask, start a run then:

$ rm test/dummy/app/tasks/maintenance/update_posts_task.rb
$ (cd test/dummy; bundle exec sidekiq)
# crash

This is fixed by the first commit.

$ git checkout task-deleted-during-run^
$ (cd test/dummy; bundle exec sidekiq)
# this should pick up the retry, otherwise clean up and redo the steps above

$ git restore -- test/dummy/app/tasks/maintenance/update_posts_task.rb
$ (cd test/dummy; bundle exec sidekiq)
# enqueue the job, once it starts, hit Ctrl-C to stop sidekiq and interrupt the task
$ rm test/dummy/app/tasks/maintenance/update_posts_task.rb
$ (cd test/dummy; bundle exec sidekiq)
# crash

This is fixed by the second commit:

$ git checkout task-deleted-during-run^
$ (cd test/dummy; bundle exec sidekiq)
# this should pick up the retry, otherwise clean up and redo the steps above

[rafaelfranca]

Why the variable would not be defined? Because before_perform did run?

[etiennebarrie]

before_perform runs but has an exception before @ticker is set, on line 57:

maintenance_tasks/app/jobs/maintenance_tasks/task_job.rb

Lines 55 to 62 in 42fd4ee

	def before_perform
	@run = arguments.first
	@task = Task.named(@run.task_name).new
	@run.job_id = job_id
	@run.running! unless @run.stopping?
	@ticker = Ticker.new(MaintenanceTasks.ticker_delay) do |ticks, duration|

[etiennebarrie]

At first I just used @ticker&.persist but it didn't pass the test because we use verbose mode. We could also add a @ticker = nil at the beginning of before_perform too but I felt that would feel weird.

[etiennebarrie]

Worth noting that a similar exception could happen if there's a job for a run that is then removed, except the exception would be during deserialization, but then in on_error we would fail because we access @run. This is not something that can be done without accessing a console or removing a row in the DB though.

This PR solves an issue that can happen with a simple rollback before the job starts running or while the job is still running (it will be interrupted, then would fail upon resuming).

[adrianna-chang-shopify]

Nice find Étienne! Changes LGTM, all was as expected on the tophat 👍

One small request, could we tweak:

maintenance_tasks/app/validators/maintenance_tasks/run_status_validator.rb

Line 14 in 2a3f4bd

# enqueued -> errored occurs when the task job fails to be enqueued.

to add

 # enqueued -> errored occurs when the task job fails to be enqueued,
or if the Task is deleted before is starts running.