修复了一些连接上的BUG - #5181
Closed
sdfsdfw2 wants to merge 8 commits into
Closed
Conversation
Contributor
WalkthroughThis PR adds automatic channel deletion triggered by upstream error keywords. Channels can now be configured to delete when request failures match configured deletion keywords, with deletion taking priority over the existing disable-on-error behavior. The frontend build stages are also removed from the Docker image. ChangesAutomatic Channel Deletion Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
setting/operation_setting/operation_setting.go (2)
34-50: 💤 Low valueOptional: deduplicate the keyword serialize/parse helpers.
These three additions are an exact copy of the
AutomaticDisableKeywordsvariants. A small shared helper that operates on a*[]stringwould remove the copy-paste and keep both in sync.♻️ Possible shared helper
func keywordsToString(kw []string) string { return strings.Join(kw, "\n") } func keywordsFromString(s string) []string { out := []string{} for _, k := range strings.Split(s, "\n") { k = strings.ToLower(strings.TrimSpace(k)) if k != "" { out = append(out, k) } } return out }Then
AutomaticDeleteKeywordsFromStringbecomesAutomaticDeleteKeywords = keywordsFromString(s).🤖 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 `@setting/operation_setting/operation_setting.go` around lines 34 - 50, The AutomaticDeleteKeywords serialization/parsing (AutomaticDeleteKeywordsToString, AutomaticDeleteKeywordsFromString) duplicate logic already present for AutomaticDisableKeywords; create a small shared helper pair (e.g., keywordsToString(kw []string) string and keywordsFromString(s string) []string or a helper that accepts *[]string) and replace AutomaticDeleteKeywordsToString to call keywordsToString(AutomaticDeleteKeywords) and AutomaticDeleteKeywordsFromString to assign AutomaticDeleteKeywords = keywordsFromString(s) so both keyword sets reuse the same normalized join/split/trimming/lowercasing logic.
40-50: ⚡ Quick winDelete-keyword matching is case-insensitive already.
AutomaticDeleteKeywordsFromStringlowercases stored keywords, andservice/channel.go’sShouldDeleteChannellowercaseserr.Error()(strings.ToLower(err.Error())) before matching againstoperation_setting.AutomaticDeleteKeywords, so uppercase in the upstream error won’t prevent matches.
Optional:AutomaticDisableKeywordsFromStringandAutomaticDeleteKeywordsFromStringare identical—could be factored to avoid copy-paste.🤖 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 `@setting/operation_setting/operation_setting.go` around lines 40 - 50, The current behavior already lowercases keywords so matching is case-insensitive; to remove duplication, extract the shared logic into a helper like parseKeywordsFromString(s string) []string that trims, lowercases and filters empty lines, then update AutomaticDeleteKeywordsFromString and AutomaticDisableKeywordsFromString to call this helper and assign the result to AutomaticDeleteKeywords/AutomaticDisableKeywords respectively; ensure the helper is referenced from those functions (AutomaticDeleteKeywordsFromString, AutomaticDisableKeywordsFromString) so the duplicate loop is removed.web/default/src/features/system-settings/integrations/monitoring-settings-section.tsx (1)
417-440: 💤 Low valueConsider reusing
SettingsSwitchItemfor visual consistency.The sibling switches (
AutomaticDisableChannelEnabled,AutomaticEnableChannelEnabled, scheduled tests) useSettingsSwitchItem/SettingsSwitchContent, while this new switch hand-rolls aFormItemwith manual flex/border classes. Aligning with the shared component keeps styling consistent and reduces drift.🤖 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/system-settings/integrations/monitoring-settings-section.tsx` around lines 417 - 440, Replace the hand-rolled FormField/FormItem for the AutomaticDeleteChannelEnabled switch with the shared SettingsSwitchItem/SettingsSwitchContent components to match sibling switches; locate the FormField with name 'AutomaticDeleteChannelEnabled' and swap its render to use SettingsSwitchItem (pass the label "Delete on keyword match") and SettingsSwitchContent (pass the description "Automatically delete channels when failure keywords match"), wiring the form control/value by connecting the Switch's checked to field.value and onCheckedChange to field.onChange so behavior stays the same and visual styling is consistent with AutomaticDisableChannelEnabled/AutomaticEnableChannelEnabled.service/channel.go (1)
99-102: 💤 Low valueLog message refers to "auto-disable" but should mention "auto-delete" or "auto-ban".
The log message on line 100 says
未启用自动禁用功能，跳过删除操作("auto-disable function not enabled, skipping delete operation"). The first part mentions "auto-disable" (自动禁用) but the operation being skipped is deletion. For consistency, consider updating the message to refer to "automatic channel management" (自动管理功能) or "auto-ban" (自动禁用) more generically, sinceAutoBangates both disabling and deletion.Suggested clarification
- common.SysLog(fmt.Sprintf("通道「%s」（#%d）未启用自动禁用功能，跳过删除操作", channelError.ChannelName, channelError.ChannelId)) + common.SysLog(fmt.Sprintf("通道「%s」（#%d）未启用自动管理功能，跳过删除操作", channelError.ChannelName, channelError.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 `@service/channel.go` around lines 99 - 102, Update the log message in the common.SysLog call that references channelError.AutoBan to correctly reflect the guarded behavior: change the current message that says "未启用自动禁用功能，跳过删除操作" to something like "未启用自动管理功能（自动禁用/删除），跳过删除操作" or explicitly "未启用自动禁用/删除功能，跳过删除操作" so it consistently refers to both ban and delete; edit the string passed to common.SysLog in the block that checks channelError.AutoBan (using channelError.ChannelName and channelError.ChannelId) to the new wording.
🤖 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 `@model/channel.go`:
- Around line 875-881: Rename the misleading function DeleteChannelByStatusAndId
to DeleteChannelById and update its signature/usage accordingly: keep the
implementation that loads a Channel by ID and calls channel.Delete(), and remove
any expectation of a status parameter (the status-based logic remains in
DeleteChannelByStatus); then find and update all call sites that reference
DeleteChannelByStatusAndId (e.g., in the channel service layer) to call
DeleteChannelById instead so names accurately reflect behavior.
---
Nitpick comments:
In `@service/channel.go`:
- Around line 99-102: Update the log message in the common.SysLog call that
references channelError.AutoBan to correctly reflect the guarded behavior:
change the current message that says "未启用自动禁用功能，跳过删除操作" to something like
"未启用自动管理功能（自动禁用/删除），跳过删除操作" or explicitly "未启用自动禁用/删除功能，跳过删除操作" so it
consistently refers to both ban and delete; edit the string passed to
common.SysLog in the block that checks channelError.AutoBan (using
channelError.ChannelName and channelError.ChannelId) to the new wording.
In `@setting/operation_setting/operation_setting.go`:
- Around line 34-50: The AutomaticDeleteKeywords serialization/parsing
(AutomaticDeleteKeywordsToString, AutomaticDeleteKeywordsFromString) duplicate
logic already present for AutomaticDisableKeywords; create a small shared helper
pair (e.g., keywordsToString(kw []string) string and keywordsFromString(s
string) []string or a helper that accepts *[]string) and replace
AutomaticDeleteKeywordsToString to call
keywordsToString(AutomaticDeleteKeywords) and AutomaticDeleteKeywordsFromString
to assign AutomaticDeleteKeywords = keywordsFromString(s) so both keyword sets
reuse the same normalized join/split/trimming/lowercasing logic.
- Around line 40-50: The current behavior already lowercases keywords so
matching is case-insensitive; to remove duplication, extract the shared logic
into a helper like parseKeywordsFromString(s string) []string that trims,
lowercases and filters empty lines, then update
AutomaticDeleteKeywordsFromString and AutomaticDisableKeywordsFromString to call
this helper and assign the result to
AutomaticDeleteKeywords/AutomaticDisableKeywords respectively; ensure the helper
is referenced from those functions (AutomaticDeleteKeywordsFromString,
AutomaticDisableKeywordsFromString) so the duplicate loop is removed.
In
`@web/default/src/features/system-settings/integrations/monitoring-settings-section.tsx`:
- Around line 417-440: Replace the hand-rolled FormField/FormItem for the
AutomaticDeleteChannelEnabled switch with the shared
SettingsSwitchItem/SettingsSwitchContent components to match sibling switches;
locate the FormField with name 'AutomaticDeleteChannelEnabled' and swap its
render to use SettingsSwitchItem (pass the label "Delete on keyword match") and
SettingsSwitchContent (pass the description "Automatically delete channels when
failure keywords match"), wiring the form control/value by connecting the
Switch's checked to field.value and onCheckedChange to field.onChange so
behavior stays the same and visual styling is consistent with
AutomaticDisableChannelEnabled/AutomaticEnableChannelEnabled.
🪄 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: 30176b72-f5f7-4cbe-a493-71b0bfa09943
⛔ Files ignored due to path filters (1)
new-api.taris excluded by!**/*.tar
📒 Files selected for processing (31)
.dockerignoreDockerfilecommon/constants.gocommon/init.gocontroller/channel-test.gocontroller/relay.gomodel/channel.gomodel/option.goservice/channel.goservice/http_client.gosetting/operation_setting/operation_setting.goweb/classic/src/components/settings/OperationSetting.jsxweb/classic/src/i18n/locales/en.jsonweb/classic/src/i18n/locales/fr.jsonweb/classic/src/i18n/locales/ja.jsonweb/classic/src/i18n/locales/ru.jsonweb/classic/src/i18n/locales/vi.jsonweb/classic/src/i18n/locales/zh-CN.jsonweb/classic/src/i18n/locales/zh-TW.jsonweb/classic/src/i18n/locales/zh.jsonweb/classic/src/pages/Setting/Operation/SettingsMonitoring.jsxweb/default/src/features/system-settings/integrations/monitoring-settings-section.tsxweb/default/src/features/system-settings/operations/index.tsxweb/default/src/features/system-settings/operations/section-registry.tsxweb/default/src/features/system-settings/types.tsweb/default/src/i18n/locales/en.jsonweb/default/src/i18n/locales/fr.jsonweb/default/src/i18n/locales/ja.jsonweb/default/src/i18n/locales/ru.jsonweb/default/src/i18n/locales/vi.jsonweb/default/src/i18n/locales/zh.json
💤 Files with no reviewable changes (2)
- .dockerignore
- Dockerfile
Comment on lines
+875
to
+881
| func DeleteChannelByStatusAndId(channelId int) error { | ||
| var channel Channel | ||
| if err := DB.First(&channel, channelId).Error; err != nil { | ||
| return err | ||
| } | ||
| return channel.Delete() | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Misleading function name: no status parameter despite "ByStatusAndId" naming.
The function is named DeleteChannelByStatusAndId but only accepts channelId as a parameter—there is no status parameter or status-based filtering. This is inconsistent with the existing DeleteChannelByStatus(status int64) on line 870, which does filter by status. The current implementation simply loads a channel by ID and deletes it.
Consider renaming to DeleteChannelById to accurately reflect its behavior and avoid confusion.
Proposed rename
-func DeleteChannelByStatusAndId(channelId int) error {
+func DeleteChannelById(channelId int) error {
var channel Channel
if err := DB.First(&channel, channelId).Error; err != nil {
return err
}
return channel.Delete()
}Also update the call site in service/channel.go line 104:
- err := model.DeleteChannelByStatusAndId(channelError.ChannelId)
+ err := model.DeleteChannelById(channelError.ChannelId)📝 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 DeleteChannelByStatusAndId(channelId int) error { | |
| var channel Channel | |
| if err := DB.First(&channel, channelId).Error; err != nil { | |
| return err | |
| } | |
| return channel.Delete() | |
| } | |
| func DeleteChannelById(channelId int) error { | |
| var channel Channel | |
| if err := DB.First(&channel, channelId).Error; err != nil { | |
| return err | |
| } | |
| return channel.Delete() | |
| } |
🤖 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.go` around lines 875 - 881, Rename the misleading function
DeleteChannelByStatusAndId to DeleteChannelById and update its signature/usage
accordingly: keep the implementation that loads a Channel by ID and calls
channel.Delete(), and remove any expectation of a status parameter (the
status-based logic remains in DeleteChannelByStatus); then find and update all
call sites that reference DeleteChannelByStatusAndId (e.g., in the channel
service layer) to call DeleteChannelById instead so names accurately reflect
behavior.
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.
Important
📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)
Summary by CodeRabbit
Release Notes
New Features
Infrastructure