search

mhenrixon / sidekiq-unique-jobs

Prevents duplicate Sidekiq jobs
MIT License
1.47k stars 277 forks source link

incompatibility with sidekiq-failures #790

Closed fwolfst closed 8 months ago

fwolfst commented 1 year ago

Describe the bug Sorry for cross-posting https://github.com/mhfs/sidekiq-failures/issues/148 - I am not sure what the best way would be.

Update/TL;DR This might be due to https://github.com/mhenrixon/sidekiq-unique-jobs/issues/761#issuecomment-1557922059 , where timestamps can be nil. However, the situation is still unlucky and could re-occur, since both gems define a global helper with the same name.

sidekiq_unique_jobs has a Locks Web UI in which current locks are rendered. The page also shows when the lock was created, in a safe_relative_time. sidekiq-failures also implements a safe_relative_times. See https://github.com/mhenrixon/sidekiq-unique-jobs/blob/f381562ba7a57748132ca3e988172f3021997d7c/lib/sidekiq_unique_jobs/web/helpers.rb#L148

and https://github.com/mhfs/sidekiq-failures/blob/2b30cb1c87ce09cfb1758f90cc720ddf50dcb591/lib/sidekiq/failures/web_extension.rb#L9

However, the implementation in sidekiq_unique_jobs seems to be more robust (surviving nil because of a time.to_s in parse_time), resulting in

image

Now, I am not 100% sure why we have nils here (there is always one weird lock without a timestamp, like here: https://github.com/mhenrixon/sidekiq-unique-jobs/issues/761#issuecomment-1557922059), but if I remove sidekiq-failures from the bundle, everything runs fine.

Expected behavior Time rendered just fine.

Current behavior See screenshot above, there is a conversion error because a nil timestamp is passed on to the safe_relative_time implementation of sidekiq-failures.

fwolfst commented 1 year ago

Will happily craft a PR if you tell me in which direction you would like a solution to be done.

mhenrixon commented 11 months ago

This is indeed unfortunate, I am guessing it would be better to add sidekiq-unique-jobs last to ensure the safe relative time helper is from this gem (since sometimes the timestamp can be nil).

Apart from that, I have no good ideas at the moment.

fwolfst commented 11 months ago

I guess one way would be to somewhat namespace the helper function (SidekiqUniqueJobs::Webhelpers::safe_relative_times); or rename it ...

leedrum commented 9 months ago

This issue will be resolved by PR-152. But I don't know when it be released