Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions controller/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,15 @@ func UpdateOption(c *gin.Context) {
})
return
}
case "AutomaticRetryStatusCodes":
_, err = operation_setting.ParseHTTPStatusCodeRanges(option.Value.(string))
if err != nil {
c.JSON(http.StatusOK, gin.H{
"success": false,
"message": err.Error(),
})
return
}
case "console_setting.api_info":
err = console_setting.ValidateConsoleSettings(option.Value.(string), "ApiInfo")
if err != nil {
Expand Down
27 changes: 6 additions & 21 deletions controller/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"github.com/QuantumNous/new-api/relay/helper"
"github.com/QuantumNous/new-api/service"
"github.com/QuantumNous/new-api/setting"
"github.com/QuantumNous/new-api/setting/operation_setting"
"github.com/QuantumNous/new-api/types"

"github.com/bytedance/gopkg/util/gopool"
Expand Down Expand Up @@ -316,30 +317,14 @@ func shouldRetry(c *gin.Context, openaiErr *types.NewAPIError, retryTimes int) b
if _, ok := c.Get("specific_channel_id"); ok {
return false
}
if openaiErr.StatusCode == http.StatusTooManyRequests {
return true
}
if openaiErr.StatusCode == 307 {
return true
}
if openaiErr.StatusCode/100 == 5 {
// 超时不重试
if openaiErr.StatusCode == 504 || openaiErr.StatusCode == 524 {
return false
}
return true
}
if openaiErr.StatusCode == http.StatusBadRequest {
return false
}
if openaiErr.StatusCode == 408 {
// azure处理超时不重试
code := openaiErr.StatusCode
if code >= 200 && code < 300 {
return false
}
if openaiErr.StatusCode/100 == 2 {
return false
if code < 100 || code > 599 {
return true
}
return true
return operation_setting.ShouldRetryByStatusCode(code)
}

func processChannelError(c *gin.Context, channelError types.ChannelError, err *types.NewAPIError) {
Expand Down
3 changes: 3 additions & 0 deletions model/option.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ func InitOptionMap() {
common.OptionMap["StreamCacheQueueLength"] = strconv.Itoa(setting.StreamCacheQueueLength)
common.OptionMap["AutomaticDisableKeywords"] = operation_setting.AutomaticDisableKeywordsToString()
common.OptionMap["AutomaticDisableStatusCodes"] = operation_setting.AutomaticDisableStatusCodesToString()
common.OptionMap["AutomaticRetryStatusCodes"] = operation_setting.AutomaticRetryStatusCodesToString()
common.OptionMap["ExposeRatioEnabled"] = strconv.FormatBool(ratio_setting.IsExposeRatioEnabled())

// 自动添加所有注册的模型配置
Expand Down Expand Up @@ -447,6 +448,8 @@ func updateOptionMap(key string, value string) (err error) {
operation_setting.AutomaticDisableKeywordsFromString(value)
case "AutomaticDisableStatusCodes":
err = operation_setting.AutomaticDisableStatusCodesFromString(value)
case "AutomaticRetryStatusCodes":
err = operation_setting.AutomaticRetryStatusCodesFromString(value)
case "StreamCacheQueueLength":
setting.StreamCacheQueueLength, _ = strconv.Atoi(value)
case "PayMethods":
Expand Down
63 changes: 50 additions & 13 deletions setting/operation_setting/status_code_ranges.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,20 @@ type StatusCodeRange struct {

var AutomaticDisableStatusCodeRanges = []StatusCodeRange{{Start: 401, End: 401}}

// Default behavior matches legacy hardcoded retry rules in controller/relay.go shouldRetry:
// retry for 1xx, 3xx, 4xx(except 400/408), 5xx(except 504/524), and no retry for 2xx.
var AutomaticRetryStatusCodeRanges = []StatusCodeRange{
{Start: 100, End: 199},
{Start: 300, End: 399},
{Start: 401, End: 407},
{Start: 409, End: 499},
{Start: 500, End: 503},
{Start: 505, End: 523},
{Start: 525, End: 599},
}
Comment on lines +17 to +27

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, locate and read the file in question
fd -type f status_code_ranges.go

Repository: QuantumNous/new-api

Length of output: 233


🏁 Script executed:

# Search for usage of AutomaticRetryStatusCodeRanges and AutomaticDisableStatusCodeRanges
rg "AutomaticRetryStatusCodeRanges|AutomaticDisableStatusCodeRanges" --type go -B 2 -A 2

Repository: QuantumNous/new-api

Length of output: 4279


🏁 Script executed:

# Search for operation settings updates and locking mechanisms
rg "operation.*[Ss]etting" --type go | head -30

Repository: QuantumNous/new-api

Length of output: 2793


🏁 Script executed:

# Look for "shouldRetry" and references to controller/relay.go
rg "shouldRetry|controller/relay" --type go -B 2 -A 2 | head -50

Repository: QuantumNous/new-api

Length of output: 1870


🏁 Script executed:

# Check if there's a config or settings struct with locks/atomics
rg "sync\.(Mutex|RWMutex|Atomic)" --type go -B 2 -A 2 | head -40

Repository: QuantumNous/new-api

Length of output: 2242


🏁 Script executed:

# Read the complete status_code_ranges.go file
cat setting/operation_setting/status_code_ranges.go

Repository: QuantumNous/new-api

Length of output: 4573


🏁 Script executed:

# Check for any locking mechanism specific to operation_setting package
rg "Mutex" setting/operation_setting/ --type go

Repository: QuantumNous/new-api

Length of output: 45


Major: data race and sortedness invariant in global mutable status code ranges.

  1. Data race: AutomaticRetryStatusCodeRanges and AutomaticDisableStatusCodeRanges are unprotected exported globals. They are read in request paths (e.g., service/channel.go calls ShouldDisableByStatusCode) and written via SetAutomaticRetryStatusCodes/SetAutomaticDisableStatusCodes. Concurrent reads/writes are a Go data race. Other operation settings modules (e.g., setting/ratio_setting/model_ratio.go, setting/user_usable_group.go) consistently protect mutable globals with sync.RWMutex.

  2. Sortedness invariant: shouldMatchStatusCodeRanges relies on sorted ranges—it returns false when code < r.Start, skipping later ranges. ParseHTTPStatusCodeRanges enforces sorting, but the globals can be assigned directly (as tests do), bypassing this guarantee and silently breaking the matching logic.

Fix: Add sync.RWMutex around both globals (consistent with other settings), and make shouldMatchStatusCodeRanges order-independent:

Proposed changes
-func shouldMatchStatusCodeRanges(ranges []StatusCodeRange, code int) bool {
+func shouldMatchStatusCodeRanges(ranges []StatusCodeRange, code int) bool {
 	if code < 100 || code > 599 {
 		return false
 	}
 	for _, r := range ranges {
-		if code < r.Start {
-			return false
-		}
-		if code <= r.End {
+		if code >= r.Start && code <= r.End {
 			return true
 		}
 	}
 	return false
 }

Also applies to: 35-52 (update functions), 74-84 (shouldMatchStatusCodeRanges)

🤖 Prompt for AI Agents
In `@setting/operation_setting/status_code_ranges.go` around lines 17 - 27,
AutomaticRetryStatusCodeRanges and AutomaticDisableStatusCodeRanges are exported
mutable globals causing a data race and a broken sortedness invariant; add a
package-level sync.RWMutex (e.g., statusCodeRangesMu) and use it to protect all
reads/writes of these globals (acquire write lock in
SetAutomaticRetryStatusCodes and SetAutomaticDisableStatusCodes, acquire read
lock in ShouldDisableByStatusCode and any readers). Also ensure setters
validate/sort the slices (or store them in a normalized form) so the sortedness
invariant holds, and make shouldMatchStatusCodeRanges robust to unsorted input
by either sorting a copy under the lock or by scanning all ranges instead of
bailing early when code < r.Start; update the
SetAutomaticRetryStatusCodes/SetAutomaticDisableStatusCodes and
shouldMatchStatusCodeRanges implementations accordingly.


func AutomaticDisableStatusCodesToString() string {
if len(AutomaticDisableStatusCodeRanges) == 0 {
return ""
}
parts := make([]string, 0, len(AutomaticDisableStatusCodeRanges))
for _, r := range AutomaticDisableStatusCodeRanges {
if r.Start == r.End {
parts = append(parts, strconv.Itoa(r.Start))
continue
}
parts = append(parts, fmt.Sprintf("%d-%d", r.Start, r.End))
}
return strings.Join(parts, ",")
return statusCodeRangesToString(AutomaticDisableStatusCodeRanges)
}

func AutomaticDisableStatusCodesFromString(s string) error {
Expand All @@ -39,10 +40,46 @@ func AutomaticDisableStatusCodesFromString(s string) error {
}

func ShouldDisableByStatusCode(code int) bool {
return shouldMatchStatusCodeRanges(AutomaticDisableStatusCodeRanges, code)
}

func AutomaticRetryStatusCodesToString() string {
return statusCodeRangesToString(AutomaticRetryStatusCodeRanges)
}

func AutomaticRetryStatusCodesFromString(s string) error {
ranges, err := ParseHTTPStatusCodeRanges(s)
if err != nil {
return err
}
AutomaticRetryStatusCodeRanges = ranges
return nil
}

func ShouldRetryByStatusCode(code int) bool {
return shouldMatchStatusCodeRanges(AutomaticRetryStatusCodeRanges, code)
}

func statusCodeRangesToString(ranges []StatusCodeRange) string {
if len(ranges) == 0 {
return ""
}
parts := make([]string, 0, len(ranges))
for _, r := range ranges {
if r.Start == r.End {
parts = append(parts, strconv.Itoa(r.Start))
continue
}
parts = append(parts, fmt.Sprintf("%d-%d", r.Start, r.End))
}
return strings.Join(parts, ",")
}

func shouldMatchStatusCodeRanges(ranges []StatusCodeRange, code int) bool {
if code < 100 || code > 599 {
return false
}
for _, r := range AutomaticDisableStatusCodeRanges {
for _, r := range ranges {
if code < r.Start {
return false
}
Expand Down
27 changes: 27 additions & 0 deletions setting/operation_setting/status_code_ranges_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,30 @@ func TestShouldDisableByStatusCode(t *testing.T) {
require.True(t, ShouldDisableByStatusCode(500))
require.False(t, ShouldDisableByStatusCode(200))
}

func TestShouldRetryByStatusCode(t *testing.T) {
orig := AutomaticRetryStatusCodeRanges
t.Cleanup(func() { AutomaticRetryStatusCodeRanges = orig })

AutomaticRetryStatusCodeRanges = []StatusCodeRange{
{Start: 429, End: 429},
{Start: 500, End: 599},
}

require.True(t, ShouldRetryByStatusCode(429))
require.True(t, ShouldRetryByStatusCode(500))
require.False(t, ShouldRetryByStatusCode(400))
require.False(t, ShouldRetryByStatusCode(200))
}

func TestShouldRetryByStatusCode_DefaultMatchesLegacyBehavior(t *testing.T) {
require.False(t, ShouldRetryByStatusCode(200))
require.False(t, ShouldRetryByStatusCode(400))
require.True(t, ShouldRetryByStatusCode(401))
require.False(t, ShouldRetryByStatusCode(408))
require.True(t, ShouldRetryByStatusCode(429))
require.True(t, ShouldRetryByStatusCode(500))
require.False(t, ShouldRetryByStatusCode(504))
require.False(t, ShouldRetryByStatusCode(524))
require.True(t, ShouldRetryByStatusCode(599))
}
Comment on lines +54 to +79

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Global mutation in tests can become flaky if package tests are (now or later) run in parallel.

Consider either:

  • documenting “do not use t.Parallel() in this package/file because of global config”, or
  • protecting AutomaticRetryStatusCodeRanges with a package-level mutex and using it in tests + production.
🤖 Prompt for AI Agents
In `@setting/operation_setting/status_code_ranges_test.go` around lines 54 - 79,
Tests mutate the global AutomaticRetryStatusCodeRanges which will race if tests
run in parallel; protect access by introducing a package-level sync.RWMutex
(e.g., automaticRetryMu) and update reads/writes: wrap reads in
ShouldRetryByStatusCode with a RLock/RUnlock and wrap assignments (including in
tests) with Lock/Unlock, or alternatively stop mutating the global by providing
a setter/getter that uses the mutex and update the tests to call the setter and
restore original via the getter; this ensures safe concurrent access to
AutomaticRetryStatusCodeRanges without relying on test ordering.

71 changes: 71 additions & 0 deletions web/src/components/settings/HttpStatusCodeRulesInput.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
Copyright (C) 2025 QuantumNous

This program is free software: you can redistribute it and/or modify
it under the terms of the GNU Affero General Public License as
published by the Free Software Foundation, either version 3 of the
License, or (at your option) any later version.

This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU Affero General Public License for more details.

You should have received a copy of the GNU Affero General Public License
along with this program. If not, see <https://www.gnu.org/licenses/>.

For commercial licensing, please contact support@quantumnous.com
*/

import React from 'react';
import { Form, Tag, Typography } from '@douyinfe/semi-ui';

export default function HttpStatusCodeRulesInput(props) {
const { Text } = Typography;
const {
label,
field,
placeholder,
extraText,
onChange,
parsed,
invalidText,
} = props;

return (
<>
<Form.Input
label={label}
placeholder={placeholder}
extraText={extraText}
field={field}
onChange={onChange}
/>
{parsed?.ok && parsed.tokens?.length > 0 && (
<div
style={{
display: 'flex',
flexWrap: 'wrap',
gap: 8,
marginTop: 8,
}}
>
{parsed.tokens.map((token) => (
<Tag key={token} size='small'>
{token}
</Tag>
))}
</div>
)}
{!parsed?.ok && (
<Text type='danger' style={{ display: 'block', marginTop: 8 }}>
{invalidText}
{parsed?.invalidTokens && parsed.invalidTokens.length > 0
? `: ${parsed.invalidTokens.join(', ')}`
: ''}
</Text>
)}
Comment on lines +60 to +67

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Guard against undefined parsed prop to avoid false error display.

When parsed is undefined or null (e.g., before initial parsing), !parsed?.ok evaluates to true, which will display the error text even though no actual validation error occurred.

🐛 Proposed fix
-      {!parsed?.ok && (
+      {parsed && !parsed.ok && (
         <Text type='danger' style={{ display: 'block', marginTop: 8 }}>
           {invalidText}
           {parsed?.invalidTokens && parsed.invalidTokens.length > 0
             ? `: ${parsed.invalidTokens.join(', ')}`
             : ''}
         </Text>
       )}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{!parsed?.ok && (
<Text type='danger' style={{ display: 'block', marginTop: 8 }}>
{invalidText}
{parsed?.invalidTokens && parsed.invalidTokens.length > 0
? `: ${parsed.invalidTokens.join(', ')}`
: ''}
</Text>
)}
{parsed && !parsed.ok && (
<Text type='danger' style={{ display: 'block', marginTop: 8 }}>
{invalidText}
{parsed?.invalidTokens && parsed.invalidTokens.length > 0
? `: ${parsed.invalidTokens.join(', ')}`
: ''}
</Text>
)}
🤖 Prompt for AI Agents
In `@web/src/components/settings/HttpStatusCodeRulesInput.jsx` around lines 60 -
67, The current conditional {!parsed?.ok && (...)} shows errors when parsed is
null/undefined; update the guard to only render the error block when parsed is
present and indicates a real validation failure (for example: parsed != null &&
parsed.ok === false) and ensure you access parsed.invalidTokens safely (e.g.,
parsed.invalidTokens?.length) so invalidText/invalidTokens are only shown when
parsed exists and has invalid tokens; change the conditional around the Text
component (and any uses of parsed.invalidTokens) accordingly.

</>
);
}

1 change: 1 addition & 0 deletions web/src/components/settings/OperationSetting.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ const OperationSetting = () => {
AutomaticEnableChannelEnabled: false,
AutomaticDisableKeywords: '',
AutomaticDisableStatusCodes: '401',
AutomaticRetryStatusCodes: '100-199,300-399,401-407,409-499,500-503,505-523,525-599',
'monitor_setting.auto_test_channel_enabled': false,
'monitor_setting.auto_test_channel_minutes': 10 /* 签到设置 */,
'checkin_setting.enabled': false,
Expand Down
2 changes: 2 additions & 0 deletions web/src/i18n/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -1925,6 +1925,8 @@
"自动禁用关键词": "Automatic disable keywords",
"自动禁用状态码": "Auto-disable status codes",
"自动禁用状态码格式不正确": "Invalid auto-disable status code format",
"自动重试状态码": "Auto-retry status codes",
"自动重试状态码格式不正确": "Invalid auto-retry status code format",
"支持填写单个状态码或范围(含首尾),使用逗号分隔": "Supports single status codes or inclusive ranges; separate with commas",
"例如:401, 403, 429, 500-599": "e.g. 401,403,429,500-599",
"自动选择": "Auto Select",
Expand Down
2 changes: 2 additions & 0 deletions web/src/i18n/locales/zh.json
Original file line number Diff line number Diff line change
Expand Up @@ -1911,6 +1911,8 @@
"自动禁用关键词": "自动禁用关键词",
"自动禁用状态码": "自动禁用状态码",
"自动禁用状态码格式不正确": "自动禁用状态码格式不正确",
"自动重试状态码": "自动重试状态码",
"自动重试状态码格式不正确": "自动重试状态码格式不正确",
"支持填写单个状态码或范围(含首尾),使用逗号分隔": "支持填写单个状态码或范围(含首尾),使用逗号分隔",
"例如:401, 403, 429, 500-599": "例如:401,403,429,500-599",
"自动选择": "自动选择",
Expand Down
Loading