fix: handle peer not found case

server_cached_router.go

@@ -10,6 +10,7 @@ import (
 	"github.com/ipfs/boxo/routing/http/types/iter"
 	"github.com/ipfs/go-cid"
 	"github.com/libp2p/go-libp2p/core/peer"
+	"github.com/libp2p/go-libp2p/core/routing"
 	ma "github.com/multiformats/go-multiaddr"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promauto"
@@ -68,10 +69,27 @@ func (r cachedRouter) FindProviders(ctx context.Context, key cid.Cid, limit int)
 // no point in trying to dispatch an additional FindPeer call.
 func (r cachedRouter) FindPeers(ctx context.Context, pid peer.ID, limit int) (iter.ResultIter[*types.PeerRecord], error) {
 	it, err := r.router.FindPeers(ctx, pid, limit)
+
+	if err == routing.ErrNotFound {
+		// If we didn't find the peer, try the cache
+		cachedAddrs := r.withAddrsFromCache(addrQueryOriginPeers, &pid, nil)
+		if len(cachedAddrs) > 0 {
+			return iter.ToResultIter(iter.FromSlice([]*types.PeerRecord{
+				{
+					Schema: types.SchemaPeer,
+					ID:     &pid,
+					Addrs:  cachedAddrs,
+				},
+			})), nil
+		}
+		return nil, routing.ErrNotFound
+	}
+
 	if err != nil {
 		return nil, err
 	}
 
+	// If the peer was found, there is likely no point in looking up the cache (because kad-dht will connect to it as part of FindPeers), but we'll do it just in case.
 	return iter.Map(it, func(record iter.Result[*types.PeerRecord]) iter.Result[*types.PeerRecord] {
 		record.Val.Addrs = r.withAddrsFromCache(addrQueryOriginPeers, record.Val.ID, record.Val.Addrs)
 		return record

server_cached_router_test.go

@@ -89,7 +89,6 @@ func TestCachedRouter(t *testing.T) {
 	})
 
 	t.Run("FindPeers with cache hit", func(t *testing.T) {
-		t.Skip("skipping until we decide if FindPeers should look up cache")
 		ctx := context.Background()
 		pid := peer.ID("test-peer")
 
@@ -161,71 +160,6 @@ func TestCachedRouter(t *testing.T) {
 		require.Equal(t, publicAddr.String(), results[0].Addrs[0].String())
 	})
 
-	t.Run("FindPeers handles records with and without addresses", func(t *testing.T) {
-		ctx := context.Background()
-		pid := peer.ID("test-peer")
-		publicAddr := mustMultiaddr(t, "/ip4/137.21.14.12/tcp/4001")
-
-		// Create mock router that returns a record without addresses
-		mr := &mockRouter{}
-		mockIter := newMockResultIter([]iter.Result[*types.PeerRecord]{
-			{Val: &types.PeerRecord{Schema: "peer", ID: &pid, Addrs: nil}},
-		})
-		mr.On("FindPeers", mock.Anything, pid, 10).Return(mockIter, nil)
-
-		// Create cached address book with test addresses
-		cab, err := newCachedAddrBook()
-		require.NoError(t, err)
-		cab.addrBook.AddAddrs(pid, []multiaddr.Multiaddr{publicAddr.Multiaddr}, time.Hour)
-
-		// Create cached router
-		cr := NewCachedRouter(mr, cab)
-
-		it, err := cr.FindPeers(ctx, pid, 10)
-		require.NoError(t, err)
-
-		results, err := iter.ReadAllResults(it)
-		require.NoError(t, err)
-		require.Len(t, results, 1)
-
-		// Verify cached addresses were added to the record
-		require.Equal(t, pid, *results[0].ID)
-		require.Len(t, results[0].Addrs, 1)
-		require.Equal(t, publicAddr.String(), results[0].Addrs[0].String())
-	})
-
-	t.Run("FindPeers returns same addresses as underlying router", func(t *testing.T) {
-		ctx := context.Background()
-		pid := peer.ID("test-peer")
-		publicAddr := mustMultiaddr(t, "/ip4/137.21.14.12/tcp/4001")
-
-		// Create mock router that returns a record with addresses
-		mr := &mockRouter{}
-		mockIter := newMockResultIter([]iter.Result[*types.PeerRecord]{
-			{Val: &types.PeerRecord{Schema: "peer", ID: &pid, Addrs: []types.Multiaddr{publicAddr}}},
-		})
-		mr.On("FindPeers", mock.Anything, pid, 10).Return(mockIter, nil)
-
-		// Create cached address book without any addresses
-		cab, err := newCachedAddrBook()
-		require.NoError(t, err)
-
-		// Create cached router
-		cr := NewCachedRouter(mr, cab)
-
-		it, err := cr.FindPeers(ctx, pid, 10)
-		require.NoError(t, err)
-
-		results, err := iter.ReadAllResults(it)
-		require.NoError(t, err)
-		require.Len(t, results, 1)
-
-		// Verify the addresses returned are the same as those from the underlying router
-		require.Equal(t, pid, *results[0].ID)
-		require.Len(t, results[0].Addrs, 1)
-		require.Equal(t, publicAddr.String(), results[0].Addrs[0].String())
-	})
-
 }
 
 func TestCacheFallbackIter(t *testing.T) {