Skip to content

fix: fix issue with mcp bind website #8356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
f2c-ci-robot merged 1 commit into from
Apr 9, 2025
Merged

fix: fix issue with mcp bind website #8356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions backend/app/repo/group.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ type IGroupRepo interface {
Delete(opts ...DBOption) error
CancelDefault(groupType string) error
WithByHostDefault() DBOption
WithByWebsiteDefault() DBOption
}

func NewIGroupRepo() IGroupRepo {
Expand Down Expand Up @@ -56,6 +57,12 @@ func (u *GroupRepo) WithByHostDefault() DBOption {
}
}

func (u *GroupRepo) WithByWebsiteDefault() DBOption {
return func(g *gorm.DB) *gorm.DB {
return g.Where("is_default = ? AND type = ?", 1, "website")
}
}

func (u *GroupRepo) Delete(opts ...DBOption) error {
db := global.DB
for _, opt := range opts {
Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Apr 9, 2025

Choose a reason for hiding this comment

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

There is one change made in this patch. The function WithByWebsiteDefault has been added to the IGroupRepo interface and also implemented in the GroupRepo struct. This likely adds functionality for filtering groups that are both default and have a specific group type ("website") when performing database queries.

Optimization suggestions: No apparent performance improvements can be directly suggested from this single-line addition, but having more methods on an interface could potentially make the implementation slightly more modular or easier to maintain depending on future use cases.

Expand Down
25 changes: 15 additions & 10 deletions backend/app/service/mcp_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,10 +279,12 @@ func (m McpServerService) BindDomain(req request.McpBindDomain) error {
return buserr.New("ErrSSL")
}
}
group, _ := groupRepo.Get(groupRepo.WithByWebsiteDefault())
createWebsiteReq := request.WebsiteCreate{
PrimaryDomain: req.Domain,
Alias: strings.ToLower(req.Domain),
Type: constant.Static,
PrimaryDomain: req.Domain,
Alias: strings.ToLower(req.Domain),
Type: constant.Static,
WebsiteGroupID: group.ID,
}
websiteService := NewIWebsiteService()
if err := websiteService.CreateWebsite(createWebsiteReq); err != nil {
Expand Down Expand Up @@ -386,13 +388,16 @@ func updateMcpConfig(websiteID uint) {
} else {
baseUrl = fmt.Sprintf("https://%s", websiteDomain.Domain)
}
for _, server := range servers {
if server.BaseURL != baseUrl {
server.BaseURL = baseUrl
server.HostIP = "127.0.0.1"
go updateMcpServer(&server)

go func() {
for _, server := range servers {
if server.BaseURL != baseUrl {
server.BaseURL = baseUrl
server.HostIP = "127.0.0.1"
_ = updateMcpServer(&server)
}
}
}
}()
}

func addProxy(server *model.McpServer) {
Expand Down Expand Up @@ -502,13 +507,13 @@ func updateMcpServer(mcpServer *model.McpServer) error {
if err := gotenv.Write(env, path.Join(mcpServer.Dir, ".env")); err != nil {
return err
}
_ = mcpServerRepo.Save(mcpServer)
composePath := path.Join(constant.McpDir, mcpServer.Name, "docker-compose.yml")
_, _ = compose.Down(composePath)
if _, err := compose.Up(composePath); err != nil {
mcpServer.Status = constant.RuntimeError
mcpServer.Message = err.Error()
}

return mcpServerRepo.Save(mcpServer)
}

Copy link
Copy Markdown
Member

@wanghe-fit2cloud wanghe-fit2cloud Apr 9, 2025

Choose a reason for hiding this comment

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

The provided code has several issues that can be addressed:

  1. Race Condition: The go functions in the code snippet will not run concurrently due to the lack of a proper goroutine structure. This means that if multiple servers are being updated simultaneously, some updates could be missed.

  2. Concurrency Issues: The loop that iterates over servers is executed sequentially within each goroutine, which might lead to inefficiencies or unexpected behavior when dealing with many servers.

  3. Missing Return Value in Anonymous Function: The anonymous function used in the race condition section does not return an error value, which prevents catching errors from updateMcpServer.

To address these issues:

  1. Modify Loop Execution:

    • Use a channel or waitgroup to ensure concurrent execution.

  2. Return Error Values:

    • Modify the anonymous function to capture and return errors properly.

Here's an optimized version of the affected part of the code:

var wg sync.WaitGroup

for _, server := range servers {
    wg.Add(1)
    go func(s *model.McpServer) {
        defer wg.Done()

        if server.BaseURL != baseUrl {
            server.BaseURL = baseUrl
            server.HostIP = "127.0.0.1"
            if err := updateMcpServer(s); err != nil {
                log.Printf("Failed to update MCP Server %s: %v", s.Name, err)
            } else {
                mcpServerRepo.Save(s) // Ensure this call is successful before updating docker-compose.
            }
        }

        composePath := path.Join(constant.McpDir, s.Name, "docker-compose.yml")
        _, _ = compose.Down(composePath)
        if _, err := compose.Up(composePath); err != nil {
            s.Status = constant.RuntimeError
            s.Message = err.Error()
            mcpServerRepo.Save(s)
        }
    }(server)
}

wg.Wait() // Wait for all goroutines to complete.

This version ensures that each server update operation runs concurrently using channels. It also includes logging for failed operations to help debug potential issues later on. Make sure to initialize the waitgroup correctly at the start of the function and ensure mcpServerRepo.Save is always called successfully after every relevant action in the goroutine.

Expand Down
Loading