diff --git a/lib/sidekiq.rb b/lib/sidekiq.rb index 92d6cf2ef..1e5527dae 100644 --- a/lib/sidekiq.rb +++ b/lib/sidekiq.rb @@ -252,6 +252,10 @@ def self.on(event, &block) options[:lifecycle_events][event] << block end + def self.strict_mode!(val = true) + options[:raise_on_complex_arguments] = val + end + # We are shutting down Sidekiq but what about workers that # are working on some long job? This error is # raised in workers that have not finished within the hard diff --git a/lib/sidekiq/job_util.rb b/lib/sidekiq/job_util.rb index e9a14cc24..75af6f4dd 100644 --- a/lib/sidekiq/job_util.rb +++ b/lib/sidekiq/job_util.rb @@ -12,11 +12,26 @@ def validate(item) raise(ArgumentError, "Job class must be either a Class or String representation of the class name: `#{item}`") unless item["class"].is_a?(Class) || item["class"].is_a?(String) raise(ArgumentError, "Job 'at' must be a Numeric timestamp: `#{item}`") if item.key?("at") && !item["at"].is_a?(Numeric) raise(ArgumentError, "Job tags must be an Array: `#{item}`") if item["tags"] && !item["tags"].is_a?(Array) + + if Sidekiq.options[:raise_on_complex_arguments] && !json_safe?(item) + msg = <<~EOM + Arguments must be native JSON types, see https://github.com/mperham/sidekiq/wiki/Best-Practices. + + To disable this error, remove `Sidekiq.strict_mode!` from your initializer. + EOM + raise(ArgumentError, msg) + elsif !Sidekiq.options[:disable_complex_argument_warning] && Sidekiq.options[:environment] == "development" && !json_safe?(item) + Sidekiq.logger.warn <<~EOM + Job arguments do not serialize to JSON safely. This will raise an error in Sidekiq 7.0. + + See https://github.com/mperham/sidekiq/wiki/Best-Practices or raise the error today + by calling `Sidekiq.strict_mode!` during Sidekiq initialization. + EOM + end end def normalize_item(item) validate(item) - # raise(ArgumentError, "Arguments must be native JSON types, see https://github.com/mperham/sidekiq/wiki/Best-Practices") unless JSON.load(JSON.dump(item['args'])) == item['args'] # merge in the default sidekiq_options for the item's class and/or wrapped element # this allows ActiveJobs to control sidekiq_options too. @@ -42,5 +57,11 @@ def normalized_hash(item_class) Sidekiq.default_worker_options end end + + private + + def json_safe?(item) + JSON.parse(JSON.dump(item['args'])) == item['args'] + end end end diff --git a/test/test_client.rb b/test/test_client.rb index f9d399550..550e471e8 100644 --- a/test/test_client.rb +++ b/test/test_client.rb @@ -122,6 +122,110 @@ class QueuedWorker assert QueuedWorker.perform_async(1, 2) assert_equal 1, Sidekiq::Queue.new('flimflam').size end + + describe 'argument checking' do + class InterestingWorker + include Sidekiq::Worker + + def perform(an_argument) + end + end + + it 'enqueues jobs with a symbol as an argument' do + InterestingWorker.perform_async(:symbol) + end + + it 'enqueues jobs with a Date as an argument' do + InterestingWorker.perform_async(Date.new(2021, 1, 1)) + end + + it 'enqueues jobs with a Hash with symbols and string as keys as an argument' do + InterestingWorker.perform_async( + { + some: 'hash', + 'with' => 'different_keys' + } + ) + end + + it 'enqueues jobs with a Struct as an argument' do + InterestingWorker.perform_async( + Struct.new(:x, :y).new(0, 0) + ) + end + + it 'works with a JSON-friendly deep, nested structure' do + InterestingWorker.perform_async( + { + 'foo' => ['a', 'b', 'c'], + 'bar' => ['x', 'y', 'z'] + } + ) + end + + describe 'strict mode is enabled' do + before do + Sidekiq.strict_mode! + end + + after do + Sidekiq.strict_mode!(false) + end + + it 'raises an error when using a symbol as an argument' do + assert_raises ArgumentError do + InterestingWorker.perform_async(:symbol) + end + end + + it 'raises an error when using a Date as an argument' do + assert_raises ArgumentError do + InterestingWorker.perform_async(Date.new(2021, 1, 1)) + end + end + + it 'raises an error when using a Hash with symbols and string as keys as an argument' do + assert_raises ArgumentError do + InterestingWorker.perform_async( + { + some: 'hash', + 'with' => 'different_keys' + } + ) + end + end + + it 'raises an error when using a Struct as an argument' do + assert_raises ArgumentError do + InterestingWorker.perform_async( + Struct.new(:x, :y).new(0, 0) + ) + end + end + + it 'works with a JSON-friendly deep, nested structure' do + InterestingWorker.perform_async( + { + 'foo' => ['a', 'b', 'c'], + 'bar' => ['x', 'y', 'z'] + } + ) + end + + describe 'worker that takes deep, nested structures' do + it 'raises an error on JSON-unfriendly structures' do + assert_raises ArgumentError do + InterestingWorker.perform_async( + { + 'foo' => [:a, :b, :c], + bar: ['x', 'y', 'z'] + } + ) + end + end + end + end + end end describe 'bulk' do