search

fractaledmind / acidic_job

Idempotent operations for Rails apps, built for ActiveJob or Sidekiq.
https://fractaledmind.github.io/acidic_job/
MIT License
447 stars 10 forks source link

#retry_job raises NoMethodError when used with GoodJob #92

Open ahacop opened 1 month ago

ahacop commented 1 month ago

When used with GoodJob, retrying a failed job raises a NoMethodError: "undefined method `utc' for an instance of Float".

Backtrace:

Error:
RetryJobTest#test_job_with_AcidicJob_mixed_in_raises_on_retry_with_GoodJob:
NoMethodError: undefined method `utc' for an instance of Float
    activejob (7.2.0) lib/active_job/core.rb:120:in `serialize'
    good_job (4.2.0) app/models/good_job/job.rb:242:in `enqueue_args'
    good_job (4.2.0) app/models/good_job/job.rb:346:in `block in enqueue'
    activesupport (7.2.0) lib/active_support/notifications.rb:212:in `instrument'
    good_job (4.2.0) app/models/good_job/job.rb:340:in `enqueue'
    good_job (4.2.0) lib/good_job/adapter.rb:161:in `block in enqueue_at'
    activesupport (7.2.0) lib/active_support/execution_wrapper.rb:91:in `wrap'
    good_job (4.2.0) lib/good_job/adapter.rb:140:in `enqueue_at'
    activejob (7.2.0) lib/active_job/enqueuing.rb:131:in `raw_enqueue'
    activejob (7.2.0) lib/active_job/enqueue_after_transaction_commit.rb:24:in `raw_enqueue'
    activejob (7.2.0) lib/active_job/enqueuing.rb:118:in `block in enqueue'
    activesupport (7.2.0) lib/active_support/callbacks.rb:121:in `block in run_callbacks'
    activejob (7.2.0) lib/active_job/instrumentation.rb:40:in `block in instrument'
    activesupport (7.2.0) lib/active_support/notifications.rb:210:in `block in instrument'
    activesupport (7.2.0) lib/active_support/notifications/instrumenter.rb:58:in `instrument'
    activesupport (7.2.0) lib/active_support/notifications.rb:210:in `instrument'
    activejob (7.2.0) lib/active_job/instrumentation.rb:39:in `instrument'
    activerecord (7.2.0) lib/active_record/railties/job_runtime.rb:18:in `instrument'
    activejob (7.2.0) lib/active_job/instrumentation.rb:21:in `block (2 levels) in <module:Instrumentation>'
    activesupport (7.2.0) lib/active_support/callbacks.rb:130:in `instance_exec'
    activesupport (7.2.0) lib/active_support/callbacks.rb:130:in `block in run_callbacks'
    activesupport (7.2.0) lib/active_support/tagged_logging.rb:138:in `block in tagged'
    activesupport (7.2.0) lib/active_support/tagged_logging.rb:38:in `tagged'
    activesupport (7.2.0) lib/active_support/tagged_logging.rb:138:in `tagged'
    activesupport (7.2.0) lib/active_support/broadcast_logger.rb:241:in `method_missing'
    activejob (7.2.0) lib/active_job/logging.rb:39:in `tag_logger'
    activejob (7.2.0) lib/active_job/logging.rb:28:in `block (2 levels) in <module:Logging>'
    activesupport (7.2.0) lib/active_support/callbacks.rb:130:in `instance_exec'
    activesupport (7.2.0) lib/active_support/callbacks.rb:130:in `block in run_callbacks'
    activesupport (7.2.0) lib/active_support/callbacks.rb:141:in `run_callbacks'
    activejob (7.2.0) lib/active_job/enqueuing.rb:117:in `enqueue'
    activejob (7.2.0) lib/active_job/exceptions.rb:153:in `block in retry_job'
    activejob (7.2.0) lib/active_job/instrumentation.rb:40:in `block in instrument'
    activesupport (7.2.0) lib/active_support/notifications.rb:210:in `block in instrument'
    activesupport (7.2.0) lib/active_support/notifications/instrumenter.rb:58:in `instrument'
    activesupport (7.2.0) lib/active_support/notifications.rb:210:in `instrument'
    activejob (7.2.0) lib/active_job/instrumentation.rb:39:in `instrument'
    activerecord (7.2.0) lib/active_record/railties/job_runtime.rb:18:in `instrument'
    activejob (7.2.0) lib/active_job/exceptions.rb:152:in `retry_job'
    good_job (4.2.0) app/models/good_job/job.rb:481:in `block (3 levels) in retry_job'
    activerecord (7.2.0) lib/active_record/connection_adapters/abstract/transaction.rb:616:in `block in within_new_transaction'
    activesupport (7.2.0) lib/active_support/concurrency/load_interlock_aware_monitor.rb:23:in `handle_interrupt'
    activesupport (7.2.0) lib/active_support/concurrency/load_interlock_aware_monitor.rb:23:in `block in synchronize'
    activesupport (7.2.0) lib/active_support/concurrency/load_interlock_aware_monitor.rb:19:in `handle_interrupt'
    activesupport (7.2.0) lib/active_support/concurrency/load_interlock_aware_monitor.rb:19:in `synchronize'
    activerecord (7.2.0) lib/active_record/connection_adapters/abstract/transaction.rb:613:in `within_new_transaction'
    activerecord (7.2.0) lib/active_record/connection_adapters/abstract/database_statements.rb:361:in `transaction'
    activerecord (7.2.0) lib/active_record/transactions.rb:234:in `block in transaction'
    activerecord (7.2.0) lib/active_record/connection_adapters/abstract/connection_pool.rb:407:in `with_connection'
    activerecord (7.2.0) lib/active_record/connection_handling.rb:296:in `with_connection'
    activerecord (7.2.0) lib/active_record/transactions.rb:233:in `transaction'
    good_job (4.2.0) app/models/good_job/job.rb:480:in `block (2 levels) in retry_job'
    good_job (4.2.0) lib/good_job/current_thread.rb:113:in `within'
    good_job (4.2.0) app/models/good_job/job.rb:476:in `block in retry_job'
    good_job (4.2.0) app/models/concerns/good_job/advisory_lockable.rb:368:in `with_advisory_lock'
    good_job (4.2.0) app/models/good_job/job.rb:456:in `retry_job'
    test/jobs/retry_job_test.rb:36:in `block in <class:RetryJobTest>'
    minitest (5.25.1) lib/minitest/test.rb:94:in `block (2 levels) in run'
    minitest (5.25.1) lib/minitest/test.rb:190:in `capture_exceptions'
    minitest (5.25.1) lib/minitest/test.rb:89:in `block in run'
    minitest (5.25.1) lib/minitest.rb:367:in `time_it'
    minitest (5.25.1) lib/minitest/test.rb:88:in `run'
    activesupport (7.2.0) lib/active_support/executor/test_helper.rb:5:in `block in run'
    activesupport (7.2.0) lib/active_support/execution_wrapper.rb:104:in `perform'
    activesupport (7.2.0) lib/active_support/executor/test_helper.rb:5:in `run'
    minitest (5.25.1) lib/minitest.rb:1207:in `run_one_method'
    minitest (5.25.1) lib/minitest.rb:446:in `run_one_method'
    minitest (5.25.1) lib/minitest.rb:433:in `block (2 levels) in run'
    minitest (5.25.1) lib/minitest.rb:429:in `each'
    minitest (5.25.1) lib/minitest.rb:429:in `block in run'
    minitest (5.25.1) lib/minitest.rb:471:in `on_signal'
    minitest (5.25.1) lib/minitest.rb:458:in `with_info_handler'
    minitest (5.25.1) lib/minitest.rb:428:in `run'
    railties (7.2.0) lib/rails/test_unit/line_filtering.rb:10:in `run'
    minitest (5.25.1) lib/minitest.rb:331:in `block in __run'
    minitest (5.25.1) lib/minitest.rb:331:in `map'
    minitest (5.25.1) lib/minitest.rb:331:in `__run'
    minitest (5.25.1) lib/minitest.rb:287:in `run'
    minitest (5.25.1) lib/minitest.rb:85:in `block in autorun'

Removing the to_f's here, fixes the problem: https://github.com/fractaledmind/acidic_job/blob/f49e6b311aa36efd542aab8c5aa1a2a71d101201/lib/acidic_job/mixin.rb#L65-L67

The overridden ActiveJob::Core#set doesn't cast the timestamps: https://github.com/rails/rails/blob/888d28460f5bd1444eeb171cb827cd91739c3759/activejob/lib/active_job/core.rb#L165-L167

Here is a repro: https://github.com/ahacop/acidic-job-good-job-repro, with a test that demonstrates the issue.

Curious to hear your thoughts. I don't know what the consequences are of removing these casts, although no tests fail. Are these to_f's, perhaps, necessary for things to work with Sidekiq?

ahacop commented 5 days ago

The same error seems to happen when using SolidQueue and attempting to schedule an acidic job to occur at a later date (using set(wait: 1.minute) for example). The issue is resolved by removing the casts. Happy to submit the PR if you would accept it.

fractaledmind commented 5 days ago

I definitely want the problem solved