Skip to content

feat: add channel-level rate limiting - #5067

Draft
zhibisora wants to merge 2 commits into
QuantumNous:mainfrom
zhibisora:upstream-main-channel-rate-limit
Draft

feat: add channel-level rate limiting#5067
zhibisora wants to merge 2 commits into
QuantumNous:mainfrom
zhibisora:upstream-main-channel-rate-limit

Conversation

@zhibisora

@zhibisora zhibisora commented May 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add per-user channel rate limiting backed by Redis or in-memory token buckets
  • support channel-wide and per-key limits for multi-key channels, with retry exclusion when a key or channel is limited
  • add admin UI controls in both default and classic channel editors

Testing

  • docker run --rm -v "/Users/sora/conductor/workspaces/new-api/hamburg-v2":/src -w /src golang:1.25.1 go test ./controller ./middleware ./model ./relay ./service -run 'Test(GetChannelReselectsWhenContextChannelExcluded|AllowChannelRateLimitMemoryAllowsConfiguredBurst|AllowChannelRateLimitMemoryCleansExpiredBuckets|CheckSelectedChannelRateLimitIsUserScoped|GetNextEnabledKeyExcludingSkipsLimitedKeys|GetNextEnabledKeyExcludingReturnsNoAvailableKey|GetChannelWithExclusionsNoPriorityReturnsNil|ValidateOtherSettingsRejectsFractionalChannelRateLimit|ValidateOtherSettingsAcceptsIntegerChannelRateLimit|ResolveOriginTaskSetsLockedMultiKeyContext|ResetStatusCode|ShouldDisableChannelIgnoresLocalRateLimit)'
  • docker run --rm -v "/Users/sora/conductor/workspaces/new-api/hamburg-v2":/src -w /src -v newapi-gomod:/go/pkg/mod -v newapi-gobuild:/root/.cache/go-build golang:1.25.1 go test ./common/... ./constant/... ./controller/... ./dto/... ./i18n/... ./logger/... ./middleware/... ./model/... ./oauth/... ./pkg/... ./relay/... ./router/... ./service/... ./setting/... ./types/... -run '^$'
  • docker run --rm -v "/Users/sora/conductor/workspaces/new-api/hamburg-v2":/src:ro oven/bun:1 sh -lc 'cp -a /src/web/default /tmp/default && cd /tmp/default && bun install --frozen-lockfile && bun run build:check'
  • docker run --rm -v "/Users/sora/conductor/workspaces/new-api/hamburg-v2":/src:ro oven/bun:1 sh -lc 'cp -a /src/web/classic /tmp/classic && cd /tmp/classic && bun install --frozen-lockfile && bun run build'

Summary by CodeRabbit

Release Notes

  • New Features
    • Added channel-level request rate limiting with configurable request caps per time period (token-bucket).
    • Added rate-limit scope selection: apply limits per channel or per channel key.
    • Added a “Channel Rate Limit” configuration section to the channel UI, with updated multi-language help text.
  • Improvements
    • Channel selection now retries with eligible non-rate-limited channels and updates exclusions during retries.
    • Added dedicated channel rate-limited error codes and clearer retry behavior (including for Midjourney routes).

@coderabbitai

coderabbitai Bot commented May 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

This PR adds configurable per-channel request rate limiting with Redis or in-memory token buckets, supports channel and key scopes, excludes limited channels or keys during retries, integrates checks into relay and Midjourney flows, and adds configuration UI, localization, documentation, and tests.

Changes

Channel Rate Limiting Feature

