feat(routing/http): return 200 for empty results per IPIP-513 (#1032)

* feat: implement IPIP-513 for routing v1 endpoints

return HTTP 200 with empty results instead of 404 when no
records found, while maintaining backward compatibility
in clients to handle both response codes

Fixes: #1024
Spec: ipfs/specs#513

* docs: add IPIP-513 changes to changelog

* fix: correct off-by-one error in routing_http_client_length metric

the metric now correctly reports:
- 0 for empty results (instead of 1)
- n for n results (instead of n+1)

* feat: add metrics for IPNS operations

- GetIPNS now records latency, status code, and length (0 or 1)
- PutIPNS now records latency and status code
- uses consistent pattern with httpReq.Host for host extraction

* docs: explain histogram bucket interpretation for metrics

- document how OpenCensus bucket selection works (value < boundary)
- explain mapping to Prometheus le buckets for both latency and length
- provide examples of how to extract meaningful data from metrics

* feat: add simple counter metrics for requests and positive responses

- routing_http_client_requests_total: counts all requests including errors
- routing_http_client_positive_responses_total: counts requests with 1+ results
- avoids confusing histogram bucket math for common queries
- documented with simple Grafana query examples

* fix: drain response body for connection reuse in 404 cases

ensure http connection can be reused by draining response body before closing

Author: lidel

CHANGELOG.md

@@ -23,6 +23,10 @@ The following emojis are used to highlight certain changes:
 
 ### Changed
 
+- `routing/http`: ✨ Delegated Routing V1 HTTP endpoints now return 200 with empty results instead of 404 when no records are found, per [IPIP-513](https://github.com/ipfs/specs/pull/513) ([#1024](https://github.com/ipfs/boxo/issues/1024))
+  - Server endpoints (`/routing/v1/providers/{cid}`, `/routing/v1/peers/{peer-id}`, `/routing/v1/ipns/{name}`) return HTTP 200 with empty JSON arrays or appropriate content types for empty results
+  - Client maintains backward compatibility by treating both 200 with empty results and 404 as "no records found"
+  - IPNS endpoint distinguishes between valid records (Content-Type: `application/vnd.ipfs.ipns-record`) and no record found (any other content type)
 - `verifcid`: 🛠 Enhanced Allowlist interface with per-hash size limits ([#1018](https://github.com/ipfs/boxo/pull/1018))
   - Expanded `Allowlist` interface with `MinDigestSize(code uint64)` and `MaxDigestSize(code uint64)` methods for per-hash function size validation
   - Added public constants: `DefaultMinDigestSize` (20 bytes), `DefaultMaxDigestSize` (128 bytes for cryptographic hashes), and `DefaultMaxIdentityDigestSize` (128 bytes for identity CIDs)
@@ -37,6 +41,12 @@ The following emojis are used to highlight certain changes:
 
 ### Fixed
 
+- `routing/http/client`:
+  - Fixed off-by-one error in `routing_http_client_length` metric - the metric now correctly reports 0 for empty results instead of 1
+  - Added metrics for IPNS operations (`GetIPNS` and `PutIPNS`) - these now report latency, status code, and result count (0 or 1 for GetIPNS)
+  - Added simple counter metrics to avoid confusing histogram bucket math:
+    - `routing_http_client_requests_total` - total requests including errors
+    - `routing_http_client_positive_responses_total` - requests that returned at least 1 result
 - `ipld/unixfs/mod`:
   - `DagModifier` now correctly preserves raw node codec when modifying data under the chunker threshold, instead of incorrectly forcing everything to dag-pb
   - `DagModifier` prevents creation of identity CIDs exceeding `verifcid.DefaultMaxIdentityDigestSize` limit when modifying data, automatically switching to proper cryptographic hash while preserving small identity CIDs

routing/http/client/client.go

@@ -27,6 +27,7 @@ import (
 	logging "github.com/ipfs/go-log/v2"
 	"github.com/libp2p/go-libp2p/core/crypto"
 	"github.com/libp2p/go-libp2p/core/peer"
+	"github.com/libp2p/go-libp2p/core/routing"
 	"github.com/multiformats/go-multiaddr"
 )
 
@@ -234,8 +235,11 @@ type measuringIter[T any] struct {
 }
 
 func (c *measuringIter[T]) Next() bool {
-	c.m.length++
-	return c.Iter.Next()
+	hasNext := c.Iter.Next()
+	if hasNext {
+		c.m.length++
+	}
+	return hasNext
 }
 
 func (c *measuringIter[T]) Val() T {
@@ -279,7 +283,10 @@ func (c *Client) FindProviders(ctx context.Context, key cid.Cid) (providers iter
 	}
 
 	m.statusCode = resp.StatusCode
+	// Per IPIP-0513: Handle 404 as empty results for backward compatibility
+	// New servers return 200 with empty results, old servers return 404
 	if resp.StatusCode == http.StatusNotFound {
+		io.Copy(io.Discard, resp.Body) // Drain body for connection reuse
 		resp.Body.Close()
 		m.record(ctx)
 		return iter.FromSlice[iter.Result[types.Record]](nil), nil
@@ -462,7 +469,10 @@ func (c *Client) FindPeers(ctx context.Context, pid peer.ID) (peers iter.ResultI
 	}
 
 	m.statusCode = resp.StatusCode
+	// Per IPIP-0513: Handle 404 as empty results for backward compatibility
+	// New servers return 200 with empty results, old servers return 404
 	if resp.StatusCode == http.StatusNotFound {
+		io.Copy(io.Discard, resp.Body) // Drain body for connection reuse
 		resp.Body.Close()
 		m.record(ctx)
 		return iter.FromSlice[iter.Result[*types.PeerRecord]](nil), nil
@@ -518,7 +528,20 @@ func (c *Client) FindPeers(ctx context.Context, pid peer.ID) (peers iter.ResultI
 // GetIPNS tries to retrieve the [ipns.Record] for the given [ipns.Name]. The record is
 // validated against the given name. If validation fails, an error is returned, but no
 // record.
-func (c *Client) GetIPNS(ctx context.Context, name ipns.Name) (*ipns.Record, error) {
+func (c *Client) GetIPNS(ctx context.Context, name ipns.Name) (record *ipns.Record, err error) {
+	m := newMeasurement("GetIPNS")
+	start := time.Now()
+	defer func() {
+		m.latency = time.Since(start)
+		m.err = err
+		if record != nil {
+			m.length = 1
+		} else {
+			m.length = 0
+		}
+		m.record(ctx)
+	}()
+
 	url := c.baseURL + "/routing/v1/ipns/" + name.String()
 
 	httpReq, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
@@ -527,23 +550,42 @@ func (c *Client) GetIPNS(ctx context.Context, name ipns.Name) (*ipns.Record, err
 	}
 	httpReq.Header.Set("Accept", mediaTypeIPNSRecord)
 
+	m.host = httpReq.Host
+
 	resp, err := c.httpClient.Do(httpReq)
 	if err != nil {
 		return nil, fmt.Errorf("making HTTP req to get IPNS record: %w", err)
 	}
 	defer resp.Body.Close()
 
+	m.statusCode = resp.StatusCode
+
+	// Per IPIP-0513: Handle 404 as "no record found" for backward compatibility
+	// New servers return 200 with text/plain for no record, old servers return 404
+	if resp.StatusCode == http.StatusNotFound {
+		io.Copy(io.Discard, resp.Body) // Drain body for connection reuse
+		return nil, routing.ErrNotFound
+	}
+
 	if resp.StatusCode != http.StatusOK {
 		return nil, httpError(resp.StatusCode, resp.Body)
 	}
 
+	// Per IPIP-0513: Only Content-Type: application/vnd.ipfs.ipns-record indicates a valid record
+	// Any other content type (e.g., text/plain) means no record found
+	contentType := resp.Header.Get("Content-Type")
+	if !strings.Contains(contentType, mediaTypeIPNSRecord) {
+		io.Copy(io.Discard, resp.Body) // Drain body for connection reuse
+		return nil, routing.ErrNotFound
+	}
+
 	// Limit the reader to the maximum record size.
 	rawRecord, err := io.ReadAll(io.LimitReader(resp.Body, int64(ipns.MaxRecordSize)))
 	if err != nil {
 		return nil, fmt.Errorf("making HTTP req to get IPNS record: %w", err)
 	}
 
-	record, err := ipns.UnmarshalRecord(rawRecord)
+	record, err = ipns.UnmarshalRecord(rawRecord)
 	if err != nil {
 		return nil, fmt.Errorf("IPNS record from remote endpoint is not valid: %w", err)
 	}
@@ -557,7 +599,16 @@ func (c *Client) GetIPNS(ctx context.Context, name ipns.Name) (*ipns.Record, err
 }
 
 // PutIPNS attempts at putting the given [ipns.Record] for the given [ipns.Name].
-func (c *Client) PutIPNS(ctx context.Context, name ipns.Name, record *ipns.Record) error {
+func (c *Client) PutIPNS(ctx context.Context, name ipns.Name, record *ipns.Record) (err error) {
+	m := newMeasurement("PutIPNS")
+	start := time.Now()
+	defer func() {
+		m.latency = time.Since(start)
+		m.err = err
+		// Don't set m.length for PutIPNS - not meaningful
+		m.record(ctx)
+	}()
+
 	url := c.baseURL + "/routing/v1/ipns/" + name.String()
 
 	rawRecord, err := ipns.MarshalRecord(record)
@@ -571,12 +622,16 @@ func (c *Client) PutIPNS(ctx context.Context, name ipns.Name, record *ipns.Recor
 	}
 	httpReq.Header.Set("Content-Type", mediaTypeIPNSRecord)
 
+	m.host = httpReq.Host
+
 	resp, err := c.httpClient.Do(httpReq)
 	if err != nil {
 		return fmt.Errorf("making HTTP req to get IPNS record: %w", err)
 	}
 	defer resp.Body.Close()
 
+	m.statusCode = resp.StatusCode
+
 	if resp.StatusCode != http.StatusOK {
 		return httpError(resp.StatusCode, resp.Body)
 	}

routing/http/client/client_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/ipfs/go-cid"
 	"github.com/libp2p/go-libp2p/core/crypto"
 	"github.com/libp2p/go-libp2p/core/peer"
+	"github.com/libp2p/go-libp2p/core/routing"
 	"github.com/multiformats/go-multiaddr"
 	"github.com/multiformats/go-multibase"
 	"github.com/multiformats/go-multihash"
@@ -317,6 +318,19 @@ func TestClient_FindProviders(t *testing.T) {
 			httpStatusCode: 404,
 			expResult:      nil,
 		},
+		{
+			name:                    "returns no providers if the HTTP server returns 200 with empty JSON array",
+			routerResult:            []iter.Result[types.Record]{},
+			expResult:               nil,
+			serverStreamingDisabled: true,
+			expJSONResponse:         true,
+		},
+		{
+			name:                 "returns no providers if the HTTP server returns 200 with empty NDJSON",
+			routerResult:         []iter.Result[types.Record]{},
+			expResult:            nil,
+			expStreamingResponse: true,
+		},
 	}
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
@@ -604,6 +618,19 @@ func TestClient_FindPeers(t *testing.T) {
 			httpStatusCode: 404,
 			expResult:      nil,
 		},
+		{
+			name:                    "returns no providers if the HTTP server returns 200 with empty JSON array",
+			routerResult:            []iter.Result[*types.PeerRecord]{},
+			expResult:               nil,
+			serverStreamingDisabled: true,
+			expJSONResponse:         true,
+		},
+		{
+			name:                 "returns no providers if the HTTP server returns 200 with empty NDJSON",
+			routerResult:         []iter.Result[*types.PeerRecord]{},
+			expResult:            nil,
+			expStreamingResponse: true,
+		},
 	}
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
@@ -676,6 +703,139 @@ func TestClient_FindPeers(t *testing.T) {
 	}
 }
 
+func TestClient_EmptyResponses(t *testing.T) {
+	// Test that client handles various empty response formats correctly
+	cid := makeCID()
+	pid := peer.ID("12D3KooWD3eckifWpRn9wQpMG9R9hX3sD158z7EqHWmweQAJU5SA")
+	_, name := makeName(t)
+
+	// Helper to create test server with given response
+	makeTestServer := func(handler http.HandlerFunc) *httptest.Server {
+		return httptest.NewServer(handler)
+	}
+
+	// Helper to verify empty results for providers
+	verifyEmptyProviders := func(t *testing.T, serverURL string) {
+		client, err := New(serverURL, WithHTTPClient((&http.Client{})))
+		require.NoError(t, err)
+
+		resultsIter, err := client.FindProviders(context.Background(), cid)
+		require.NoError(t, err)
+
+		results, err := iter.ReadAllResults(resultsIter)
+		require.NoError(t, err)
+		require.Empty(t, results)
+	}
+
+	// Helper to verify empty results for peers
+	verifyEmptyPeers := func(t *testing.T, serverURL string) {
+		client, err := New(serverURL, WithHTTPClient((&http.Client{})))
+		require.NoError(t, err)
+
+		resultsIter, err := client.FindPeers(context.Background(), pid)
+		require.NoError(t, err)
+
+		results, err := iter.ReadAllResults(resultsIter)
+		require.NoError(t, err)
+		require.Empty(t, results)
+	}
+
+	// Helper to verify IPNS not found
+	verifyIPNSNotFound := func(t *testing.T, serverURL string) {
+		client, err := New(serverURL, WithHTTPClient((&http.Client{})))
+		require.NoError(t, err)
+
+		record, err := client.GetIPNS(context.Background(), name)
+		require.ErrorIs(t, err, routing.ErrNotFound)
+		require.Nil(t, record)
+	}
+
+	testCases := []struct {
+		name     string
+		handler  http.HandlerFunc
+		testFunc func(*testing.T, string)
+	}{
+		// FindProviders tests
+		{
+			name: "404 Not Found (legacy server)",
+			handler: func(w http.ResponseWriter, r *http.Request) {
+				w.WriteHeader(http.StatusNotFound)
+				w.Write([]byte("404 page not found"))
+			},
+			testFunc: verifyEmptyProviders,
+		},
+		{
+			name: "200 with null providers in JSON",
+			handler: func(w http.ResponseWriter, r *http.Request) {
+				w.Header().Set("Content-Type", "application/json")
+				// Old server behavior - returns null instead of empty array
+				w.Write([]byte(`{"Providers":null}`))
+			},
+			testFunc: verifyEmptyProviders,
+		},
+		{
+			name: "200 with empty array in JSON",
+			handler: func(w http.ResponseWriter, r *http.Request) {
+				w.Header().Set("Content-Type", "application/json")
+				// New server behavior - returns empty array
+				w.Write([]byte(`{"Providers":[]}`))
+			},
+			testFunc: verifyEmptyProviders,
+		},
+		{
+			name: "200 with empty NDJSON stream",
+			handler: func(w http.ResponseWriter, r *http.Request) {
+				w.Header().Set("Content-Type", "application/x-ndjson")
+				// Empty body - no NDJSON lines
+			},
+			testFunc: verifyEmptyProviders,
+		},
+		// FindPeers tests
+		{
+			name: "FindPeers: 404 Not Found (legacy server)",
+			handler: func(w http.ResponseWriter, r *http.Request) {
+				w.WriteHeader(http.StatusNotFound)
+				w.Write([]byte("404 page not found"))
+			},
+			testFunc: verifyEmptyPeers,
+		},
+		{
+			name: "FindPeers: 200 with null peers in JSON",
+			handler: func(w http.ResponseWriter, r *http.Request) {
+				w.Header().Set("Content-Type", "application/json")
+				// Old server behavior - returns null instead of empty array
+				w.Write([]byte(`{"Peers":null}`))
+			},
+			testFunc: verifyEmptyPeers,
+		},
+		// IPNS tests
+		{
+			name: "IPNS: 404 Not Found (legacy server)",
+			handler: func(w http.ResponseWriter, r *http.Request) {
+				w.WriteHeader(http.StatusNotFound)
+				w.Write([]byte("404 page not found"))
+			},
+			testFunc: verifyIPNSNotFound,
+		},
+		{
+			name: "IPNS: 200 with text/plain (no record found)",
+			handler: func(w http.ResponseWriter, r *http.Request) {
+				w.Header().Set("Content-Type", "text/plain; charset=utf-8")
+				w.Write([]byte("delegate error: routing: not found"))
+			},
+			testFunc: verifyIPNSNotFound,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			server := makeTestServer(tc.handler)
+			defer server.Close()
+			tc.testFunc(t, server.URL)
+		})
+	}
+}
+
 func TestNormalizeBaseURL(t *testing.T) {
 	cases := []struct {
 		name        string