Sidekiq: Pass in a hash with symbol as key

[mech]

Ruby version: 2.5.1
Sidekiq / Pro / Enterprise version(s): 5.1.3

I have read the Best Practice and know that symbol is not to be used and it will convert to string due to JSON.dump, but when I do a simple test at Rails 5.2, it appear to not be the case.

class TestSymbol < ApplicationJob
  def perform(hash)
    TestMailer.hello(hash['version']).deliver_now
  end
end

TestSymbol.perform_later({version: '123'})

Notice that I use hash['version'] as I expect the symbol to be a String, but it appear to be a symbol anyway.

I thought the argument of {version: '123'} will be converted to {'version' => '123'}, so I should reference it as hash['version'] in my worker code?

[mperham]

You are using ActuveJob, not Sidekiq, to handle the job. It handles symbols.

[mech]

I think I get what you mean now. Somehow ActiveJob will retain the symbol key in the hash while Sidekiq will convert it to a String. Too totally different approach I guess. Got it!

[olivierlacan]

@mperham Is this documented behavior because it's extremely surprising to queue a job with a symbol-keyed hash and see exceptions roll in because the code in perform receives a hash with string keys instead.

To me this is an Sidekiq onboarding gotcha that I feel I should be warned against somehow, for example when perform receives hashes with symbol keys as arguments.

It doesn't feel like I should need a gem like this to use Sidekiq without scary surprises before/after JSON serialization: https://github.com/aprescott/sidekiq-symbols

I can definitely use it, but it feels like a bit of unnecessary friction.

[mperham]

It’s item #1 on the Best Practices wiki page. Sidekiq has been native JSON since day one.

[olivierlacan]

That's true but as it's written there's some room for confusion:

The arguments you pass to perform_async must be composed of simple JSON datatypes: string, integer, float, boolean, null(nil), array and hash. This means you must not use ruby symbols as arguments.

Reading this I can think that hash is fine to use because technically I'm not using a symbol as the argument to perform, but I am using a hash with symbol keys. I think it'd be better to specify that only hashes with JSON-like string keys should be used.

I'm not saying do my homework for me, but I am saying the library could enforce this best practice (only in the test env to avoid performance issues for example). I've used Sidekiq for years without knowing this hash-related gotcha perhaps because I haven't tried to pass a hash to perform_async until today. It's a sharp edge. Although it didn't take long to recover, it di take an exception monitoring tool outputting worker args for me to notice.

Also, thank you so much for Sidekiq. 99.999999% of the time it brings me joy. ❤️

[mperham]

I agree it would be awesome if Sidekiq could verify/warn in development mode. How do I do this while also preserving backwards compatibility with existing app code? Strict mode?

On Aug 15, 2019, at 08:26, Olivier Lacan [e-mail] wrote:

That's true but as it's written there's some room for confusion:

The arguments you pass to perform_async must be composed of simple JSON datatypes: string, integer, float, boolean, null(nil), array and hash. This means you must not use ruby symbols as arguments.

Reading this I can think that hash is fine to use because technically I'm not using a symbol as the argument to perform, but I am using a hash with symbol keys. I think it'd be better to specify that only hashes with JSON-like string keys should be used.

I'm not saying do my homework for me, but I am saying the library could enforce this best practice (only in the test env to avoid performance issues for example). I've used Sidekiq for years without knowing this hash-related gotcha perhaps because I haven't tried to pass a hash to perform_async until today. It's a sharp edge. Although it didn't take long to recover, it di take an exception monitoring tool outputting worker args for me to notice.

Also, thank you so much for Sidekiq. 99.999999% of the time it brings me joy. ❤️

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[olivierlacan]

Seems like strict mode wouldn't be on by default which might help newcomers and goofballs like me but at the same time introducing a "strict" deep parameter check would probably piss off people so I guess it could be opt-in in a minor and become opt-out in the next major? Is that how you do things?

[feliperaul]

@mperham This has bitten me numerous times, because It's not rare for us to use SomeWorker.new.perform(xxx) (because it's a simple PORO) and find out the bug only after deploying (because then SomeWorker.perform_async(xxx) will get serialized and then the ruby hash with symbol keys will stop working).

Im inclined to do something like:

def perform(args)
args = args.with_indifferent_access

so args[:some_symbol_key] will work regardless of the way it was called.

[mperham]

I would be open to adding a middleware which will make any hash argument indifferent before calling perform if Rails is detected. It would be turned on by default in Sidekiq 6.0.

On Aug 20, 2019, at 06:00, flprben [e-mail] wrote:

@mperham This has bitten me numerous times, because It's not rare for us to use SomeWorker.new.perform(xxx) (because it's a simple PORO) and find out the bug only after deploying (because then SomeWorker.perform_async(xxx) will get serialized and then the ruby hash with symbol keys will stop working).

Im inclined to do something like:

def perform(args)
args = args.with_indifferent_access

so args[:some_symbol_key] will work regardless of the way it was called.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[feliperaul]

That would be amazing.

[mperham]

I'm rethinking this. I just implemented it but HashWithIndifferentAccess only ensures keys work. It won't handle values that are symbols:

perform_async({ :some => :key })

def perform(hash)
  p hash[:some]
end

=> "key"

I'm not sure it's a good idea to supply something that only half works. What's the point if there are still gotchas?

[feliperaul]

Mike, I see your point.

But even tough I agree that there are still gotchas, you are reducing it's surface area considerably. Using symbols as hash keys is the default way of using hash literals in Ruby ({key: 'value'}), and as a matter of user experience, I think it would greatly reduce the number of bites due to the serialization.

The wiki will still inform users about the need to not use symbols in args, but using string keys ({'some key' => 'value'}) is way less idiomatic in my opinion.

I know it's anecdotal, but this would have helped me a few times.