Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#retry_job raises NoMethodError when used with GoodJob #92

Open
ahacop opened this issue Aug 21, 2024 · 2 comments
Open

#retry_job raises NoMethodError when used with GoodJob #92

ahacop opened this issue Aug 21, 2024 · 2 comments

Comments

@ahacop
Copy link

ahacop commented Aug 21, 2024

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:

def set(options = {}) # :nodoc:
self.scheduled_at = options[:wait].seconds.from_now.to_f if options[:wait]
self.scheduled_at = options[:wait_until].to_f if options[:wait_until]

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
Copy link
Author

ahacop commented Sep 18, 2024

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
Copy link
Owner

I definitely want the problem solved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants