fix(ipip-476): wire dht router to composableRouter #127
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…sabled fixes two issues with the GetClosestPeers endpoint: 1. endpoint returned HTTP 200 with empty results instead of actual DHT peers 2. when DHT disabled, returned HTTP 200 with empty results instead of error root cause: during rebase of PR #124 (commit 0dad3f7) when integrating with autoconf refactor (PR #123 / ec76365), the dhtRouters initialization was accidentally omitted from server.go. the autoconf PR renamed getCombinedRouting to combineRouters and changed its signature, but the rebase failed to preserve the dhtRouters creation line that existed in commit 42bd221. changes: - server.go:201-208: add explicit dhtRouters creation with caching and sanitization, similar to original 42bd221 implementation - server.go:232: wire dhtRouters to composableRouter.dht field - server.go:338: update combineRouters signature to accept host.Host parameter needed for GetClosestPeers peerstore lookups - server_routers.go:73-77: return routing.ErrNotSupported instead of empty iterator when DHT is nil, resulting in HTTP 500 instead of misleading HTTP 200 - server_test.go:16,20,24: update combineRouters test calls with new signature - server_dht_test.go:355-379: add test verifying HTTP 500 when DHT disabled
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes two issues with the GetClosestPeers endpoint:
IIUC during rebase of PR #124 (commit 0dad3f7) when integrating with autoconf refactor (PR #123 / ec76365), the
dhtRoutersinitialization was accidentally omitted fromserver.go. the autoconf PR renamedgetCombinedRoutingtocombineRoutersand changed its signature, but the rebase failed to preserve thedhtRouterscreation line that existed in commit 42bd221.This PR fixes that and now
GET /routing/v1/dht/closest/peers/{cid}returns HTTP 200 with results OR HTTP 500 if DHT is not available.cc @hsanjuan @gammazero for sanity check if I got this right