Fix MoneySerializer not registered on Rails 8.1.0+ - #780
Conversation
yukideluxe
changed the title
| # `on_load(:active_job)` on 8.1.0/8.1.1, `on_load(:active_job_arguments)` on 8.1.2+), | ||
| # any of which an `:active_job` hook here could lose the race to. See issue #779 and | ||
| # rails/rails commits b7cd3018ef (8.1.0) and c0dc92e9e9 (8.1.2). | ||
| if defined?(::ActiveJob::Serializers) |
There was a problem hiding this comment.
Is it possible for ActiveJob::Serializers to be defined and Rails.application.config.active_job to not be available? I don't know how but I wonder.
There was a problem hiding this comment.
thanks for raising this! not really sure but I am proposing another solution now! Let me know!
2a02d27 to
80c98f1
Compare
|
@jaredmoody reported my original fix did not solve the problem and @sk- said the current code doesn't work when I must admit this is going a bit over my head and my main goal is to simplify the code and avoid Rails version checks at all costs, so after a bunch of back and forth I think the whole thing can be simplified by just adding the Could somebody help me confirm?! 🙏🏻 Ping @sunny PS: CI failures seem unrelated, I will fix in |
wwahammy
left a comment
wwahammy
left a comment
There was a problem hiding this comment.
@yukideluxe I think this all makes sense to me. I think it looks good.
One concern I do have is that the the CI only appeared to test on MongoDB so I'd be worried about whether the tests are missing anything due to that.
money-rails registered its Active Job serializer by appending to `config.active_job.custom_serializers` inside an `ActiveSupport.on_load(:active_job)` block. Rails 8.1.0 (rails/rails@b7cd3018ef) moved consumption of that list out of `after_initialize` into a load hook, and 8.1.2 (rails/rails@c0dc92e9e9) changed that hook to `on_load(:active_job_arguments)`. Either way money-rails ends up appending the serializer after Rails has already read the list, so it is never registered: - On 8.1.0/8.1.1 Rails reads the list in `on_load(:active_job)` -- the same hook money-rails uses -- and Rails' framework callback runs first, so normal job enqueuing of `Money` arguments breaks. - On 8.1.2+ Rails reads it in `on_load(:active_job_arguments)`; normal enqueuing works again, but `ActiveJob::Arguments.serialize` called without loading `ActiveJob::Base` (a documented public path) gets no custom serializers. Register the serializer during initialization instead, so it is present in `config.active_job.custom_serializers` before Rails consumes the list -- regardless of how Rails consumes it or which of `ActiveJob::Base`/`ActiveJob::Arguments` loads first. The dummy test app now loads `active_job/railtie` (it previously didn't, so the serializer specs were silently skipped). Adds an integration test that round-trips a `Money` through `ActiveJob::Arguments`, which runs under every supported Rails gemfile (7.0-8.1). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Appending to config.active_job.custom_serializers during initialization is not enough: Rails consumes that list exactly once, from a point that varies by version (after_initialize on <= 8.0, the :active_job load hook on 8.1.0/8.1.1, the :active_job_arguments load hook on 8.1.2+) and that can fire mid-boot, before money-rails' initializer runs - e.g. any boot-time reference to ActiveJob::Arguments, ActiveJob::SerializationError or ActiveJob::DeserializationError triggers the 8.1.2+ hook, leaving the serializer unregistered. Instead, add the serializer directly to the live ActiveJob::Serializers registry, which nothing resets, so registration during initialization takes effect no matter when (or whether) each Rails version reads the config list. This also stops touching Rails.application.config.active_job, which does not exist when the activejob gem is loaded without its railtie. Verified against Rails 7.0.10, 7.1.6, 7.2.3.1, 8.0.5, 8.1.0, 8.1.1, 8.1.2 and 8.1.3, with Active Job loaded lazily, eagerly, and before application boot. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3114946 to
0095293
Compare
3 participants
Fixes #779
Hello @sunny and whoever is watching 👋🏻 Claude and I had a stab at the reported issue and this is what we've come up with!
Seems that on Rails 8.1+,
MoneyRails::ActiveJob::MoneySerializeris not registered with Active Job, so serializing aMoneyjob argument fails.Root cause
money-rails registered its serializer by appending to
config.active_job.custom_serializersinside anActiveSupport.on_load(:active_job)block. Rails reads that list at a version-dependent moment:custom_serializersviaconfig.after_initializeon_load(:active_job)on_load(:active_job_arguments)Rails <= 8.0 read
config.active_job.custom_serializersinafter_initialize(end of boot), so money-rails's appended serializer was always there in time.8.1.0 (b7cd3018ef) moved that read into
on_load(:active_job)— the same hook money-rails uses. Load-hook callbacks run in registration order, and Rails registers before money-rails (a gem). So Rails reads the list while it's still empty, then money-rails appends too late .8.1.2 changed Rails' read to
on_load(:active_job_arguments)but changing money-rails to use that hook doesn't help. First, it is not compatible with older Rails version and, second, the hook could potentially be called before the custom serializer is in the list.) would break on 7.0–8.1.1.Fix
Register the serializer during initialization instead of inside a load hook, so it is present
config.active_job.custom_serializersbefore Rails reads the list regardless of how Rails consumes it.I added a simple integration test that round-trips a
MoneythroughActiveJob::Arguments@jaredmoody would you mind checking if this fixes what you are seeing? thanks!