Skip to content

Commit 6d7b7c0

Browse files
achingbrainachingbrain
authored
fix: do not throw when routing fails to find providers (#178)
If there are no providers available, the routing implementation should just yield no results instead of throwing an error.
1 parent 99e959b commit 6d7b7c0

File tree

7 files changed

+92
-40
lines changed

7 files changed

+92
-40
lines changed

‎packages/client/.aegir.js‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,11 @@ const options = {
2323
callCount++
2424
try {
2525

26-
let data
27-
try {
28-
// when passed data from a test where `body=providers.map(prov => JSON.stringify(prov)).join('\n')`
29-
data = { Providers: req.body.split('\n').map(line => JSON.parse(line)) }
30-
} catch (err) {
31-
// when passed data from a test where `body=JSON.stringify({ Providers: providers })`
32-
data = req.body
26+
// if request body content-type was json it's already been parsed
27+
const data = typeof req.body === 'string' ? JSON.parse(req.body) : req.body
28+
29+
if (!Array.isArray(data.Providers)) {
30+
throw new Error('Data must be { Providers: [] }')
3331
}
3432

3533
providers.set(req.params.cid, data)
@@ -50,6 +48,12 @@ const options = {
5048
const providerData = providers.get(req.params.cid) || { Providers: [] }
5149
const acceptHeader = req.headers.accept
5250

51+
if (providerData?.Providers?.length === 0) {
52+
res.statusCode = 404
53+
res.end()
54+
return
55+
}
56+
5357
if (acceptHeader?.includes('application/x-ndjson')) {
5458
res.setHeader('Content-Type', 'application/x-ndjson')
5559
const providers = Array.isArray(providerData.Providers) ? providerData.Providers : []

‎packages/client/src/client.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,14 +118,16 @@ export class DefaultDelegatedRoutingV1HttpApiClient implements DelegatedRoutingV
118118
await onStart.promise
119119

120120
// https://specs.ipfs.tech/routing/http-routing-v1/
121-
const url = new URL(`${this.clientUrl}routing/v1/providers/${cid.toString()}`)
121+
const url = new URL(`${this.clientUrl}routing/v1/providers/${cid}`)
122+
122123
this.#addFilterParams(url, options.filterAddrs, options.filterProtocols)
123124
const getOptions = { headers: { Accept: 'application/x-ndjson' }, signal }
124125
const res = await this.#makeRequest(url.toString(), getOptions)
125126

126127
if (res == null) {
127128
throw new BadResponseError('No response received')
128129
}
130+
129131
if (!res.ok) {
130132
if (res.status === 404) {
131133
// https://specs.ipfs.tech/routing/http-routing-v1/#response-status-codes

‎packages/client/src/routings.ts‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,22 @@ export class DelegatedRoutingV1HttpApiClientContentRouting implements ContentRou
2525
}
2626

2727
async * findProviders (cid: CID, options: AbortOptions = {}): AsyncIterable<PeerInfo> {
28-
yield * map(this.client.getProviders(cid, options), (record) => {
29-
return {
30-
id: record.ID,
31-
multiaddrs: record.Addrs ?? []
28+
try {
29+
yield * map(this.client.getProviders(cid, options), (record) => {
30+
return {
31+
id: record.ID,
32+
multiaddrs: record.Addrs ?? []
33+
}
34+
})
35+
} catch (err) {
36+
// NotFoundError means no providers were found so end the iterator instead
37+
// of throwing which means there was an error
38+
if (err instanceof NotFoundError) {
39+
return
3240
}
33-
})
41+
42+
throw err
43+
}
3444
}
3545

3646
async provide (): Promise<void> {

‎packages/client/test/index.spec.ts‎

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,11 @@ describe('delegated-routing-v1-http-api-client', () => {
5151
const cid = CID.parse('QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn')
5252

5353
// load providers for the router to fetch
54-
await fetch(`${process.env.ECHO_SERVER}/add-providers/${cid.toString()}`, {
54+
await fetch(`${process.env.ECHO_SERVER}/add-providers/${cid}`, {
5555
method: 'POST',
56-
body: providers.map(prov => JSON.stringify(prov)).join('\n')
56+
body: JSON.stringify({
57+
Providers: providers
58+
})
5759
})
5860

5961
const provs = await all(client.getProviders(cid))
@@ -84,12 +86,14 @@ describe('delegated-routing-v1-http-api-client', () => {
8486

8587
for (const contentType of contentTypes) {
8688
// Add providers with proper payload structure
87-
await fetch(`${process.env.ECHO_SERVER}/add-providers/${cid.toString()}`, {
89+
await fetch(`${process.env.ECHO_SERVER}/add-providers/${cid}`, {
8890
method: 'POST',
8991
headers: {
9092
'Content-Type': contentType
9193
},
92-
body: JSON.stringify({ Providers: providers })
94+
body: JSON.stringify({
95+
Providers: providers
96+
})
9397
})
9498

9599
await new Promise((resolve) => setTimeout(resolve, 100))
@@ -122,9 +126,11 @@ describe('delegated-routing-v1-http-api-client', () => {
122126
const cid = CID.parse('QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn')
123127

124128
// load providers for the router to fetch
125-
await fetch(`${process.env.ECHO_SERVER}/add-providers/${cid.toString()}`, {
129+
await fetch(`${process.env.ECHO_SERVER}/add-providers/${cid}`, {
126130
method: 'POST',
127-
body: providers.map(prov => JSON.stringify(prov)).join('\n')
131+
body: JSON.stringify({
132+
Providers: providers
133+
})
128134
})
129135

130136
await all(client.getProviders(cid, { filterProtocols: ['transport-bitswap', 'unknown'], filterAddrs: ['webtransport', '!p2p-circuit'] }))
@@ -162,9 +168,13 @@ describe('delegated-routing-v1-http-api-client', () => {
162168
const cid = CID.parse('QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn')
163169

164170
// load providers for the router to fetch
165-
await fetch(`${process.env.ECHO_SERVER}/add-providers/${cid.toString()}`, {
171+
await fetch(`${process.env.ECHO_SERVER}/add-providers/${cid}`, {
166172
method: 'POST',
167-
body: 'not json'
173+
body: JSON.stringify({
174+
Providers: [
175+
'not json'
176+
]
177+
})
168178
})
169179

170180
const provs = await all(client.getProviders(cid))
@@ -188,9 +198,11 @@ describe('delegated-routing-v1-http-api-client', () => {
188198
const cid = CID.parse('QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn')
189199

190200
// load providers for the router to fetch
191-
await fetch(`${process.env.ECHO_SERVER}/add-providers/${cid.toString()}`, {
201+
await fetch(`${process.env.ECHO_SERVER}/add-providers/${cid}`, {
192202
method: 'POST',
193-
body: providers.map(prov => JSON.stringify(prov)).join('\n')
203+
body: JSON.stringify({
204+
Providers: providers
205+
})
194206
})
195207

196208
const provs = await all(client.getProviders(cid))
@@ -351,9 +363,11 @@ describe('delegated-routing-v1-http-api-client', () => {
351363
}]
352364

353365
// load providers for the router to fetch
354-
await fetch(`${process.env.ECHO_SERVER}/add-providers/${cid.toString()}`, {
366+
await fetch(`${process.env.ECHO_SERVER}/add-providers/${cid}`, {
355367
method: 'POST',
356-
body: providers.map(prov => JSON.stringify(prov)).join('\n')
368+
body: JSON.stringify({
369+
Providers: providers
370+
})
357371
})
358372

359373
// Reset call count before our test
@@ -404,9 +418,11 @@ describe('delegated-routing-v1-http-api-client', () => {
404418
}]
405419

406420
// load providers for the router to fetch
407-
await fetch(`${process.env.ECHO_SERVER}/add-providers/${cid.toString()}`, {
421+
await fetch(`${process.env.ECHO_SERVER}/add-providers/${cid}`, {
408422
method: 'POST',
409-
body: providers.map(prov => JSON.stringify(prov)).join('\n')
423+
body: JSON.stringify({
424+
Providers: providers
425+
})
410426
})
411427

412428
// Reset call count

‎packages/client/test/routings.spec.ts‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,11 @@ describe('libp2p content-routing', () => {
6767
const cid = CID.parse('QmUNLLsPACCz1vLxQVkXqqLX5R1X345qqfHbsf67hvA3Nn')
6868

6969
// load providers for the router to fetch
70-
await fetch(`${process.env.ECHO_SERVER}/add-providers/${cid.toString()}`, {
70+
await fetch(`${process.env.ECHO_SERVER}/add-providers/${cid}`, {
7171
method: 'POST',
72-
body: providers.map(prov => JSON.stringify(prov)).join('\n')
72+
body: JSON.stringify({
73+
Providers: providers
74+
})
7375
})
7476

7577
const provs = await all(routing.findProviders(cid))
@@ -82,6 +84,26 @@ describe('libp2p content-routing', () => {
8284
})))
8385
})
8486

87+
it('should yield no results if no providers exist', async () => {
88+
const routing = getContentRouting(client)
89+
90+
if (routing == null) {
91+
throw new Error('ContentRouting not found')
92+
}
93+
94+
const cid = CID.parse('QmawceGscqN4o8Y8Fv26UUmB454kn2bnkXV5tEQYc4jBd7')
95+
96+
// load providers for the router to fetch
97+
await fetch(`${process.env.ECHO_SERVER}/add-providers/${cid}`, {
98+
method: 'POST',
99+
body: JSON.stringify({
100+
Providers: []
101+
})
102+
})
103+
104+
await expect(all(routing.findProviders(cid))).to.eventually.have.lengthOf(0)
105+
})
106+
85107
it('should respect abort signal when finding providers', async () => {
86108
const routing = getContentRouting(client)
87109

@@ -97,9 +119,11 @@ describe('libp2p content-routing', () => {
97119
const cid = CID.parse('QmawceGscqN4o8Y8Fv26UUmB454kn2bnkXV5tEQYc4jBd6')
98120

99121
// load providers for the router to fetch
100-
await fetch(`${process.env.ECHO_SERVER}/add-providers/${cid.toString()}`, {
122+
await fetch(`${process.env.ECHO_SERVER}/add-providers/${cid}`, {
101123
method: 'POST',
102-
body: providers.map(prov => JSON.stringify(prov)).join('\n')
124+
body: JSON.stringify({
125+
Providers: providers
126+
})
103127
})
104128

105129
let findProvidersFinished = false

‎packages/interop/package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@
2020
"lint": "aegir lint",
2121
"dep-check": "aegir dep-check",
2222
"build": "aegir build --bundle false",
23-
"test": "aegir test -t node",
24-
"test:node": "aegir test -t node --cov"
23+
"test": "aegir test -t node -- --exit",
24+
"test:node": "aegir test -t node --cov -- --exit"
2525
},
2626
"devDependencies": {
2727
"@helia/delegated-routing-v1-http-api-client": "^5.0.0",

‎packages/interop/test/index.spec.ts‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,9 @@ describe('delegated-routing-v1-http-api interop', () => {
5050
})
5151

5252
afterEach(async () => {
53-
await stop(client)
54-
55-
if (server != null) {
56-
await server.close()
57-
}
58-
5953
await Promise.all(network.map(async node => node.stop()))
54+
await server?.close()
55+
await stop(client)
6056
})
6157

6258
it('should find providers', async () => {
@@ -98,7 +94,7 @@ describe('delegated-routing-v1-http-api interop', () => {
9894

9995
// use client to resolve the published record
10096
const record = await client.getIPNS(result.publicKey.toCID())
101-
expect(record.value).to.equal(`/ipfs/${cid.toString()}`)
97+
expect(record.value).to.equal(`/ipfs/${cid}`)
10298
})
10399

104100
it.skip('should put an IPNS record', async () => {

0 commit comments

Comments
 (0)