Shouldn't before_perform stop the execution of an ActiveJob when returning false?

[Claudio B.]

Hello! I have started using ActiveJob and I noticed the presence of callbacks like before_perform.

I was expecting these callbacks to behave like ActiveRecord ones: if a before_* callback returns false, all the later callbacks and the associated action are cancelled.

That is not the case with ActiveJob, so my question is: should this be changed? And if not… does it mean that writing code inside before_perform is exactly the same than writing code at the beginning of the perform method?

Thanks!

[DHH]

This is by intent. I never liked how returning false in a filter stops the execution. There are too many side effects, like @stuff = Model.something_returning_false, that accidentally halts the callback chain. In AJ, we're instead going for the explicit approach that requires you to raise an exception, if you want to stop execution. We'll be changing callbacks elsewhere to move away from the "false means halt" in Rails 5.0.

before_* is indeed intended to be the same as writing your code before execution, but in such a way that it can be abstracted and mixed in. So you can write before_* callbacks in super classes without having the child need to call super, or in modules that you mix in.

[Andy Jeffries]

On 9 October 2014 00:00, DHH <da...@loudthinking.com> wrote:

This is by intent. I never liked how returning false in a filter stops the execution. There are too many side effects, like @stuff = Model.something_returning_false, that accidentally halts the callback chain. In AJ, we're instead going for the explicit approach that requires you to raise an exception, if you want to stop execution. We'll be changing callbacks elsewhere to move away from the "false means halt" in Rails 5.0.

So is there a recommended exception to raise that is caught silently by ActiveJob as just an "I've already logged the problem, raised a new job, I just want to cancel the job as returning false would have previously" type exception?  Obviously there are plenty of cases where the problem is handled and you just want the job cancelled but don't want the exception bubbling up to the top and being globally logged as unhandled exception?

Disclaimer: I haven't used ActiveJob yet...

Cheers,

Andy

[DHH]

We have rescue_from as well, so that'd be easy for someone to add. We can also look into adding this directly, like throwing a symbol. "throw :skip_job" or something like that. But it should be an explicit action, not a side-effect of returning false. Feel free to look into the throw option as a PR.

[Claudio B.]

DHH, I gave a try to your suggestion and made a PR: https://github.com/rails/rails/pull/17227
Take a look when you have time and send feedback, thanks!