Skip to content

Early exit from job triggers execution_failed #885

@brayden-onesignal

Description

@brayden-onesignal

Describe the bug
Returning nil or a falsey value will trigger the execution_failed reflection. In our system, we capture all execution_failed and send them to our error reporting tool. Currently, this is causing us waves of false errors when we exit early or return anything falsey.

An easy workaround is to change all early returns to return true if some_condition, but that's not ideal in the long run as it differs from the expected behavior of a traditional Sidekiq perform function.

Expected behavior
I expect the early exit to return and exit gracefully.

Current behavior
Job returns, but it triggers the execution_failed reflection.

Worker class

class SillyWorker
  include Sidekiq::Worker

  sidekiq_options lock: :until_expired, lock_ttl: 1.second

  def perform(do_success = true)
    return unless do_success

    logger.info("cowboy")
    sleep(1) # hardcore processing
    logger.info("beebop")
  end
end
SillyWorker.perform_async(true) # => all good
SillyWorker.perform_async(false) # => triggers execution_failed

Additional context
From what I understand, the issue has to do with the checks in the lock executions. For example, until_expired has this block:

      def execute(&block)
        executed = locksmith.execute(&block)

        reflect(:execution_failed, item) unless executed
      end

Since executed would be nil in the case of the early returns, it fails.
What I'm curious about is whether locksmith.execute(...) should return the response of the job's perform() method. I'd imagine the response of perform would be irrelevant to the locking mechanism. Looking to verify if that should be the case or not.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions