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

incompatibility with sidekiq-failures #790

Closed
fwolfst opened this issue Jun 9, 2023 · 4 comments
Closed

incompatibility with sidekiq-failures #790

fwolfst opened this issue Jun 9, 2023 · 4 comments

Comments

@fwolfst
Copy link
Contributor

fwolfst commented Jun 9, 2023

Describe the bug
Sorry for cross-posting mhfs/sidekiq-failures#148 - I am not sure what the best way would be.

Update/TL;DR This might be due to #761 (comment) , 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

def safe_relative_time(time)

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: #761 (comment)), 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
Copy link
Contributor Author

fwolfst commented Jun 9, 2023

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

@mhenrixon
Copy link
Owner

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

fwolfst commented Nov 15, 2023

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

@leedrum
Copy link

leedrum commented Jan 2, 2024

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

mhenrixon added a commit that referenced this issue Feb 7, 2024
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

No branches or pull requests

3 participants