fix(client): keep reference-level request timeout over consumer default (#3757)
* fix(client): keep reference-level request timeout over consumer default
Motivation:
ReferenceOptions.refer() builds the reference URL from getURLMap(), which
sets the timeout from the reference-level RequestTimeout, and then
unconditionally overwrote it with the consumer-level RequestTimeout. An
explicit WithRequestTimeout(500ms) on a reference therefore ended up as the
consumer default (e.g. 3s) in the URL timeout param, which the Triple client
reads as its default call timeout.
Approach:
Use the consumer-level timeout only as a fallback when the reference-level
timeout is empty. Behaviour is unchanged when only the consumer timeout (or
neither) is set. Per-call WithCallRequestTimeout is untouched.
Validation:
- Added TestReferenceTimeoutPrecedence (client/timeout_precedence_test.go),
which drives ReferenceOptions.Refer() with a mock protocol/cluster and
checks the timeout param on the URL. Without the fix the "reference wins"
case fails (got 3s, want 500ms); with the fix both cases pass.
- go test ./client/ -count=1: ok
- go build ./..., go vet ./client/, golangci-lint run ./client/...: clean
Not run: an end-to-end RPC measuring actual call duration; the test checks
URL construction only.
Report: https://github.com/apache/dubbo-go/issues/3756
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
Assisted-by: claude-sonnet-5-5 (via Claude Code)
* test(client): move timeout precedence test to action_test.go; clone overallReference per dial
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
---------
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
Co-authored-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
Co-authored-by: Xuetao Li <m134679102365478@163.com>
diff --git a/client/action.go b/client/action.go
index 935b668..23a7aef 100644
--- a/client/action.go
+++ b/client/action.go
@@ -126,6 +126,13 @@
ref.Loadbalance = constant.LoadBalanceKeyP2C
}
+ // The reference-level timeout (already in getURLMap) takes precedence over the consumer-level one.
+ // TODO: remove consumer timeout fallback after old config removed
+ timeout := ref.RequestTimeout
+ if len(timeout) == 0 {
+ timeout = refOpts.Consumer.RequestTimeout
+ }
+
// cfgURL is an interface-level invoker url, in the other words, it represents an interface.
cfgURL := common.NewURLWithOptions(
common.WithPath(ref.InterfaceName),
@@ -134,8 +141,7 @@
common.WithParams(refOpts.getURLMap()),
common.WithParamsValue(constant.BeanNameKey, refOpts.id),
common.WithParamsValue(constant.MetadataTypeKey, refOpts.metaDataType),
- // TODO: remove TimeoutKey after old confid removed
- common.WithParamsValue(constant.TimeoutKey, refOpts.Consumer.RequestTimeout),
+ common.WithParamsValue(constant.TimeoutKey, timeout),
// Compatibility: propagate legacy reference-level keepalive settings.
// TODO: remove KeepAliveInterval and KeepAliveTimeout in version 4.0.0.
diff --git a/client/action_test.go b/client/action_test.go
index fb2834c..09139a4 100644
--- a/client/action_test.go
+++ b/client/action_test.go
@@ -21,6 +21,7 @@
"os"
"strconv"
"testing"
+ "time"
)
import (
@@ -28,9 +29,13 @@
)
import (
+ "dubbo.apache.org/dubbo-go/v3/cluster/cluster"
+ "dubbo.apache.org/dubbo-go/v3/cluster/directory"
"dubbo.apache.org/dubbo-go/v3/common"
"dubbo.apache.org/dubbo-go/v3/common/constant"
+ "dubbo.apache.org/dubbo-go/v3/common/extension"
"dubbo.apache.org/dubbo-go/v3/global"
+ "dubbo.apache.org/dubbo-go/v3/protocol/base"
)
func TestGetEnv(t *testing.T) {
@@ -226,3 +231,73 @@
require.Error(t, err)
require.Contains(t, err.Error(), "no urls available")
}
+
+type timeoutCaptureProtocol struct{ captured *common.URL }
+
+func (p *timeoutCaptureProtocol) Export(base.Invoker) base.Exporter { return nil }
+func (p *timeoutCaptureProtocol) Destroy() {}
+func (p *timeoutCaptureProtocol) Refer(u *common.URL) base.Invoker {
+ p.captured = u
+ return base.NewBaseInvoker(u)
+}
+
+type timeoutCaptureCluster struct{}
+
+func (timeoutCaptureCluster) Join(d directory.Directory) base.Invoker {
+ return base.NewBaseInvoker(d.GetURL())
+}
+
+func TestReferenceTimeoutPrecedence(t *testing.T) {
+ tests := []struct {
+ name string
+ opts []ReferenceOption
+ want string
+ }{
+ {"reference timeout wins over consumer", []ReferenceOption{WithRequestTimeout(500 * time.Millisecond)}, "500ms"},
+ {"consumer timeout is the fallback", nil, "3s"},
+ }
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ p := &timeoutCaptureProtocol{}
+ extension.SetProtocol("timeout-precedence", func() base.Protocol { return p })
+ extension.SetCluster("timeout-precedence", func() cluster.Cluster { return timeoutCaptureCluster{} })
+ t.Cleanup(func() { extension.UnregisterProtocol("timeout-precedence") })
+
+ opts := defaultReferenceOptions()
+ opts.Consumer = global.DefaultConsumerConfig()
+ opts.Consumer.RequestTimeout = "3s"
+ for _, o := range tt.opts {
+ o(opts)
+ }
+ opts.Reference.InterfaceName = "example.TimeoutService"
+ opts.Reference.Protocol = "timeout-precedence"
+ opts.Reference.Cluster = "timeout-precedence"
+ opts.Reference.URL = "timeout-precedence://127.0.0.1:20000"
+ opts.Reference.Filter = "-default"
+ opts.Refer()
+
+ require.NotNil(t, p.captured)
+ require.Equal(t, tt.want, p.captured.GetParam(constant.TimeoutKey, ""))
+ })
+ }
+}
+
+func TestDialRequestTimeoutDoesNotLeakAcrossDials(t *testing.T) {
+ p := &timeoutCaptureProtocol{}
+ extension.SetProtocol("timeout-leak", func() base.Protocol { return p })
+ extension.SetCluster("timeout-leak", func() cluster.Cluster { return timeoutCaptureCluster{} })
+ t.Cleanup(func() { extension.UnregisterProtocol("timeout-leak") })
+
+ cli, err := NewClient()
+ require.NoError(t, err)
+ cli.cliOpts.Consumer.RequestTimeout = "3s"
+
+ dial := func(opts ...ReferenceOption) string {
+ opts = append(opts, WithProtocol("timeout-leak"), WithURL("timeout-leak://127.0.0.1:20000"), WithFilter("-default"), WithCluster("timeout-leak"))
+ _, err := cli.Dial("example.TimeoutService", opts...)
+ require.NoError(t, err)
+ return p.captured.GetParam(constant.TimeoutKey, "")
+ }
+ require.Equal(t, "500ms", dial(WithRequestTimeout(500*time.Millisecond)))
+ require.Equal(t, "3s", dial())
+}
diff --git a/client/client.go b/client/client.go
index 21c39be..6bef6c8 100644
--- a/client/client.go
+++ b/client/client.go
@@ -207,7 +207,7 @@
func (cli *Client) dial(interfaceName string, info *ClientInfo, srv any, opts ...ReferenceOption) (*Connection, error) {
newRefOpts := defaultReferenceOptions()
finalOpts := []ReferenceOption{
- setReference(cli.cliOpts.overallReference),
+ setReference(cli.cliOpts.overallReference.Clone()),
setApplication(cli.cliOpts.Application),
setRegistries(cli.cliOpts.Registries),
setConsumer(cli.cliOpts.Consumer),
