-
-
Notifications
You must be signed in to change notification settings - Fork 0
feat(consumer): broker-level backoff retry middleware #19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ package rabbitmq | |
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "fmt" | ||
| "time" | ||
| ) | ||
|
|
@@ -104,3 +105,116 @@ func RetryMiddleware(maxRetries int, delay time.Duration) Middleware { | |
| } | ||
| } | ||
| } | ||
|
|
||
| // DelayedPublisher is the publish capability BackoffRetryMiddleware needs to | ||
| // schedule a retry at the broker. It is satisfied by *Publisher. | ||
| type DelayedPublisher interface { | ||
| PublishDelayedToExchange(ctx context.Context, exchange, routingKey string, msg *Message, delay time.Duration) error | ||
| } | ||
|
|
||
| // retryCountHeader carries the number of broker-level retries a message has | ||
| // already been through (see BackoffRetryMiddleware). | ||
| const retryCountHeader = "x-rabbitwrap-retry-count" | ||
|
|
||
| // BackoffRetryMiddleware retries a failed message at the broker instead of | ||
| // in-process. On failure it schedules a delayed copy of the message back onto | ||
| // queue (via PublishDelayedToExchange) and returns nil, so the consumer acks and | ||
| // removes the original — freeing the handler goroutine and its prefetch slot for | ||
| // the whole backoff. This is the counterpart to RetryMiddleware, which holds both | ||
| // while it sleeps; prefer this one for anything but short retries. | ||
| // | ||
| // The delay grows exponentially: base, 2*base, 4*base, ..., snapped up to the | ||
| // nearest DelayLadder rung and capped at the largest rung. After maxRetries | ||
| // scheduled retries the message is terminal: it is rejected without requeue — | ||
| // dead-lettered if a dead-letter exchange is configured, else discarded — | ||
| // regardless of the consumer's RequeueOnError or a handler ErrRequeue, so a | ||
| // failing message can never loop forever. A handler that returns ErrDrop opts out | ||
| // of retrying (the error is terminal immediately); every other non-nil error — | ||
| // including ErrRequeue — is retried until the retries are exhausted. | ||
| // | ||
| // queue must be a named work queue reachable via the default exchange (the retry | ||
| // is dead-lettered straight to it by name); server-named queues are unsupported. | ||
| // Place this middleware outermost in the chain so it observes the final error. | ||
| // A nil pub or empty queue disables retrying (errors pass through unchanged). | ||
| // | ||
| // Retrying is at-least-once: scheduling the copy and acking the original are not | ||
| // atomic, so a crash in between can duplicate the message. Handlers should be | ||
| // idempotent. | ||
| func BackoffRetryMiddleware(pub DelayedPublisher, queue string, maxRetries int, base time.Duration) Middleware { | ||
| if maxRetries < 0 { | ||
| maxRetries = 0 | ||
| } | ||
| return func(next MessageHandler) MessageHandler { | ||
| return func(ctx context.Context, d *Delivery) error { | ||
| err := next(ctx, d) | ||
| if err == nil { | ||
| return nil | ||
| } | ||
| if pub == nil || queue == "" || errors.Is(err, ErrDrop) { | ||
| return err | ||
| } | ||
|
|
||
| attempt := retryCount(d) | ||
| if attempt >= maxRetries { | ||
| // Bounded: an exhausted message is terminal and must never loop, so | ||
| // force no-requeue regardless of a handler ErrRequeue or the | ||
| // consumer's RequeueOnError. Wrapping ErrDrop (and keeping the | ||
| // original only as text) makes requeueDecision reject it, so it is | ||
| // dead-lettered if a DLX is configured, else discarded. | ||
| return fmt.Errorf("rabbitmq: backoff retries (%d) exhausted: %v: %w", maxRetries, err, ErrDrop) | ||
| } | ||
|
|
||
| retryMsg := d.clone() | ||
| retryMsg.Headers[retryCountHeader] = attempt + 1 | ||
|
Comment on lines
+157
to
+168
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Map relevant file(s) before reading
git ls-files | rg '^middleware\.go$|/middleware\.go$|clone|retryCount|ErrDrop'
# Read the relevant section of middleware.go
if [ -f middleware.go ]; then
sed -n '1,260p' middleware.go | cat -n
fiRepository: KARTIKrocks/rabbitwrap Length of output: 9424 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Find the definitions used by the comment
rg -n "func .*clone|type Delivery|retryCountHeader|func retryCount|ErrDrop|RequeueOnError|clone\(" -S .Repository: KARTIKrocks/rabbitwrap Length of output: 6369 🏁 Script executed: #!/bin/bash
set -euo pipefail
# Focus on the exact line range around the reported snippet if middleware.go exists
if [ -f middleware.go ]; then
sed -n '130,190p' middleware.go | cat -n
fiRepository: KARTIKrocks/rabbitwrap Length of output: 3050 🏁 Script executed: #!/bin/bash
set -euo pipefail
git ls-files | rg '^middleware\.go$|/middleware\.go$|clone|retryCount|ErrDrop'
if [ -f middleware.go ]; then
sed -n '130,190p' middleware.go | cat -n
fiRepository: KARTIKrocks/rabbitwrap Length of output: 3064 🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n "func .*clone|type Delivery|retryCountHeader|func retryCount|ErrDrop|RequeueOnError|clone\(" -S .Repository: KARTIKrocks/rabbitwrap Length of output: 6369 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '160,240p' message.go | cat -nRepository: KARTIKrocks/rabbitwrap Length of output: 2729 🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n "type MessageHandler|BackoffRetryMiddleware|RetryMiddleware|LoggingMiddleware|RecoveryMiddleware|handler\(ctx, d\)|next\(ctx, d\)" -S consumer.go middleware.go message.goRepository: KARTIKrocks/rabbitwrap Length of output: 1410 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '1,120p' message.go | cat -n
sed -n '560,640p' consumer.go | cat -nRepository: KARTIKrocks/rabbitwrap Length of output: 5963 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '160,240p' message.go | cat -n
sed -n '1,120p' message.go | cat -n
sed -n '560,640p' consumer.go | cat -nRepository: KARTIKrocks/rabbitwrap Length of output: 8532 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '160,240p' message.go | cat -n
sed -n '560,640p' consumer.go | cat -nRepository: KARTIKrocks/rabbitwrap Length of output: 5183 🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n "type MessageHandler|type Delivery struct|Message struct|zero value|nil delivery|nil message|handler receives" -S message.go consumer.go README.md middleware.goRepository: KARTIKrocks/rabbitwrap Length of output: 411 Guard nil deliveries before cloning. 🤖 Prompt for AI Agents |
||
| if schedErr := pub.PublishDelayedToExchange(ctx, "", queue, retryMsg, backoffDelay(base, attempt)); schedErr != nil { | ||
| // Couldn't schedule the retry (delayed publish / queue declare | ||
| // failed): surface that failure while still applying the original | ||
| // error's disposition (kept in the chain via %w, so an ErrRequeue | ||
| // still requeues and a plain error still dead-letters/discards). | ||
| return fmt.Errorf("rabbitmq: scheduling backoff retry failed: %v: %w", schedErr, err) | ||
| } | ||
| return nil | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // retryCount reads the broker-level retry count from a delivery's headers. AMQP | ||
| // round-trips integer headers as int32/int64, so several integer types are | ||
| // accepted; a missing, unrecognized, or negative header counts as zero (a | ||
| // negative value must not let the attempt >= maxRetries check be bypassed). | ||
| func retryCount(d *Delivery) int { | ||
| if d.Message == nil { | ||
| return 0 | ||
| } | ||
| var n int | ||
| switch v := d.Headers[retryCountHeader].(type) { | ||
| case int: | ||
| n = v | ||
| case int32: | ||
| n = int(v) | ||
| case int64: | ||
| n = int(v) | ||
| } | ||
| if n < 0 { | ||
| return 0 | ||
| } | ||
| return n | ||
| } | ||
|
|
||
| // backoffDelay returns the delay before retry number attempt (0-based): | ||
| // base * 2^attempt, with base<=0 defaulting to 1s and the result clamped to the | ||
| // largest DelayLadder rung so it never overflows or exceeds ErrDelayTooLong. | ||
| func backoffDelay(base time.Duration, attempt int) time.Duration { | ||
| if base <= 0 { | ||
| base = 1 * time.Second | ||
| } | ||
| maxDelay := delayLadder[len(delayLadder)-1] | ||
| delay := base | ||
| for i := 0; i < attempt && delay < maxDelay; i++ { | ||
| delay *= 2 | ||
| } | ||
| if delay > maxDelay { | ||
| delay = maxDelay | ||
| } | ||
| return delay | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not detach
Consumegoroutines without checking their result.Both goroutines discard
consumer.Consumeerrors and are never joined. If consumption exits early, the test times out with less context; if it exits during cleanup, the goroutine lifetime is unsynchronized. Send the result to a buffered channel, wrap unexpected errors with queue context, and wait for it aftercancel().As per coding guidelines,
**/*.go: wrap errors with context and never silently swallow them. As per path instructions,**/*.go: scrutinize goroutine lifecycle/leaks.Also applies to: 2445-2450
🤖 Prompt for AI Agents
Sources: Coding guidelines, Path instructions