Skip to content

Commit

Permalink
fix: *url.Errors are not detected as equal
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
fho committed Oct 21, 2024
1 parent 297d15a commit b85ff6e
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 23 deletions.
6 changes: 5 additions & 1 deletion consul/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
57 changes: 57 additions & 0 deletions consul/resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package consul
import (
"errors"
"fmt"
"net"
"net/url"
"testing"
"time"
Expand Down Expand Up @@ -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())
}
}
17 changes: 13 additions & 4 deletions internal/mocks/clientconn.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -27,6 +28,7 @@ func (t *ClientConn) ReportError(err error) {
t.mutex.Lock()
defer t.mutex.Unlock()

t.reportErrorCallcnt++
t.lastReportedError = err
}

Expand All @@ -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()
Expand Down
41 changes: 23 additions & 18 deletions internal/mocks/consulhealth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}

0 comments on commit b85ff6e

Please sign in to comment.