fix: make relay retry limit flag work on consumer/smart router#2232
Merged
Conversation
Review Summary by QodoFix relay retry dead code and add protocol error retry limit
WalkthroughsDescription• Fix dead code in HasRequiredNodeResults that prevented --set-retry-count-on-node-error flag from working • Add new --set-retry-count-on-protocol-error flag for protocol error retry limits • Remove unreachable needsMoreRetries guard and call shouldRetryRelay directly • Add comprehensive tests for both node and protocol error retry limits Diagramflowchart LR
A["HasRequiredNodeResults<br/>with dead guard"] -->|Remove guard| B["Call shouldRetryRelay<br/>directly"]
B -->|Check node errors| C["RelayCountOnNodeError<br/>limit"]
B -->|Check protocol errors| D["RelayCountOnProtocolError<br/>limit"]
C -->|Exceeded| E["Stop retrying<br/>return error"]
D -->|Exceeded| E
C -->|Under limit| F["Continue retrying"]
D -->|Under limit| F
File Changes1. protocol/common/cobra_common.go
|
Code Review by Qodo
1. Protocol flag overridden
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on March 23
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
5e79a93 to
38b193b
Compare
38b193b to
9369f95
Compare
Review Summary by QodoFix relay retry limit flag to work on consumer/smart router
WalkthroughsDescription• Fix relay retry limit flag by removing unreachable code in HasRequiredNodeResults - Dead needsMoreRetries guard prevented shouldRetryRelay from being called - Consumers/smart routers always retried up to hardcoded limit regardless of flag • Consolidate retry policy with single RelayRetryLimit flag for all error types - Applies independently to both node errors and protocol errors - Replaces separate RelayCountOnNodeError flag with unified configuration • Move selection mode check into shouldRetryRelay for cleaner retry logic - Stateful mode always retries when no successful results - CrossValidation mode never retries in this function • Add comprehensive unit and integration tests for retry limit behavior - Test node errors, protocol errors, and mixed error scenarios - Verify consumer and smart router state machines respect retry limits Diagramflowchart LR
A["HasRequiredNodeResults<br/>called"] --> B["Remove dead<br/>needsMoreRetries guard"]
B --> C["Call shouldRetryRelay<br/>with all error types"]
C --> D["Check selection mode<br/>Stateful/CrossValidation"]
D --> E["Check RelayRetryLimit<br/>against error counts"]
E --> F["Return retry decision<br/>to state machine"]
File Changes1. protocol/common/cobra_common.go
|
Code Review by Qodo
1. Legacy retry flag removed
|
9369f95 to
ec666e8
Compare
The --set-retry-count-on-node-error flag (now --set-relay-retry-limit) was dead code on the consumer/smart router side. The shouldRetryRelay() function was unreachable inside HasRequiredNodeResults() due to a needsMoreRetries guard that was always true when reached. This meant consumers always retried up to the hardcoded MaximumNumberOfTickerRelayRetries=10 regardless of the flag value. - Remove dead needsMoreRetries guard in HasRequiredNodeResults - Move selection mode check into shouldRetryRelay for consolidated retry policy - Use single RelayRetryLimit flag for both node and protocol error limits - Add unit tests for relay processor retry limits (node + protocol errors) - Add integration tests for consumer and smart router state machines Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ec666e8 to
e4d5c2f
Compare
AnnaR-prog
previously approved these changes
Feb 25, 2026
The header was computed as protocolErrors + nodeErrors, which counts raw error responses. This is off-by-one when the final result is an error (e.g., unsupported method with no retry), reporting 1 retry when 0 occurred. Fix by computing totalAttempts - 1 instead, where totalAttempts = successes + nodeErrors + protocolErrors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 6 files with indirect coverage changes 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The --set-retry-count-on-node-error flag (now --set-relay-retry-limit)
was dead code on the consumer/smart router side. The shouldRetryRelay()
function was unreachable inside HasRequiredNodeResults() due to a
needsMoreRetries guard that was always true when reached. This meant
consumers always retried up to the hardcoded MaximumNumberOfTickerRelayRetries=10
regardless of the flag value.
Description
Closes: #XXXX
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changemainbranchReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
Note
Medium Risk
Changes core retry/stop conditions for relay processing and state machines, which can alter request behavior under error conditions; risk is mitigated by extensive new unit and integration tests covering node/protocol/mixed errors and header reporting.
Overview
Fixes consumer/smart-router relay retry behavior by replacing the node-error-only retry knob with a unified
RelayRetryLimitthat caps total retries across node + protocol errors (with epoch-mismatch still always retrying), and by makingHasRequiredNodeResultsactually consult the consolidatedshouldRetryRelaypolicy.Updates the consumer and smart router CLIs to use
--set-relay-retry-limit, and adjusts theLava-Retriesheader to report actual retries astotal attempts - 1rather than raw error counts. Adds targeted tests for relay-processor retry limiting (node/protocol/mixed), end-to-end state machine stopping behavior, and retry header correctness.Written by Cursor Bugbot for commit 108fa70. This will update automatically on new commits. Configure here.