Skip to content

Conversation

@mensfeld
Copy link
Collaborator

@mensfeld mensfeld commented Dec 13, 2025

FIFO queues in SQS do not support per-message DelaySeconds. This caused confusing AWS errors when using ActiveJob's retry_on with FIFO queues, since Rails 6.1+ defaults to wait: 3.seconds.

Add early validation in enqueue_at to raise a clear ArgumentError with actionable guidance when attempting to use delays with FIFO queues. The check happens before any async wrapping to ensure synchronous failure.

Fixes #924

close #924

Summary by CodeRabbit

Release Notes

  • New Features

    • Added validation for FIFO queues that raises an error when delays are used, preventing incompatible configurations and providing clearer feedback.
  • Documentation

    • Updated documentation to clarify FIFO queue behavior with delays and retry mechanisms.
  • Tests

    • Added test coverage for FIFO queue delay validation behavior.

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

@coderabbitai
Copy link

coderabbitai bot commented Dec 13, 2025

Warning

Rate limit exceeded

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

⌛ 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 8acf7a2 and 04f4a29.

📒 Files selected for processing (3)
  • CHANGELOG.md (1 hunks)
  • lib/active_job/queue_adapters/shoryuken_adapter.rb (1 hunks)
  • spec/shared_examples_for_active_job.rb (1 hunks)

Walkthrough

The Shoryuken adapter's enqueue_at method now validates per-message delays for FIFO queues, raising ArgumentError if a positive delay is detected with a FIFO queue. New test cases verify this validation behavior alongside existing enqueue_at functionality.

Changes

Cohort / File(s) Change Summary
FIFO Queue Delay Validation
lib/active_job/queue_adapters/shoryuken_adapter.rb
Adds validation in enqueue_at to inspect target queue and raise ArgumentError when a positive delay is used with FIFO queues; preserves existing behavior for non-positive delays and non-FIFO queues. Documentation updated to note the new exception.
FIFO Queue Test Coverage
spec/shared_examples_for_active_job.rb
Adds new FIFO-specific test context for enqueue_at that verifies ArgumentError is raised for positive delays and that send_message is invoked for zero delays.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Straightforward validation logic checking queue type and delay value
  • Test additions follow existing FIFO test patterns
  • Minimal code changes with clear intent
  • Focus areas: verify queue inspection logic is accurate and exception message clarity

Poem

🐰 A rabbit hops through queues so fine,
But FIFO needs no delayed time,
We catch delays before they start,
And keep those messages right on chart! ✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately and concisely describes the main change: adding ArgumentError validation when delays are used with FIFO queues.
Linked Issues check ✅ Passed The PR implements a solution matching issue #924 requirements: raises ArgumentError with clear guidance when delays are used with FIFO queues, fixing the incompatibility.
Out of Scope Changes check ✅ Passed All changes are directly related to the linked issue: validation in enqueue_at for FIFO delays and corresponding test coverage in shared_examples.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4b11bd3 and 8acf7a2.

📒 Files selected for processing (2)
  • lib/active_job/queue_adapters/shoryuken_adapter.rb (1 hunks)
  • spec/shared_examples_for_active_job.rb (1 hunks)
🧰 Additional context used
🪛 RuboCop (1.81.7)
spec/shared_examples_for_active_job.rb

[convention] 292-292: Line is too long. [128/125]

(Layout/LineLength)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Integrations (3.2)
  • GitHub Check: Integrations (3.3)
  • GitHub Check: Integrations (3.4)
  • GitHub Check: Integrations (3.4)
  • GitHub Check: Integrations (3.3)
  • GitHub Check: Integrations (3.2)

@mensfeld mensfeld force-pushed the fix/fifo-queue-delay-error-924 branch 2 times, most recently from 6833c97 to d2d9006 Compare December 13, 2025 16:46
@mensfeld mensfeld enabled auto-merge (squash) December 13, 2025 16:50
@mensfeld mensfeld disabled auto-merge December 13, 2025 16:50
FIFO queues in SQS do not support per-message DelaySeconds. This caused
confusing AWS errors when using ActiveJob's retry_on with FIFO queues,
since Rails 6.1+ defaults to wait: 3.seconds.

Add early validation in enqueue_at to raise a clear ArgumentError with
actionable guidance when attempting to use delays with FIFO queues.
The check happens before any async wrapping to ensure synchronous failure.

Fixes #924

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@mensfeld mensfeld force-pushed the fix/fifo-queue-delay-error-924 branch from d2d9006 to 04f4a29 Compare December 13, 2025 16:54
@mensfeld mensfeld merged commit 7bf2848 into main Dec 13, 2025
17 checks passed
@mensfeld mensfeld deleted the fix/fifo-queue-delay-error-924 branch December 13, 2025 17:02
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.

Bug/Doc: FIFO queues incompatible with ActiveJob retry_on

2 participants