From 9568f2c9c62a67a91bd503901804f8c27e15c19b Mon Sep 17 00:00:00 2001 From: achingbrain Date: Fri, 24 Apr 2020 07:47:05 +0100 Subject: [PATCH 1/2] fix: do not abort dht operation on error responses Response streams from DHT operations contain lots of message types, one of which is 'Error', but they are emitted for all sorts of reasons, failing to dial peers, etc and valid responses can arrive after several errors. The change here is to ignore error messages sent by the remote node as they do not indicate that the request has failed. It also makes the response types consistent in that previously we only turned some peer IDs into CIDs, now they are all CIDs all the time. Fixes #2991 --- packages/ipfs-http-client/src/dht/find-peer.js | 9 +++------ packages/ipfs-http-client/src/dht/find-provs.js | 14 +++----------- packages/ipfs-http-client/src/dht/get.js | 12 ++---------- packages/ipfs-http-client/src/dht/provide.js | 9 +-------- packages/ipfs-http-client/src/dht/put.js | 9 +-------- packages/ipfs-http-client/src/dht/query.js | 2 +- .../ipfs-http-client/src/dht/response-types.js | 14 ++++++++++++++ 7 files changed, 25 insertions(+), 44 deletions(-) create mode 100644 packages/ipfs-http-client/src/dht/response-types.js diff --git a/packages/ipfs-http-client/src/dht/find-peer.js b/packages/ipfs-http-client/src/dht/find-peer.js index 6453d973b3..a72e2b1249 100644 --- a/packages/ipfs-http-client/src/dht/find-peer.js +++ b/packages/ipfs-http-client/src/dht/find-peer.js @@ -5,6 +5,7 @@ const CID = require('cids') const multiaddr = require('multiaddr') const configure = require('../lib/configure') const toUrlSearchParams = require('../lib/to-url-search-params') +const { FinalPeer } = require('./response-types') module.exports = configure(api => { return async function findPeer (peerId, options = {}) { @@ -18,14 +19,10 @@ module.exports = configure(api => { }) for await (const data of res.ndjson()) { - if (data.Type === 3) { - throw new Error(data.Extra) - } - - if (data.Type === 2 && data.Responses) { + if (data.Type === FinalPeer && data.Responses) { const { ID, Addrs } = data.Responses[0] return { - id: ID, + id: new CID(ID), addrs: (Addrs || []).map(a => multiaddr(a)) } } diff --git a/packages/ipfs-http-client/src/dht/find-provs.js b/packages/ipfs-http-client/src/dht/find-provs.js index 02eece3ac8..4442aa1145 100644 --- a/packages/ipfs-http-client/src/dht/find-provs.js +++ b/packages/ipfs-http-client/src/dht/find-provs.js @@ -4,6 +4,7 @@ const CID = require('cids') const multiaddr = require('multiaddr') const configure = require('../lib/configure') const toUrlSearchParams = require('../lib/to-url-search-params') +const { Provider } = require('./response-types') module.exports = configure(api => { return async function * findProvs (cid, options = {}) { @@ -17,19 +18,10 @@ module.exports = configure(api => { }) for await (const message of res.ndjson()) { - // 3 = QueryError - // https://github.com/libp2p/go-libp2p-core/blob/6e566d10f4a5447317a66d64c7459954b969bdab/routing/query.go#L18 - // https://github.com/libp2p/go-libp2p-kad-dht/blob/master/routing.go#L525-L526 - if (message.Type === 3) { - throw new Error(message.Extra) - } - - // 4 = Provider - // https://github.com/libp2p/go-libp2p-core/blob/6e566d10f4a5447317a66d64c7459954b969bdab/routing/query.go#L20 - if (message.Type === 4 && message.Responses) { + if (message.Type === Provider && message.Responses) { for (const { ID, Addrs } of message.Responses) { yield { - id: ID, + id: new CID(ID), addrs: (Addrs || []).map(a => multiaddr(a)) } } diff --git a/packages/ipfs-http-client/src/dht/get.js b/packages/ipfs-http-client/src/dht/get.js index 1e157664bb..52922bfb20 100644 --- a/packages/ipfs-http-client/src/dht/get.js +++ b/packages/ipfs-http-client/src/dht/get.js @@ -4,6 +4,7 @@ const { Buffer } = require('buffer') const encodeBufferURIComponent = require('../lib/encode-buffer-uri-component') const configure = require('../lib/configure') const toUrlSearchParams = require('../lib/to-url-search-params') +const { Value } = require('./response-types') module.exports = configure(api => { return async function get (key, options = {}) { @@ -21,16 +22,7 @@ module.exports = configure(api => { }) for await (const message of res.ndjson()) { - // 3 = QueryError - // https://github.com/libp2p/go-libp2p-core/blob/6e566d10f4a5447317a66d64c7459954b969bdab/routing/query.go#L18 - // https://github.com/ipfs/go-ipfs/blob/eb11f569b064b960d1aba4b5b8ca155a3bd2cb21/core/commands/dht.go#L472-L473 - if (message.Type === 3) { - throw new Error(message.Extra) - } - - // 5 = Value - // https://github.com/libp2p/go-libp2p-core/blob/6e566d10f4a5447317a66d64c7459954b969bdab/routing/query.go#L21 - if (message.Type === 5) { + if (message.Type === Value) { return message.Extra } } diff --git a/packages/ipfs-http-client/src/dht/provide.js b/packages/ipfs-http-client/src/dht/provide.js index fa744c82a6..11200ddce2 100644 --- a/packages/ipfs-http-client/src/dht/provide.js +++ b/packages/ipfs-http-client/src/dht/provide.js @@ -20,18 +20,11 @@ module.exports = configure(api => { }) for await (let message of res.ndjson()) { - // 3 = QueryError - // https://github.com/libp2p/go-libp2p-core/blob/6e566d10f4a5447317a66d64c7459954b969bdab/routing/query.go#L18 - // https://github.com/ipfs/go-ipfs/blob/eb11f569b064b960d1aba4b5b8ca155a3bd2cb21/core/commands/dht.go#L283-L284 - if (message.Type === 3) { - throw new Error(message.Extra) - } - message = toCamel(message) message.id = new CID(message.id) if (message.responses) { message.responses = message.responses.map(({ ID, Addrs }) => ({ - id: ID, + id: new CID(ID), addrs: (Addrs || []).map(a => multiaddr(a)) })) } else { diff --git a/packages/ipfs-http-client/src/dht/put.js b/packages/ipfs-http-client/src/dht/put.js index 6a069cede1..fb52c55bf1 100644 --- a/packages/ipfs-http-client/src/dht/put.js +++ b/packages/ipfs-http-client/src/dht/put.js @@ -21,18 +21,11 @@ module.exports = configure(api => { }) for await (let message of res.ndjson()) { - // 3 = QueryError - // https://github.com/libp2p/go-libp2p-core/blob/6e566d10f4a5447317a66d64c7459954b969bdab/routing/query.go#L18 - // https://github.com/ipfs/go-ipfs/blob/eb11f569b064b960d1aba4b5b8ca155a3bd2cb21/core/commands/dht.go#L472-L473 - if (message.Type === 3) { - throw new Error(message.Extra) - } - message = toCamel(message) message.id = new CID(message.id) if (message.responses) { message.responses = message.responses.map(({ ID, Addrs }) => ({ - id: ID, + id: new CID(ID), addrs: (Addrs || []).map(a => multiaddr(a)) })) } diff --git a/packages/ipfs-http-client/src/dht/query.js b/packages/ipfs-http-client/src/dht/query.js index cdca5b6f8d..471b1aa72f 100644 --- a/packages/ipfs-http-client/src/dht/query.js +++ b/packages/ipfs-http-client/src/dht/query.js @@ -21,7 +21,7 @@ module.exports = configure(api => { message = toCamel(message) message.id = new CID(message.id) message.responses = (message.responses || []).map(({ ID, Addrs }) => ({ - id: ID, + id: new CID(ID), addrs: (Addrs || []).map(a => multiaddr(a)) })) yield message diff --git a/packages/ipfs-http-client/src/dht/response-types.js b/packages/ipfs-http-client/src/dht/response-types.js new file mode 100644 index 0000000000..bf52a709ac --- /dev/null +++ b/packages/ipfs-http-client/src/dht/response-types.js @@ -0,0 +1,14 @@ +'use strict' + +// Response types are defined here: +// https://github.com/libp2p/go-libp2p-core/blob/6e566d10f4a5447317a66d64c7459954b969bdab/routing/query.go#L15-L24 +module.exports = { + SendingQuery: 0, + PeerResponse: 1, + FinalPeer: 2, + QueryError: 3, + Provider: 4, + Value: 5, + AddingPeer: 6, + DialingPeer: 7 +} From 5cb4ea93b69ad7ac87d480d00d2086b595884903 Mon Sep 17 00:00:00 2001 From: achingbrain Date: Fri, 24 Apr 2020 16:46:06 +0100 Subject: [PATCH 2/2] fix: make peer ids strings --- packages/ipfs-http-client/src/dht/find-peer.js | 2 +- packages/ipfs-http-client/src/dht/find-provs.js | 2 +- packages/ipfs-http-client/src/dht/provide.js | 2 +- packages/ipfs-http-client/src/dht/put.js | 2 +- packages/ipfs-http-client/src/dht/query.js | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/ipfs-http-client/src/dht/find-peer.js b/packages/ipfs-http-client/src/dht/find-peer.js index a72e2b1249..f1ec56f90d 100644 --- a/packages/ipfs-http-client/src/dht/find-peer.js +++ b/packages/ipfs-http-client/src/dht/find-peer.js @@ -22,7 +22,7 @@ module.exports = configure(api => { if (data.Type === FinalPeer && data.Responses) { const { ID, Addrs } = data.Responses[0] return { - id: new CID(ID), + id: ID, addrs: (Addrs || []).map(a => multiaddr(a)) } } diff --git a/packages/ipfs-http-client/src/dht/find-provs.js b/packages/ipfs-http-client/src/dht/find-provs.js index 4442aa1145..54df1e689e 100644 --- a/packages/ipfs-http-client/src/dht/find-provs.js +++ b/packages/ipfs-http-client/src/dht/find-provs.js @@ -21,7 +21,7 @@ module.exports = configure(api => { if (message.Type === Provider && message.Responses) { for (const { ID, Addrs } of message.Responses) { yield { - id: new CID(ID), + id: ID, addrs: (Addrs || []).map(a => multiaddr(a)) } } diff --git a/packages/ipfs-http-client/src/dht/provide.js b/packages/ipfs-http-client/src/dht/provide.js index 11200ddce2..1dc7ad4d0b 100644 --- a/packages/ipfs-http-client/src/dht/provide.js +++ b/packages/ipfs-http-client/src/dht/provide.js @@ -24,7 +24,7 @@ module.exports = configure(api => { message.id = new CID(message.id) if (message.responses) { message.responses = message.responses.map(({ ID, Addrs }) => ({ - id: new CID(ID), + id: ID, addrs: (Addrs || []).map(a => multiaddr(a)) })) } else { diff --git a/packages/ipfs-http-client/src/dht/put.js b/packages/ipfs-http-client/src/dht/put.js index fb52c55bf1..4c4211f472 100644 --- a/packages/ipfs-http-client/src/dht/put.js +++ b/packages/ipfs-http-client/src/dht/put.js @@ -25,7 +25,7 @@ module.exports = configure(api => { message.id = new CID(message.id) if (message.responses) { message.responses = message.responses.map(({ ID, Addrs }) => ({ - id: new CID(ID), + id: ID, addrs: (Addrs || []).map(a => multiaddr(a)) })) } diff --git a/packages/ipfs-http-client/src/dht/query.js b/packages/ipfs-http-client/src/dht/query.js index 471b1aa72f..cdca5b6f8d 100644 --- a/packages/ipfs-http-client/src/dht/query.js +++ b/packages/ipfs-http-client/src/dht/query.js @@ -21,7 +21,7 @@ module.exports = configure(api => { message = toCamel(message) message.id = new CID(message.id) message.responses = (message.responses || []).map(({ ID, Addrs }) => ({ - id: new CID(ID), + id: ID, addrs: (Addrs || []).map(a => multiaddr(a)) })) yield message