Conversation
| return true | ||
| } | ||
|
|
||
| // otherwise check if the error is retryable. | ||
| return isRetryableError(err) | ||
| // otherwise check if the error is retryable for the current channel. | ||
| return isRetryableErrorForChannel(err, p.state.CurrentCandidate.Channel) |
There was a problem hiding this comment.
🚩 CanRetry now checks channel-specific codes only for the last-model fallback path
In outbound.go:608-614, the channel-specific retryable status codes are only consulted in the final fallback of CanRetry — after checking for circuit breaker, queue errors, empty responses, 429 with/without Retry-After, and remaining models. This means a channel-configured code like 400 will trigger same-channel retry only when there are no more models to try in the candidate. If the candidate has multiple models, ANY error (including non-retryable ones) at line 609 will advance to the next model regardless. This is the pre-existing design — the PR correctly extends only the final fallback path — but reviewers should confirm this is the intended semantics.
(Refers to lines 608-614)
Was this helpful? React with 👍 or 👎 to provide feedback.
Greptile SummaryThis PR adds per-channel configurable retryable HTTP status codes, allowing users to specify additional 4xx/5xx codes beyond the default 429 and 5xx retry behavior. The feature spans the full stack — GraphQL schema, Go backend (channel settings object, validation/normalization, retry logic), and a React frontend form field with real-time input parsing.
Confidence Score: 5/5Safe to merge — all code paths that accept or use channel settings correctly validate and apply the new field. The feature is well-scoped: validation runs in both create and update paths, the retry logic only adds new retryable codes on top of the existing defaults without removing any, and the frontend input parsing mirrors the backend constraints. Tests cover the key edge cases (nil channel, nil settings, default fallthrough, configured match, and non-match). No existing behavior is changed for channels that don't configure the new field. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Outbound request fails] --> B{CanRetry?}
B --> C[isRetryableErrorForChannel]
C --> D{err == nil?}
D -- yes --> E[return false]
D -- no --> F[ExtractStatusCodeFromError]
F --> G{IsHTTPStatusCodeRetryable?\n429 or 5xx}
G -- yes --> H[return true]
G -- no --> I{ch == nil OR ch.Channel == nil OR ch.Settings == nil?}
I -- yes --> J[return false]
I -- no --> K{slices.Contains channel RetryableStatusCodes?}
K -- yes --> L[return true]
K -- no --> M[return false]
N[Channel settings saved] --> O[NormalizeRetryableStatusCodes]
O --> P{codes empty?}
P -- yes --> Q[no-op, return nil]
P -- no --> R{any code outside 400-599?}
R -- yes --> S[return error]
R -- no --> T[sort + compact dedup]
T --> U[assign back to settings]
Reviews (2): Last reviewed commit: "feat: channel retryable statu codes, clo..." | Re-trigger Greptile |
| const tokens = value | ||
| .split(/[,\s]+/) | ||
| .map((token) => token.trim()) | ||
| .filter(Boolean); |
There was a problem hiding this comment.
.map((token) => token.trim()) call is redundant here: the split regex /[,\s]+/ already consumes all surrounding whitespace, so tokens can never have leading/trailing spaces after the split. The .filter(Boolean) already handles any empty strings at the boundaries. Removing the .trim() keeps the code consistent with the actual contract of the regex.
| const tokens = value | |
| .split(/[,\s]+/) | |
| .map((token) => token.trim()) | |
| .filter(Boolean); | |
| const tokens = value | |
| .split(/[,\s]+/) | |
| .filter(Boolean); |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| "channels.dialogs.bodyPassThrough.warning": "When enabled, both the request and response will be passed through only when the client request format matches the channel format. Prompt injection and prompt protection will not take effect.", | ||
| "channels.dialogs.retryableStatusCodes.label": "Retry Status Codes", | ||
| "channels.dialogs.retryableStatusCodes.placeholder": "400, 401, 403", | ||
| "channels.dialogs.retryableStatusCodes.tooltip": "Additional HTTP error status codes to retry for this channel. Empty keeps the default 429 and 5xx behavior.", |
There was a problem hiding this comment.
| "channels.dialogs.retryableStatusCodes.tooltip": "Additional HTTP error status codes to retry for this channel. Empty keeps the default 429 and 5xx behavior.", | |
| "channels.dialogs.retryableStatusCodes.tooltip": "Additional HTTP error status codes to retry for this channel. These are added on top of the default 429 and 5xx behavior, which always applies.", |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Uh oh!
There was an error while loading. Please reload this page.