Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

fix: dht.findPeer API endpoint returns ndjson #2965

Merged
merged 1 commit into from
Apr 6, 2020

Conversation

achingbrain
Copy link
Member

@achingbrain achingbrain commented Apr 2, 2020

It doesn't appear to be documented, but POST /api/v0/dht/findpeer can return multiple results, so treat it as ndjson.

Otherwise we see these sorts of test failures:

1) DelegatedPeerRouting
       findPeers
         should be able to find peers via the delegate with a peer id string:
     FetchError: invalid json response body at http://127.0.0.1:42421/api/v0/dht/findpeer?search-params=%5Bobject+Object%5D&arg=QmcvRwsBkiEfLFPC6NAXmbpt1f9oKDT9dmmgAcEsGd9ZuW reason: Unexpected token { in JSON at position 93

@@ -9,23 +9,23 @@ module.exports = configure(api => {
return async function findPeer (peerId, options = {}) {
options.arg = `${Buffer.isBuffer(peerId) ? new CID(peerId) : peerId}`

const res = await api.post('dht/findpeer', {
const res = await api.ndjson('dht/findpeer', {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this was changed from a .post it needs to include method: 'POST' in the options.
See https://github.com/ipfs/js-ipfs/blob/master/packages/ipfs-http-client/src/dht/find-provs.js#L10-L15

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point - I think @lidel was going to PR the client to change all methods to post.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can do that now though..

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taking shape here: #2977

SgtPooki referenced this pull request in ipfs/js-kubo-rpc-client Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants