diff --git a/consul/resolver.go b/consul/resolver.go index c284bfc..1eb79f0 100644 --- a/consul/resolver.go +++ b/consul/resolver.go @@ -283,7 +283,11 @@ func (c *consulResolver) reportAddress(addrs []resolver.Address) bool { } func (c *consulResolver) reportError(err error) bool { - if c.lastReporterState.err == err { //nolint: errorlint + // We compare the string representation of the errors because it is + // simple and works. http.Client.Do() returns [*url.Error]s which are not + // equal when compared with "==", neither [url.Error.Err] does because + // it e.g. can contain a *net.OpError. + if c.lastReporterState.err != nil && c.lastReporterState.err.Error() == err.Error() { return false } diff --git a/consul/resolver_test.go b/consul/resolver_test.go index e7e8dcd..2f006b8 100644 --- a/consul/resolver_test.go +++ b/consul/resolver_test.go @@ -3,6 +3,7 @@ package consul import ( "errors" "fmt" + "net" "net/url" "testing" "time" @@ -610,3 +611,59 @@ func TestRetryOnError(t *testing.T) { time.Sleep(50 * time.Millisecond) } } + +func TestErrorsOnlyReportedOnce(t *testing.T) { + var qe1 error = &url.Error{ + Op: "test", + Err: &net.OpError{Op: "none"}, + } + var qe2 error = &url.Error{ + Op: "test", + Err: &net.OpError{Op: "none"}, + } + + health := mocks.NewConsulHealthClient() + health.SetRespError(qe1) + health.ServiceMultipleTagsFn = func(c *mocks.ConsulHealthClient, _ string, _ []string, _ bool, _ *consul.QueryOptions) ([]*consul.ServiceEntry, *consul.QueryMeta, error) { + e1 := c.Err + health.SetRespError(qe2) + + c.Mutex.Lock() + defer c.Mutex.Unlock() + + c.ResolveCnt++ + + return nil, nil, e1 + } + + cleanup := replaceCreateHealthClientFn( + func(*consul.Config) (consulHealthEndpoint, error) { + return health, nil + }, + ) + t.Cleanup(cleanup) + + cc := mocks.NewClientConn() + b := NewBuilder() + target := resolver.Target{URL: url.URL{Path: "user-service"}} + + r, err := b.Build(target, cc, resolver.BuildOptions{}) + if err != nil { + t.Fatal("Build() failed:", err.Error()) + } + + r.ResolveNow(resolver.ResolveNowOptions{}) + for health.ResolveCount() < 1 { + time.Sleep(time.Millisecond) + } + + r.ResolveNow(resolver.ResolveNowOptions{}) + + for health.ResolveCount() < 2 { + time.Sleep(5 * time.Millisecond) + } + + if cc.ReportErrorCallCnt() != 1 { + t.Errorf("ReportError was called %d times, expecting 1 call", cc.ReportErrorCallCnt()) + } +} diff --git a/internal/mocks/clientconn.go b/internal/mocks/clientconn.go index 47ea516..6360a90 100644 --- a/internal/mocks/clientconn.go +++ b/internal/mocks/clientconn.go @@ -9,10 +9,11 @@ import ( ) type ClientConn struct { - mutex sync.Mutex - addrs []resolver.Address - newAddressCallCnt int - lastReportedError error + mutex sync.Mutex + addrs []resolver.Address + newAddressCallCnt int + reportErrorCallcnt int + lastReportedError error } func NewClientConn() *ClientConn { @@ -27,6 +28,7 @@ func (t *ClientConn) ReportError(err error) { t.mutex.Lock() defer t.mutex.Unlock() + t.reportErrorCallcnt++ t.lastReportedError = err } @@ -37,6 +39,13 @@ func (t *ClientConn) LastReportedError() error { return t.lastReportedError } +func (t *ClientConn) ReportErrorCallCnt() int { + t.mutex.Lock() + defer t.mutex.Unlock() + + return t.reportErrorCallcnt +} + func (t *ClientConn) UpdateState(state resolver.State) error { t.mutex.Lock() defer t.mutex.Unlock() diff --git a/internal/mocks/consulhealth.go b/internal/mocks/consulhealth.go index 384421c..8b90bf7 100644 --- a/internal/mocks/consulhealth.go +++ b/internal/mocks/consulhealth.go @@ -7,11 +7,12 @@ import ( ) type ConsulHealthClient struct { - mutex sync.Mutex - entries []*consul.ServiceEntry - queryMeta consul.QueryMeta - err error - resolveCnt int + Mutex sync.Mutex + Entries []*consul.ServiceEntry + queryMeta consul.QueryMeta + ResolveCnt int + Err error + ServiceMultipleTagsFn func(*ConsulHealthClient, string, []string, bool, *consul.QueryOptions) ([]*consul.ServiceEntry, *consul.QueryMeta, error) } func NewConsulHealthClient() *ConsulHealthClient { @@ -31,33 +32,37 @@ func (c *ConsulHealthClient) SetRespServiceEntries(s []*consul.AgentService) { } func (c *ConsulHealthClient) SetRespEntries(entries []*consul.ServiceEntry) { - c.mutex.Lock() - defer c.mutex.Unlock() + c.Mutex.Lock() + defer c.Mutex.Unlock() - c.entries = entries + c.Entries = entries } func (c *ConsulHealthClient) SetRespError(err error) { - c.mutex.Lock() - defer c.mutex.Unlock() + c.Mutex.Lock() + defer c.Mutex.Unlock() - c.err = err + c.Err = err } func (c *ConsulHealthClient) ServiceMultipleTags(_ string, _ []string, _ bool, q *consul.QueryOptions) ([]*consul.ServiceEntry, *consul.QueryMeta, error) { - c.mutex.Lock() - defer c.mutex.Unlock() - c.resolveCnt++ + if c.ServiceMultipleTagsFn != nil { + return c.ServiceMultipleTagsFn(c, "", nil, false, q) + } + + c.Mutex.Lock() + defer c.Mutex.Unlock() + c.ResolveCnt++ if q.Context().Err() != nil { return nil, nil, q.Context().Err() } - return c.entries, &c.queryMeta, c.err + return c.Entries, &c.queryMeta, c.Err } func (c *ConsulHealthClient) ResolveCount() int { - c.mutex.Lock() - defer c.mutex.Unlock() - return c.resolveCnt + c.Mutex.Lock() + defer c.Mutex.Unlock() + return c.ResolveCnt }