Skip to content

Conversation

@Saidbek
Copy link

@Saidbek Saidbek commented Feb 1, 2026

Fixes #948

Replaces ArgumentError with a custom FifoDelayNotSupportedError when attempting to use delay with FIFO queues.

Summary by CodeRabbit

  • Bug Fixes
    • Improved error handling when using delays with FIFO queues, now raises a more specific error type with clearer guidance on FIFO queue limitations and how to resolve the issue.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Feb 1, 2026

📝 Walkthrough

Walkthrough

This PR replaces the generic ArgumentError with a custom Shoryuken::Errors::FifoDelayNotSupportedError when attempting to use per-message delays with FIFO queues. The change involves adding a new error constant, updating the adapter to raise it, and adjusting corresponding test expectations.

Changes

Cohort / File(s) Summary
Error Definition
lib/shoryuken/errors.rb
Introduces new error constant FifoDelayNotSupportedError as a subclass of BaseError for FIFO queue delay scenarios.
Adapter Implementation
lib/active_job/queue_adapters/shoryuken_adapter.rb
Replaces ArgumentError with Shoryuken::Errors::FifoDelayNotSupportedError when a positive delay is applied to FIFO queues.
Test Updates
spec/shared_examples_for_active_job.rb
Updates test expectations to verify the new FifoDelayNotSupportedError exception type with the same error message pattern.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

Poem

A rabbit hops through error trees,
No more generic sighs and pleas—
Now FIFO knows its proper way,
With custom errors—hip hooray! 🐰✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: replacing ArgumentError with a custom FifoDelayNotSupportedError for FIFO queue delays.
Linked Issues check ✅ Passed The PR successfully implements the objective from issue #948 by introducing FifoDelayNotSupportedError and replacing ArgumentError with this custom error throughout the codebase.
Out of Scope Changes check ✅ Passed All changes are directly related to the linked issue objective. The modifications include the new error class, adapter implementation, and test updates—all necessary for the stated goal.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@mensfeld mensfeld requested review from Copilot and mensfeld and removed request for mensfeld February 1, 2026 12:20
@mensfeld mensfeld self-assigned this Feb 1, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR replaces the generic ArgumentError with a custom domain-specific FifoDelayNotSupportedError when attempting to use message delays with FIFO queues, which don't support per-message delays. This change aligns with the broader effort to introduce custom error types in the Shoryuken gem, making error handling more explicit and easier to rescue.

Changes:

  • Added FifoDelayNotSupportedError as a new custom error class in the Shoryuken::Errors module
  • Updated the ActiveJob adapter to raise the new custom error instead of ArgumentError when delays are used with FIFO queues
  • Updated test expectations to verify the new error type is raised

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
lib/shoryuken/errors.rb Defines the new FifoDelayNotSupportedError error class
lib/active_job/queue_adapters/shoryuken_adapter.rb Updates error raising logic and documentation to use the new custom error
spec/shared_examples_for_active_job.rb Updates test expectations to verify the new error type

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

Successfully merging this pull request may close these issues.

Raise custom for FIFO instead of ArgumentError

2 participants