Skip to content

Commit d258bfe

Browse files
committed
Consolidate and give context to after callback deprecation
The existing message only mentioned one type of before/after callback, but the config was named generally. That mismatch is confusing and users wouldn't necessarily know what the total effect of the config would be. So instead of handwriting the deprecation warning in the specific instances, consolidate it in one place and give the appropriate context. That context is the above, but also that users shouldn't update their app config, they should uncomment the line in the new defaults file, which now also has more context. I'm not totally convinced that we can't move this to when `after_enqueue`/`after_perform` is called in the job class. Doesn't seem worth it to blare this after every job enqueue/perform, when we the score at boot time. cc @Edouard-chin
1 parent 2da5439 commit d258bfe

File tree

5 files changed

+17
-19
lines changed

5 files changed

+17
-19
lines changed

activejob/lib/active_job/callbacks.rb

+10
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,16 @@ def after_enqueue(*filters, &blk)
163163
def around_enqueue(*filters, &blk)
164164
set_callback(:enqueue, :around, *filters, &blk)
165165
end
166+
167+
def warn_against_after_callbacks_execution_deprecation(callbacks) # :nodoc:
168+
if !skip_after_callbacks_if_terminated && callbacks.any? { |c| c.kind == :after }
169+
ActiveSupport::Deprecation.warn(<<~EOM)
170+
In Rails 6.2, `after_enqueue`/`after_perform` callbacks no longer run if `before_enqueue`/`before_perform` respectively halts with `throw :abort`.
171+
To enable this behavior, uncomment the `config.active_job.skip_after_callbacks_if_terminated` config
172+
in the new 6.1 framework defaults initializer.
173+
EOM
174+
end
175+
end
166176
end
167177
end
168178
end

activejob/lib/active_job/enqueuing.rb

+2-8
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,8 @@ def enqueue(options = {})
6363
if successfully_enqueued
6464
self
6565
else
66-
if !self.class.skip_after_callbacks_if_terminated && _enqueue_callbacks.any? { |c| c.kind == :after }
67-
ActiveSupport::Deprecation.warn(<<~EOM)
68-
In Rails 6.2, ActiveJob's `after_enqueue` callbacks will no longer run in case the
69-
callback chain is halted (i.e. `throw(:abort)` is thrown in a before_enqueue callback).
70-
To enable this behaviour right now, add in your application configuration file
71-
`config.active_job.skip_after_callbacks_if_terminated = true`.
72-
EOM
73-
end
66+
self.class.warn_against_after_callbacks_execution_deprecation(_enqueue_callbacks)
67+
7468
if self.class.return_false_on_aborted_enqueue
7569
false
7670
else

activejob/lib/active_job/execution.rb

+1-9
Original file line numberDiff line numberDiff line change
@@ -39,18 +39,10 @@ def perform_now
3939

4040
run_callbacks :perform do
4141
perform(*arguments)
42-
4342
successfully_performed = true
4443
end
4544

46-
if !successfully_performed && !self.class.skip_after_callbacks_if_terminated && _perform_callbacks.any? { |c| c.kind == :after }
47-
ActiveSupport::Deprecation.warn(<<~EOM)
48-
In Rails 6.2, ActiveJob's `after_perform` callbacks will no longer run in case the
49-
callback chain is halted (i.e. `throw(:abort)` is thrown in a before_perform callback).
50-
To enable this behaviour right now, add in your application configuration file
51-
`config.active_job.skip_after_callbacks_if_terminated = true`.
52-
EOM
53-
end
45+
self.class.warn_against_after_callbacks_execution_deprecation(_perform_callbacks) unless successfully_performed
5446
successfully_performed
5547
rescue => exception
5648
rescue_with_handler(exception) || raise

activejob/test/cases/callbacks_test.rb

+2-2
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ class CallbacksTest < ActiveSupport::TestCase
6565
ActiveJob::Base.skip_after_callbacks_if_terminated = false
6666
reload_job
6767
job = AbortBeforeEnqueueJob.new
68-
assert_deprecated(/`after_enqueue` callbacks will no longer run/) do
68+
assert_deprecated(/`after_enqueue`\/`after_perform` callbacks no longer run/) do
6969
job.enqueue
7070
end
7171

@@ -107,7 +107,7 @@ class CallbacksTest < ActiveSupport::TestCase
107107
ActiveJob::Base.skip_after_callbacks_if_terminated = false
108108
reload_job
109109
job = AbortBeforeEnqueueJob.new
110-
assert_deprecated(/`after_perform` callbacks will no longer run/) do
110+
assert_deprecated(/`after_enqueue`\/`after_perform` callbacks no longer run/) do
111111
job.perform_now
112112
end
113113

railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_6_1.rb.tt

+2
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
# Track Active Storage variants in the database.
1313
# Rails.application.config.active_storage.track_variants = true
1414

15+
# Stop executing `after_enqueue`/`after_perform` callbacks if
16+
# `before_enqueue`/`before_perform` respectively halts with `throw :abort`.
1517
# Rails.application.config.active_job.skip_after_callbacks_if_terminated = true
1618

1719
# Specify cookies SameSite protection level: either :none, :lax, or :strict.

0 commit comments

Comments
 (0)