Copilot commented on code in PR #3013:
URL: https://github.com/apache/dubbo-go/pull/3013#discussion_r2328396251
##########
filter/active/filter.go:
##########
@@ -71,12 +71,23 @@ func (f *activeFilter) Invoke(ctx context.Context, invoker
base.Invoker, inv bas
// OnResponse update the active count base on the request result.
func (f *activeFilter) OnResponse(ctx context.Context, result result.Result,
invoker base.Invoker, inv base.Invocation) result.Result {
startTime, err :=
strconv.ParseInt(inv.(*invocation.RPCInvocation).GetAttachmentWithDefaultValue(dubboInvokeStartTime,
"0"), 10, 64)
+
+ defer func() {
+ if err != nil {
+ // This err common is nil，when if not nil set a default
elapsed value 1
Review Comment:
The comment contains grammatical errors and unclear wording. It should be:
'// This err is commonly nil, when not nil set a default elapsed value 1' or
better yet: '// When err is not nil, use default elapsed value of 1'
```suggestion
// When err is not nil, use default elapsed value of 1
```
##########
filter/active/filter.go:
##########
@@ -71,12 +71,23 @@ func (f *activeFilter) Invoke(ctx context.Context, invoker
base.Invoker, inv bas
// OnResponse update the active count base on the request result.
func (f *activeFilter) OnResponse(ctx context.Context, result result.Result,
invoker base.Invoker, inv base.Invocation) result.Result {
startTime, err :=
strconv.ParseInt(inv.(*invocation.RPCInvocation).GetAttachmentWithDefaultValue(dubboInvokeStartTime,
"0"), 10, 64)
+
+ defer func() {
+ if err != nil {
+ // This err common is nil，when if not nil set a default
elapsed value 1
+ base.EndCount(invoker.GetURL(), inv.MethodName(), 1,
false)
+ return
+ }
+
+ elapsed := base.CurrentTimeMillis() - startTime
+ base.EndCount(invoker.GetURL(), inv.MethodName(), elapsed,
result.Error() == nil)
+ }()
+
if err != nil {
result.SetError(err)
logger.Errorf("parse dubbo_invoke_start_time to int64 failed")
return result
}
- elapsed := base.CurrentTimeMillis() - startTime
- base.EndCount(invoker.GetURL(), inv.MethodName(), elapsed,
result.Error() == nil)
+
Review Comment:
The defer function captures the `err` variable at the time the defer is
registered, not when it executes. Since `err` is assigned on line 73, it will
always have the same value in the defer function. The logic should check the
error condition and handle EndCount appropriately without relying on the
captured `err` variable.
--
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]
