Skip to content

Draft: Add cop to avoid using find in sidekiq perform

Sashi Kumar Kumaresan requested to merge sk/405061-add-cop into master

What does this MR do and why?

Calling Model.find(id) raises ActiveRecord::RecordNotFound exception when id does not exist in database. If we have a sidekiq worker with id argument(s) and calling .find() raises the exception which would result in the job being failed and getting retried for 25 times if a retry value was not set. The job would fail even on retry, this would take a toll on the error budget.

Some workers handle this by rescuing exception.

Even though we have mentioned in our docs:

# Bad
def perform(project_id)
  project = Project.find(project_id)
  # ...
end

# Good
def perform(project_id)
  project = Project.find_by_id(project_id)
  return unless project
  # ...
end

Some workers still use .find(). One such case happened with Security::SyncScanPoliciesWorker where we have .find() and the failures of retried jobs are taking a toll on our error budget.

Addresses Proposal: Create cop to avoid using find() of j... (#405061 - closed)

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Sashi Kumar Kumaresan

Merge request reports