From 0ac94f854c76797bba4d8753a1e6f5a5027b0217 Mon Sep 17 00:00:00 2001 From: Adin Schmahmann Date: Tue, 5 Sep 2023 15:06:42 -0400 Subject: [PATCH 1/4] test: add test to make sure we return not found when we get errors back with values --- compparallel_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/compparallel_test.go b/compparallel_test.go index a5d7b63..63e0512 100644 --- a/compparallel_test.go +++ b/compparallel_test.go @@ -308,6 +308,28 @@ func TestComposableParallelFixtures(t *testing.T) { {key: "/wait/100ms/a", value: "av", searchValCount: 1}, }, }, + { + Name: "Return an error even if routers return data alongside the error", + routers: []*ParallelRouter{ + { + Timeout: 0, + IgnoreError: true, + Router: &Compose{ + PeerRouting: peerRoutingDataWithError{}, + }, + }, + { + Timeout: time.Second, + IgnoreError: true, + Router: &Compose{ + PeerRouting: peerRoutingDataWithError{}, + }, + }, + }, + FindPeer: []findPeerFixture{ + {peerID: "pid1", err: routing.ErrNotFound}, + }, + }, } for _, f := range fixtures { @@ -402,6 +424,12 @@ func TestComposableParallelFixtures(t *testing.T) { } } +type peerRoutingDataWithError struct{} + +func (r peerRoutingDataWithError) FindPeer(ctx context.Context, p peer.ID) (peer.AddrInfo, error) { + return peer.AddrInfo{ID: p}, routing.ErrNotFound +} + func newDummyPeerRouting(t testing.TB, ids []peer.ID) routing.PeerRouting { pr := dummyPeerRouter{} for _, id := range ids { From 9b41d704202483f1a06ddb5f6a63e33970a65121 Mon Sep 17 00:00:00 2001 From: Adin Schmahmann Date: Tue, 5 Sep 2023 15:07:49 -0400 Subject: [PATCH 2/4] fix: for getValueOrErrorParallel do not return values if they come with errors --- compparallel.go | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/compparallel.go b/compparallel.go index 4c12188..f84f6b5 100644 --- a/compparallel.go +++ b/compparallel.go @@ -231,17 +231,23 @@ func getValueOrErrorParallel[T any]( ctx, cancel := withCancelAndOptionalTimeout(ctx, r.Timeout) defer cancel() value, empty, err := f(ctx, r.Router) - if err != nil && - !errors.Is(err, routing.ErrNotFound) && - !r.IgnoreError { - log.Debug("getValueOrErrorParallel: error calling router function for router ", r.Router, - " with timeout ", r.Timeout, - " and ignore errors ", r.IgnoreError, - " with error ", err, - ) - select { - case <-ctx.Done(): - case errCh <- err: + if err != nil { + if !errors.Is(err, routing.ErrNotFound) && + !r.IgnoreError { + log.Debug("getValueOrErrorParallel: error calling router function for router ", r.Router, + " with timeout ", r.Timeout, + " and ignore errors ", r.IgnoreError, + " with error ", err, + ) + select { + case <-ctx.Done(): + case errCh <- err: + } + } else { + log.Debug("getValueOrErrorParallel: not found or ignorable error for router ", r.Router, + " with timeout ", r.Timeout, + " and ignore errors ", r.IgnoreError, + ) } return } From 7e0f5f57a2047427b2f7c7ba93c8da5ad6365119 Mon Sep 17 00:00:00 2001 From: Jorropo Date: Wed, 6 Sep 2023 00:08:36 +0200 Subject: [PATCH 3/4] nit: invert if I couldn't follow theses too nested ifs. --- compparallel.go | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/compparallel.go b/compparallel.go index f84f6b5..bbfd770 100644 --- a/compparallel.go +++ b/compparallel.go @@ -232,22 +232,21 @@ func getValueOrErrorParallel[T any]( defer cancel() value, empty, err := f(ctx, r.Router) if err != nil { - if !errors.Is(err, routing.ErrNotFound) && - !r.IgnoreError { - log.Debug("getValueOrErrorParallel: error calling router function for router ", r.Router, - " with timeout ", r.Timeout, - " and ignore errors ", r.IgnoreError, - " with error ", err, - ) - select { - case <-ctx.Done(): - case errCh <- err: - } - } else { + if r.IgnoreError || errors.Is(err, routing.ErrNotFound) { log.Debug("getValueOrErrorParallel: not found or ignorable error for router ", r.Router, " with timeout ", r.Timeout, " and ignore errors ", r.IgnoreError, ) + return + } + log.Debug("getValueOrErrorParallel: error calling router function for router ", r.Router, + " with timeout ", r.Timeout, + " and ignore errors ", r.IgnoreError, + " with error ", err, + ) + select { + case <-ctx.Done(): + case errCh <- err: } return } From 81e91cc424cf71415e77e0ddaab70ac729717348 Mon Sep 17 00:00:00 2001 From: Jorropo Date: Wed, 6 Sep 2023 00:24:14 +0200 Subject: [PATCH 4/4] chore: release v0.7.3 --- version.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.json b/version.json index e618866..257cf0b 100644 --- a/version.json +++ b/version.json @@ -1,3 +1,3 @@ { - "version": "v0.7.2" + "version": "v0.7.3" }