DEV: Introduce deprecation warning for non-json Job arguments (#15842)
This commit introduces our own handling and warning for Sidekiq's new 'non-json-serializable' warning. This decouples us from Sidekiq's own deprecation cycle, and allows us to use our own deprecation system. It also means that the dump/parse happens in test mode, which will help us to catch occurrences before they reach production.
This commit is contained in:
parent
a8325c9016
commit
c8c23ba557
|
@ -293,32 +293,45 @@ module Jobs
|
||||||
opts[:current_site_id] ||= RailsMultisite::ConnectionManagement.current_db
|
opts[:current_site_id] ||= RailsMultisite::ConnectionManagement.current_db
|
||||||
end
|
end
|
||||||
|
|
||||||
# If we are able to queue a job, do it
|
delay = opts.delete(:delay_for)
|
||||||
|
queue = opts.delete(:queue)
|
||||||
|
|
||||||
|
# Only string keys are allowed in JSON. We call `.with_indifferent_access`
|
||||||
|
# in Jobs::Base#perform, so this is invisible to developers
|
||||||
|
opts = opts.stringify_keys
|
||||||
|
|
||||||
|
# Simulate the args being dumped/parsed through JSON
|
||||||
|
parsed_opts = JSON.parse(JSON.dump(opts))
|
||||||
|
if opts != parsed_opts
|
||||||
|
Discourse.deprecate(<<~MSG.squish, since: "v2.9", drop_from: "3.0")
|
||||||
|
#{klass.name} was enqueued with argument values which do not cleanly serialize to/from JSON.
|
||||||
|
This means that the job will be run with slightly different values than the ones supplied to `enqueue`.
|
||||||
|
Argument values should be strings, booleans, numbers, or nil (or arrays/hashes of those value types).
|
||||||
|
MSG
|
||||||
|
end
|
||||||
|
opts = parsed_opts
|
||||||
|
|
||||||
if ::Jobs.run_later?
|
if ::Jobs.run_later?
|
||||||
hash = {
|
hash = {
|
||||||
'class' => klass
|
'class' => klass,
|
||||||
|
'args' => [opts]
|
||||||
}
|
}
|
||||||
|
|
||||||
if delay = opts.delete(:delay_for)
|
if delay
|
||||||
if delay.to_f > 0
|
if delay.to_f > 0
|
||||||
hash['at'] = Time.now.to_f + delay.to_f
|
hash['at'] = Time.now.to_f + delay.to_f
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
if queue = opts.delete(:queue)
|
if queue
|
||||||
hash['queue'] = queue
|
hash['queue'] = queue
|
||||||
end
|
end
|
||||||
|
|
||||||
hash['args'] = [opts.deep_stringify_keys]
|
|
||||||
|
|
||||||
DB.after_commit { klass.client_push(hash) }
|
DB.after_commit { klass.client_push(hash) }
|
||||||
else
|
else
|
||||||
# Otherwise execute the job right away
|
# Otherwise execute the job right away
|
||||||
opts.delete(:delay_for)
|
opts["sync_exec"] = true
|
||||||
opts.delete(:queue)
|
|
||||||
|
|
||||||
opts[:sync_exec] = true
|
|
||||||
if Rails.env == "development"
|
if Rails.env == "development"
|
||||||
Scheduler::Defer.later("job") do
|
Scheduler::Defer.later("job") do
|
||||||
klass.new.perform(opts)
|
klass.new.perform(opts)
|
||||||
|
|
|
@ -122,3 +122,5 @@ end
|
||||||
|
|
||||||
Sidekiq.error_handlers.clear
|
Sidekiq.error_handlers.clear
|
||||||
Sidekiq.error_handlers << SidekiqLogsterReporter.new
|
Sidekiq.error_handlers << SidekiqLogsterReporter.new
|
||||||
|
|
||||||
|
Sidekiq.strict_args!
|
||||||
|
|
|
@ -108,7 +108,7 @@ describe Jobs do
|
||||||
end
|
end
|
||||||
|
|
||||||
it "executes the job right away" do
|
it "executes the job right away" do
|
||||||
Jobs::ProcessPost.any_instance.expects(:perform).with(post_id: 1, sync_exec: true, current_site_id: "default")
|
Jobs::ProcessPost.any_instance.expects(:perform).with("post_id" => 1, "sync_exec" => true, "current_site_id" => "default")
|
||||||
Jobs.enqueue(:process_post, post_id: 1)
|
Jobs.enqueue(:process_post, post_id: 1)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue