bensheldon / good_job

Multithreaded, Postgres-based, Active Job backend for Ruby on Rails.
https://goodjob-demo.herokuapp.com/
MIT License
2.69k stars 200 forks source link

Using string keys for cron schedule leads to RecordNotFound error #1218

Open Timmitry opened 10 months ago

Timmitry commented 10 months ago

I got an error when trying to enqueue a cron entry from the dashboard manually.

Error message:

ActiveRecord::RecordNotFound in GoodJob::CronEntriesController#enqueue 

This is raised by line 23 here:

https://github.com/bensheldon/good_job/blob/988665ee849c914a6e8edb4afb45e4bc8383203b/app/models/good_job/cron_entry.rb#L21-L25

The problem is that the GoodJob::CronEntry has a string key, and therefore entry.key == key.to_sym fails. This happens because I somehow used string keys instead of symbols for my good job configuration (which can easily be avoided, but might happen to other users):

config.good_job.cron = {
    "example" => {
      class: "SomeClass",
      cron: "every day at 04:00",
    },
  }

The issue would be fixed by adding deep_symbolize_keys here (safe navigation might be neccessary):

https://github.com/bensheldon/good_job/blob/988665ee849c914a6e8edb4afb45e4bc8383203b/lib/good_job/configuration.rb#L383-L385

Or here - note that when parsing the JSON from the env, symbolize_names is used:

https://github.com/bensheldon/good_job/blob/988665ee849c914a6e8edb4afb45e4bc8383203b/lib/good_job/configuration.rb#L205-L212

I wasn't quite sure where to best fix this tiny issue, else I would have made a PR 😇

Thank you for this great gem again 💙

bensheldon commented 9 months ago

Thanks for reporting this! It might be simplest just to raise an argument error when cron is parsed in GoodJob::CronEntry and keys are not symbols. What do you think of that?

Timmitry commented 9 months ago

Sure, why not? Since this will be easy to fix, it shouldn't be a problem to enforce symbols 🙂

ChadMoran commented 3 months ago

Why not support crons with string keys?