fix(lock): prevent false execution_failed when worker returns nil/false

[mhenrixon]

Summary

• UntilExpired#execute passed the raw worker block to locksmith.execute, using the worker's return value to determine success
• If the worker returned nil or false (e.g. early exit with return unless condition), execution_failed was incorrectly reflected
• Fix wraps the block to return item[JID] (always truthy when locked), matching the pattern already used by WhileExecuting and UntilExecuted

Closes #885

Test plan

• New test: worker returning nil does not trigger execution_failed
• New test: worker returning false does not trigger execution_failed
• Full suite: 1052 examples, 0 failures
• Rubocop: 282 files, no offenses

Summary by CodeRabbit

• Refactor

  • Updated internal job execution handling in unique job locking mechanism.

• Tests

  • Enhanced test coverage for job execution edge cases, including scenarios where workers return nil or false values.

[coderabbitai]

Warning

Rate limit exceeded

@mhenrixon has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 9 minutes and 2 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 9 minutes and 2 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0b025b5c-e854-4a09-ae53-d21d137e9579

📥 Commits

Reviewing files that changed from the base of the PR and between b6619db and 9f03de4.

📒 Files selected for processing (2)

• lib/sidekiq_unique_jobs/lock/until_expired.rb
• spec/sidekiq_unique_jobs/lock/until_expired_spec.rb

📝 Walkthrough

Walkthrough

The UntilExpired#execute method signature was updated to no longer explicitly accept a block parameter. The block execution was moved inside the locksmith.execute block, and the return logic now uses item[JID] instead of the block's return value, preventing falsey worker returns from triggering execution_failed reflections.

Changes

Cohort / File(s)	Summary
Lock Implementation lib/sidekiq_unique_jobs/lock/until_expired.rb	Modified execute method to wrap block execution within locksmith.execute and return item[JID] instead of relying on block return value for execution status determination.
Test Coverage spec/sidekiq_unique_jobs/lock/until_expired_spec.rb	Added test contexts verifying that reflect(:execution_failed, ...) is not called when the worker returns nil or false.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• PR #931: Applies the same execution-within-locksmith pattern to UntilExecuted lock class for consistency.

Poem

🐰 No more false alarms when workers exit neat,
Early returns now pass the honest beat,
The block yields inside where it should stay,
No phantom failures lead the queue astray! ✨

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main fix: preventing false execution_failed reflections when workers return nil or false values.
Linked Issues check	✅ Passed	The PR successfully addresses issue #885 by wrapping the worker block to return item[JID] instead of relying on the worker's return value, and tests confirm nil/false returns no longer trigger execution_failed.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing issue #885; only UntilExpired#execute method and its spec were modified with no unrelated alterations.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}