ElMassimo / request_store_rails

📦 Per-request global storage for Rails prepared for multi-threaded apps
https://github.com/ElMassimo/request_store_rails
MIT License
85 stars 6 forks source link

Why does `current_store_id` default to `nil`? #11

Open fsateler opened 3 years ago

fsateler commented 3 years ago

Hi,

I was bit recently by a bug because a thread (created by a library) did not properly initialize current_store_id. I added a monkey patch to add that, and all is working well now.

However, this got me thinking: why is it possible for current_store_id to ever return nil? I believe the safer default is to always set to SecureRandom.uuid if uninitialized:

def current_store_id
  Thread.current.fetch(REQUEST_STORE_ID) { SecureRandom.uuid }
end

The current default basically makes all the spawned threads share a store between them, but separate from the main thread.

It may be an even safer default to raise an error when the request store is unset. It probably means a thread was spawned without proper setup/teardown

ElMassimo commented 3 years ago

Hi Felipe, that's a good observation.

A possible solution would be to have the main thread use true unless specified, and raise an error for any child threads that are not explicitly initialized.

Thanks!