Copilot commented on code in PR #752:
URL: https://github.com/apache/dubbo-go-pixiu/pull/752#discussion_r2329175359
##########
admin/logic/logic.go:
##########
@@ -570,8 +570,7 @@ func getMethodId() int {
}
func loopGetId(k string) int {
-
- for true {
+ for {
Review Comment:
The infinite loop has been simplified from `for true {` to `for {`, but
there's no reachable return statement after the loop. The unreachable `return
ErrID` at line 618 should be removed as it will never execute.
##########
pkg/filter/network/grpcproxy/filter/proxy/grpc_proxy_filter.go:
##########
@@ -396,24 +396,21 @@ func (f *Filter) monitorConnection(cacheKey string, conn
*grpc.ClientConn) {
ticker := time.NewTicker(defaultHealthCheckInterval)
defer ticker.Stop()
- for {
- select {
- case <-ticker.C:
- state := conn.GetState()
- if state == connectivity.Shutdown || state ==
connectivity.TransientFailure {
- logger.Warnf("Connection to %s is in bad state
(%s), removing from pool",
- cacheKey, state.String())
-
- f.mu.Lock()
- if currentConn, ok :=
f.clientConnPool.Load(cacheKey); ok {
- if currentConn == conn {
-
f.clientConnPool.Delete(cacheKey)
- }
+ for range ticker.C {
+ state := conn.GetState()
+ if state == connectivity.Shutdown || state ==
connectivity.TransientFailure {
+ logger.Warnf("Connection to %s is in bad state (%s),
removing from pool",
+ cacheKey, state.String())
+
+ f.mu.Lock()
+ if currentConn, ok := f.clientConnPool.Load(cacheKey);
ok {
+ if currentConn == conn {
+ f.clientConnPool.Delete(cacheKey)
}
- f.mu.Unlock()
-
- return
}
+ f.mu.Unlock()
+
+ return
}
}
Review Comment:
The refactored loop using `for range ticker.C` removes the select statement
but may not handle ticker cleanup properly. Consider keeping the original
select structure or ensure proper ticker cleanup when the function exits.
##########
pkg/filter/mcp/mcpserver/handlers.go:
##########
@@ -428,36 +428,35 @@ func (f *MCPServerFilter) buildBackendRequest(ctx
*MCPContext, toolConfig ToolCo
queryParams := make(map[string]string)
// Process arguments based on their location (path, query, body)
- if arguments != nil {
- for argName, argValue := range arguments {
- // Find argument configuration
- var argConfig *ArgConfig
- for _, arg := range toolConfig.Args {
- if arg.Name == argName {
- argConfig = &arg
- break
- }
- }
- if argConfig == nil {
- continue // Skip unknown arguments
+ for argName, argValue := range arguments {
+ // Find argument configuration
+ var argConfig *ArgConfig
+ for _, arg := range toolConfig.Args {
+ if arg.Name == argName {
+ argConfig = &arg
+ break
}
+ }
- switch argConfig.In {
- case inPath:
- // Replace path parameters
- placeholder := fmt.Sprintf("{%s}", argName)
- replacement := fmt.Sprintf("%v", argValue)
- path = strings.ReplaceAll(path, placeholder,
replacement)
+ if argConfig == nil {
+ continue // Skip unknown arguments
+ }
- case inQuery:
- // Add to query parameters
- queryParams[argName] = fmt.Sprintf("%v",
argValue)
+ switch argConfig.In {
+ case inPath:
+ // Replace path parameters
+ placeholder := fmt.Sprintf("{%s}", argName)
+ replacement := fmt.Sprintf("%v", argValue)
+ path = strings.ReplaceAll(path, placeholder,
replacement)
- case inBody:
- // Add to request body
- bodyParams[argName] = argValue
- }
+ case inQuery:
+ // Add to query parameters
+ queryParams[argName] = fmt.Sprintf("%v", argValue)
+
+ case inBody:
+ // Add to request body
+ bodyParams[argName] = argValue
}
}
Review Comment:
The nil check for `arguments` has been removed, which could cause a panic if
`arguments` is nil. The original code checked `if arguments != nil` before
ranging over it. This should be restored to prevent potential runtime panics.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