Layer / File(s) Summary
Rate-limit contracts and Redis TTL
types/error.go, dto/channel_settings.go, common/limiter/...
Adds rate-limit settings, error identification, Redis TTL configuration, and conditional Lua key expiration.
Channel validation and exclusion-aware selection
model/..., service/channel_select.go
Validates settings, skips excluded keys and channels, traverses priorities, and records retry exclusions.
Selected-channel context and middleware
middleware/...
Checks channel or key-scoped limits through Redis or in-memory token buckets and updates exclusions when denied.
Relay and Midjourney integration
controller/relay.go, relay/..., service/channel.go
Adds selected-channel checks, retry decisions, locked-channel handling, Midjourney response mapping, and auto-disable protection.
Validation and integration tests
model/*test.go, middleware/*test.go, controller/*test.go, relay/*test.go, service/error_test.go
Covers bucket behavior, validation, user scoping, channel reselection, task context, and local versus upstream 429 responses.
Default channel form
web/default/src/features/channels/...
Adds schema fields, parsing, serialization, defaults, and advanced settings controls.
Classic UI, localization, and documentation
web/classic/src/..., docs/channel/other_setting.md
Adds classic settings controls, translations, and configuration documentation for channel rate limiting.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

Suggested reviewers: calcium-ion

Poem

🐰 I hop through buckets, neat and bright,
Keys step aside when limits bite.
Redis ticks and retries flow,
Channels find new paths to go.
Safe settings bloom in every UI.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 9.86% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding channel-level rate limiting.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch upstream-main-channel-rate-limit

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.

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 2

🧹 Nitpick comments (2)
model/channel_key_test.go (1)

53-53: ⚡ Quick win

Use a GORM delete instead of raw SQL.

Line 53 uses direct SQL even though this can be expressed with GORM and kept fully DB-agnostic in tests.

Proposed change
-	require.NoError(t, DB.Exec("DELETE FROM abilities").Error)
+	require.NoError(t, DB.Where("1 = 1").Delete(&Ability{}).Error)

As per coding guidelines, "Prefer GORM methods (Create, Find, Where, Updates, etc.) over raw SQL queries."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@model/channel_key_test.go` at line 53, Replace the raw SQL deletion with a
GORM delete call: instead of DB.Exec("DELETE FROM abilities").Error, call GORM's
Delete on the Ability model via the shared DB (e.g.,
DB.Session(&gorm.Session{AllowGlobalUpdate: true}).Delete(&Ability{}) or
DB.Where(...).Delete(&Ability{})) so the test remains DB-agnostic; update the
test to use the DB variable and the Ability struct name accordingly.
middleware/channel-rate-limit.go (1)

159-162: 💤 Low value

Use fmt.Errorf instead of errors.New(fmt.Sprintf(...)).

This is the idiomatic Go pattern for creating formatted errors.

♻️ Suggested fix
 func newChannelRateLimitError(channelID int, err error, statusCode int) *types.NewAPIError {
 	if err == nil {
-		err = errors.New(fmt.Sprintf("channel #%d rate limit reached", channelID))
+		err = fmt.Errorf("channel #%d rate limit reached", channelID)
 	}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@middleware/channel-rate-limit.go` around lines 159 - 162, In
newChannelRateLimitError, replace the non-idiomatic errors.New(fmt.Sprintf(...))
with fmt.Errorf to create the formatted error (e.g., fmt.Errorf("channel #%d
rate limit reached", channelID)); ensure the fmt package is imported if not
already and keep the same behavior of assigning the constructed error to err
when err == nil.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@web/classic/src/components/table/channels/modals/EditChannelModal.jsx`:
- Around line 1870-1881: When persisting rate-limit settings in
EditChannelModal.jsx, enforce minimums when
localInputs.channel_rate_limit_enabled is true: ensure
settings.channel_rate_limit_count = Math.max(1,
integerOrDefault(localInputs.channel_rate_limit_count, 0)) and
settings.channel_rate_limit_period_seconds = Math.max(1,
integerOrDefault(localInputs.channel_rate_limit_period_seconds, 60)));
alternatively perform validation on submit and block/save with an error if
localInputs.channel_rate_limit_enabled && (count < 1 || period_seconds < 1).
Update the assignments that set settings.channel_rate_limit_count and
settings.channel_rate_limit_period_seconds (and any related UI validation for
channel_rate_limit_count / channel_rate_limit_period_seconds) to enforce/count
>=1 and period_seconds >=1 when channel_rate_limit_enabled is true.

In `@web/default/src/features/channels/lib/channel-form.ts`:
- Around line 239-248: transformChannelToFormDefaults currently assigns
channelRateLimitCount and channelRateLimitPeriodSeconds from parsed values but
doesn't enforce the schema lower bounds, which can create an invalid form state;
update the assignment for channelRateLimitCount, channelRateLimitPeriodSeconds
(and channelRateLimitScope if relevant) to clamp values so channelRateLimitCount
>= 0 and channelRateLimitPeriodSeconds >= 1 (and if your schema has upper
bounds, also clamp to those maxima) before returning the defaults.

---

Nitpick comments:
In `@middleware/channel-rate-limit.go`:
- Around line 159-162: In newChannelRateLimitError, replace the non-idiomatic
errors.New(fmt.Sprintf(...)) with fmt.Errorf to create the formatted error
(e.g., fmt.Errorf("channel #%d rate limit reached", channelID)); ensure the fmt
package is imported if not already and keep the same behavior of assigning the
constructed error to err when err == nil.

In `@model/channel_key_test.go`:
- Line 53: Replace the raw SQL deletion with a GORM delete call: instead of
DB.Exec("DELETE FROM abilities").Error, call GORM's Delete on the Ability model
via the shared DB (e.g., DB.Session(&gorm.Session{AllowGlobalUpdate:
true}).Delete(&Ability{}) or DB.Where(...).Delete(&Ability{})) so the test
remains DB-agnostic; update the test to use the DB variable and the Ability
struct name accordingly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: afbe5495-49f1-4aa8-a84d-ed263686d904

📥 Commits

Reviewing files that changed from the base of the PR and between ebbe315 and b9c7c22.

📒 Files selected for processing (31)
  • common/limiter/limiter.go
  • common/limiter/lua/rate_limit.lua
  • controller/channel.go
  • controller/channel_rate_limit_retry_test.go
  • controller/relay.go
  • dto/channel_settings.go
  • middleware/channel-rate-limit.go
  • middleware/channel_rate_limit_test.go
  • middleware/distributor.go
  • model/ability.go
  • model/channel.go
  • model/channel_cache.go
  • model/channel_key_test.go
  • relay/mjproxy_handler.go
  • relay/relay_task.go
  • relay/relay_task_test.go
  • service/channel.go
  • service/channel_select.go
  • service/error_test.go
  • types/error.go
  • web/classic/src/components/table/channels/modals/EditChannelModal.jsx
  • web/classic/src/i18n/locales/en.json
  • web/classic/src/i18n/locales/fr.json
  • web/classic/src/i18n/locales/ja.json
  • web/classic/src/i18n/locales/ru.json
  • web/classic/src/i18n/locales/vi.json
  • web/classic/src/i18n/locales/zh-CN.json
  • web/classic/src/i18n/locales/zh-TW.json
  • web/default/src/features/channels/components/drawers/channel-mutate-drawer.tsx
  • web/default/src/features/channels/lib/channel-form.ts
  • web/default/src/features/channels/types.ts

Comment on lines +1870 to +1881
settings.channel_rate_limit_enabled =
localInputs.channel_rate_limit_enabled === true;
settings.channel_rate_limit_count = integerOrDefault(
localInputs.channel_rate_limit_count,
0,
);
settings.channel_rate_limit_period_seconds = integerOrDefault(
localInputs.channel_rate_limit_period_seconds,
60,
);
settings.channel_rate_limit_scope =
localInputs.channel_rate_limit_scope === 'key' ? 'key' : 'channel';

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 | ⚡ Quick win

Validate enabled rate-limit values before persisting.

With the current defaults, enabling rate limit can still submit channel_rate_limit_count = 0 (Line 2569 uses min={0}), which can effectively hard-throttle the channel immediately. Please enforce count >= 1 and period_seconds >= 1 when channel_rate_limit_enabled is true.

Suggested fix
@@
-    settings.channel_rate_limit_enabled =
-      localInputs.channel_rate_limit_enabled === true;
-    settings.channel_rate_limit_count = integerOrDefault(
-      localInputs.channel_rate_limit_count,
-      0,
-    );
-    settings.channel_rate_limit_period_seconds = integerOrDefault(
-      localInputs.channel_rate_limit_period_seconds,
-      60,
-    );
+    settings.channel_rate_limit_enabled =
+      localInputs.channel_rate_limit_enabled === true;
+    const channelRateLimitCount = integerOrDefault(
+      localInputs.channel_rate_limit_count,
+      0,
+    );
+    const channelRateLimitPeriodSeconds = integerOrDefault(
+      localInputs.channel_rate_limit_period_seconds,
+      60,
+    );
+    if (settings.channel_rate_limit_enabled && channelRateLimitCount < 1) {
+      showInfo(t('启用渠道限流时,每周期请求数必须大于 0'));
+      return;
+    }
+    if (
+      settings.channel_rate_limit_enabled &&
+      channelRateLimitPeriodSeconds < 1
+    ) {
+      showInfo(t('启用渠道限流时,周期秒数必须大于 0'));
+      return;
+    }
+    settings.channel_rate_limit_count = channelRateLimitCount;
+    settings.channel_rate_limit_period_seconds =
+      channelRateLimitPeriodSeconds;
@@
-                        min={0}
+                        min={1}

Also applies to: 2566-2595

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/classic/src/components/table/channels/modals/EditChannelModal.jsx` around
lines 1870 - 1881, When persisting rate-limit settings in EditChannelModal.jsx,
enforce minimums when localInputs.channel_rate_limit_enabled is true: ensure
settings.channel_rate_limit_count = Math.max(1,
integerOrDefault(localInputs.channel_rate_limit_count, 0)) and
settings.channel_rate_limit_period_seconds = Math.max(1,
integerOrDefault(localInputs.channel_rate_limit_period_seconds, 60)));
alternatively perform validation on submit and block/save with an error if
localInputs.channel_rate_limit_enabled && (count < 1 || period_seconds < 1).
Update the assignments that set settings.channel_rate_limit_count and
settings.channel_rate_limit_period_seconds (and any related UI validation for
channel_rate_limit_count / channel_rate_limit_period_seconds) to enforce/count
>=1 and period_seconds >=1 when channel_rate_limit_enabled is true.

Comment on lines +239 to +248
channelRateLimitCount = integerOrDefault(
parsed.channel_rate_limit_count,
0
)
channelRateLimitPeriodSeconds = integerOrDefault(
parsed.channel_rate_limit_period_seconds,
60
)
channelRateLimitScope =
parsed.channel_rate_limit_scope === 'key' ? 'key' : 'channel'

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 | ⚡ Quick win

Clamp parsed rate-limit values to schema bounds.

transformChannelToFormDefaults truncates persisted values but does not enforce count >= 0 and period_seconds >= 1. Bad existing settings can initialize an invalid form state and block submission.

💡 Proposed fix
-      channelRateLimitCount = integerOrDefault(
-        parsed.channel_rate_limit_count,
-        0
-      )
-      channelRateLimitPeriodSeconds = integerOrDefault(
-        parsed.channel_rate_limit_period_seconds,
-        60
-      )
+      channelRateLimitCount = Math.max(
+        0,
+        integerOrDefault(parsed.channel_rate_limit_count, 0)
+      )
+      channelRateLimitPeriodSeconds = Math.max(
+        1,
+        integerOrDefault(parsed.channel_rate_limit_period_seconds, 60)
+      )
📝 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
channelRateLimitCount = integerOrDefault(
parsed.channel_rate_limit_count,
0
)
channelRateLimitPeriodSeconds = integerOrDefault(
parsed.channel_rate_limit_period_seconds,
60
)
channelRateLimitScope =
parsed.channel_rate_limit_scope === 'key' ? 'key' : 'channel'
channelRateLimitCount = Math.max(
0,
integerOrDefault(parsed.channel_rate_limit_count, 0)
)
channelRateLimitPeriodSeconds = Math.max(
1,
integerOrDefault(parsed.channel_rate_limit_period_seconds, 60)
)
channelRateLimitScope =
parsed.channel_rate_limit_scope === 'key' ? 'key' : 'channel'
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/default/src/features/channels/lib/channel-form.ts` around lines 239 -
248, transformChannelToFormDefaults currently assigns channelRateLimitCount and
channelRateLimitPeriodSeconds from parsed values but doesn't enforce the schema
lower bounds, which can create an invalid form state; update the assignment for
channelRateLimitCount, channelRateLimitPeriodSeconds (and channelRateLimitScope
if relevant) to clamp values so channelRateLimitCount >= 0 and
channelRateLimitPeriodSeconds >= 1 (and if your schema has upper bounds, also
clamp to those maxima) before returning the defaults.

@zhibisora
zhibisora marked this pull request as draft May 23, 2026 08:47
(cherry picked from commit c8cbd5a69b1602aab81bcc7f08d8a90765a6bbb5)
@zhibisora
zhibisora force-pushed the upstream-main-channel-rate-limit branch from b9c7c22 to 0579512 Compare July 22, 2026 06:49

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
model/channel_cache.go (1)

308-341: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Blocking DB write possible while holding the global channelSyncLock.

channel.GetOtherSettings() can call channel.Save() (a DB write) when OtherSettings fails to unmarshal. In CacheUpdateChannel this call now happens while channelSyncLock.Lock() is held (lines 312-339), unlike InitChannelCache where the equivalent call happens before the lock is acquired. Since channelSyncLock also gates the hot GetRandomSatisfiedChannelWithExclusions read path used on every relay request, a slow/blocked DB write here would stall all concurrent channel selection. The channel == nil check is also performed after acquiring the lock, wasting a lock cycle on that path.

🔒 Proposed fix: resolve config before acquiring the lock
 func CacheUpdateChannel(channel *Channel) {
 	if !common.MemoryCacheEnabled {
 		return
 	}
-	channelSyncLock.Lock()
 	if channel == nil {
-		channelSyncLock.Unlock()
 		return
 	}
+	var newAdvancedConfig *dto.AdvancedCustomConfig
+	if channel.Type == constant.ChannelTypeAdvancedCustom {
+		newAdvancedConfig = channel.GetOtherSettings().AdvancedCustom
+	}
+
+	channelSyncLock.Lock()
 
 	if channelsIDM == nil {
 		channelsIDM = make(map[int]*Channel)
 	}
 	if oldChannel, ok := channelsIDM[channel.Id]; ok {
 		logger.LogDebug(nil, "CacheUpdateChannel before: id=%d, name=%s, status=%d, polling_index=%d", channel.Id, channel.Name, channel.Status, oldChannel.ChannelInfo.MultiKeyPollingIndex)
 	}
 	channelsIDM[channel.Id] = channel
 	if channel2advancedCustomConfig == nil {
 		channel2advancedCustomConfig = make(map[int]*dto.AdvancedCustomConfig)
 	}
 	delete(channel2advancedCustomConfig, channel.Id)
-	if channel.Type == constant.ChannelTypeAdvancedCustom {
-		if config := channel.GetOtherSettings().AdvancedCustom; config != nil {
-			channel2advancedCustomConfig[channel.Id] = config
-		}
+	if newAdvancedConfig != nil {
+		channel2advancedCustomConfig[channel.Id] = newAdvancedConfig
 	}
 	logger.LogDebug(nil, "CacheUpdateChannel after: id=%d, name=%s, status=%d, polling_index=%d", channel.Id, channel.Name, channel.Status, channel.ChannelInfo.MultiKeyPollingIndex)
 	channelSyncLock.Unlock()
 	InvalidatePricingCache()
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@model/channel_cache.go` around lines 308 - 341, In CacheUpdateChannel,
validate channel == nil before acquiring channelSyncLock, then resolve
channel.GetOtherSettings().AdvancedCustom before locking so any fallback Save
occurs outside the global lock. After acquiring the lock, retain the cache map
updates and use the precomputed advanced custom configuration when updating
channel2advancedCustomConfig.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@controller/channel.go`:
- Around line 467-471: Update the validation error returned by validateChannel
after channel.ValidateSettings() fails to remove the literal
“[setting/settings]” placeholder and use clear, user-facing Chinese wording
while preserving the underlying error details.

In `@docs/channel/other_setting.md`:
- Around line 20-21: Update the documentation entries for
channel_rate_limit_count and channel_rate_limit_period_seconds to state that,
when enabled, both values must be positive integers, not merely greater than
zero. Preserve the existing descriptions and clearly include the integer
requirement for each setting.

---

Outside diff comments:
In `@model/channel_cache.go`:
- Around line 308-341: In CacheUpdateChannel, validate channel == nil before
acquiring channelSyncLock, then resolve
channel.GetOtherSettings().AdvancedCustom before locking so any fallback Save
occurs outside the global lock. After acquiring the lock, retain the cache map
updates and use the precomputed advanced custom configuration when updating
channel2advancedCustomConfig.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 565a6fb6-67fc-4a7a-bffc-f8113ae6482f

📥 Commits

Reviewing files that changed from the base of the PR and between b9c7c22 and 0579512.

📒 Files selected for processing (21)
  • common/limiter/limiter.go
  • common/limiter/lua/rate_limit.lua
  • controller/channel.go
  • controller/channel_rate_limit_retry_test.go
  • controller/relay.go
  • controller/task_error_response_test.go
  • docs/channel/other_setting.md
  • dto/channel_settings.go
  • middleware/channel-rate-limit.go
  • middleware/channel_rate_limit_test.go
  • middleware/distributor.go
  • model/ability.go
  • model/channel.go
  • model/channel_cache.go
  • model/channel_key_test.go
  • relay/mjproxy_handler.go
  • relay/relay_task.go
  • relay/relay_task_test.go
  • service/channel.go
  • service/channel_select.go
  • service/error_test.go
🚧 Files skipped from review as they are similar to previous changes (12)
  • common/limiter/lua/rate_limit.lua
  • controller/channel_rate_limit_retry_test.go
  • service/error_test.go
  • common/limiter/limiter.go
  • middleware/distributor.go
  • relay/relay_task_test.go
  • service/channel_select.go
  • relay/relay_task.go
  • relay/mjproxy_handler.go
  • middleware/channel-rate-limit.go
  • dto/channel_settings.go
  • controller/relay.go

Comment thread controller/channel.go
Comment on lines 467 to 471
func validateChannel(channel *model.Channel, isAdd bool) error {
// 校验 channel settings
if err := channel.ValidateSettings(); err != nil {
return fmt.Errorf("渠道额外设置[channel setting] 格式错误:%s", err.Error())
return fmt.Errorf("渠道设置[setting/settings] 格式错误:%s", err.Error())
}

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Confusing placeholder text in validation error message.

"渠道设置[setting/settings] 格式错误:%s" mixes literal bracket notation into the user-facing Chinese message, likely a merge/placeholder artifact rather than intended copy.

✏️ Proposed fix
-		return fmt.Errorf("渠道设置[setting/settings] 格式错误:%s", err.Error())
+		return fmt.Errorf("渠道设置格式错误:%s", err.Error())
📝 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
func validateChannel(channel *model.Channel, isAdd bool) error {
// 校验 channel settings
if err := channel.ValidateSettings(); err != nil {
return fmt.Errorf("渠道额外设置[channel setting] 格式错误:%s", err.Error())
return fmt.Errorf("渠道设置[setting/settings] 格式错误:%s", err.Error())
}
func validateChannel(channel *model.Channel, isAdd bool) error {
// 校验 channel settings
if err := channel.ValidateSettings(); err != nil {
return fmt.Errorf("渠道设置格式错误:%s", err.Error())
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@controller/channel.go` around lines 467 - 471, Update the validation error
returned by validateChannel after channel.ValidateSettings() fails to remove the
literal “[setting/settings]” placeholder and use clear, user-facing Chinese
wording while preserving the underlying error details.

Comment on lines +20 to +21
- `channel_rate_limit_count`: 每周期允许的请求数;启用时必须大于 0
- `channel_rate_limit_period_seconds`: 限流周期(秒);启用时必须大于 0

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Document the integer constraint for rate-limit values.

The backend and default UI require both channel_rate_limit_count and channel_rate_limit_period_seconds to be positive integers, but the documentation only says they must be greater than zero. Clarify this to prevent users from configuring rejected fractional values.

Proposed wording
-   - `channel_rate_limit_count`: 每周期允许的请求数;启用时必须大于 0
-   - `channel_rate_limit_period_seconds`: 限流周期(秒);启用时必须大于 0
+   - `channel_rate_limit_count`: 每周期允许的请求数;启用时必须为大于 0 的整数
+   - `channel_rate_limit_period_seconds`: 限流周期(秒);启用时必须为大于 0 的整数
📝 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
- `channel_rate_limit_count`: 每周期允许的请求数;启用时必须大于 0
- `channel_rate_limit_period_seconds`: 限流周期(秒);启用时必须大于 0
- `channel_rate_limit_count`: 每周期允许的请求数;启用时必须为大于 0 的整数
- `channel_rate_limit_period_seconds`: 限流周期(秒);启用时必须为大于 0 的整数
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/channel/other_setting.md` around lines 20 - 21, Update the documentation
entries for channel_rate_limit_count and channel_rate_limit_period_seconds to
state that, when enabled, both values must be positive integers, not merely
greater than zero. Preserve the existing descriptions and clearly include the
integer requirement for each setting.

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.

1 participant