fix: *url.Errors are not detected as equal

Different instances of errors having the same struct values are unequal.
That caused that *url.Errors with the same information did not evaluate
as equale, compare the string represtation of the errors instead do
determine equality.

Author: Fabian Holler

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
 	}
 

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())
+	}
+}

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()

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
 }