From ce0e38d28240303f7afc7f37de441b067e3e855e Mon Sep 17 00:00:00 2001 From: Alex Potsides Date: Wed, 25 Oct 2023 08:35:12 +0100 Subject: [PATCH] fix: allow DHT self-query to time out (#2169) When we run our first self-query we first check the routing table to see if it has peers. If not we wait for some peers to be discovered before continuing. The change here is to use the general query AbortSignal to abort waiting for peers to be discovered. If they are not discovered allow other queries to continue and fail with eaiser to understand error messages. Also fixes some logging typos and makes error messages more understandable. --- packages/kad-dht/src/dual-kad-dht.ts | 12 +--------- packages/kad-dht/src/query-self.ts | 24 +++++++++++-------- packages/kad-dht/src/record/selectors.ts | 2 +- packages/kad-dht/src/record/validators.ts | 2 +- .../kad-dht/test/record/selection.spec.ts | 12 +++------- .../kad-dht/test/record/validator.spec.ts | 4 +--- 6 files changed, 21 insertions(+), 35 deletions(-) diff --git a/packages/kad-dht/src/dual-kad-dht.ts b/packages/kad-dht/src/dual-kad-dht.ts index efc93ab936..6d704725fb 100644 --- a/packages/kad-dht/src/dual-kad-dht.ts +++ b/packages/kad-dht/src/dual-kad-dht.ts @@ -103,8 +103,6 @@ function multiaddrIsPublic (multiaddr: Multiaddr): boolean { // dns4 or dns6 or dnsaddr if (tuples[0][0] === DNS4_CODE || tuples[0][0] === DNS6_CODE || tuples[0][0] === DNSADDR_CODE) { - log('%m is public %s', multiaddr, true) - return true } @@ -113,8 +111,6 @@ function multiaddrIsPublic (multiaddr: Multiaddr): boolean { const result = isPrivate(`${tuples[0][1]}`) const isPublic = result == null || !result - log('%m is public %s', multiaddr, isPublic) - return isPublic } @@ -170,13 +166,7 @@ export class DefaultDualKadDHT extends EventEmitter impleme components.events.addEventListener('self:peer:update', (evt) => { log('received update of self-peer info') const hasPublicAddress = evt.detail.peer.addresses - .some(({ multiaddr }) => { - const isPublic = multiaddrIsPublic(multiaddr) - - log('%m is public %s', multiaddr, isPublic) - - return isPublic - }) + .some(({ multiaddr }) => multiaddrIsPublic(multiaddr)) this.getMode() .then(async mode => { diff --git a/packages/kad-dht/src/query-self.ts b/packages/kad-dht/src/query-self.ts index 49f3826dbd..06c74d98e3 100644 --- a/packages/kad-dht/src/query-self.ts +++ b/packages/kad-dht/src/query-self.ts @@ -105,11 +105,6 @@ export class QuerySelf implements Startable { this.querySelfPromise = pDefer() - if (this.routingTable.size === 0) { - // wait to discover at least one DHT peer - await pEvent(this.routingTable, 'peer:add') - } - if (this.started) { this.controller = new AbortController() const signal = anySignal([this.controller.signal, AbortSignal.timeout(this.queryTimeout)]) @@ -122,7 +117,16 @@ export class QuerySelf implements Startable { } catch {} // fails on node < 15.4 try { + if (this.routingTable.size === 0) { + this.log('routing table was empty, waiting for some peers before running query') + // wait to discover at least one DHT peer + await pEvent(this.routingTable, 'peer:add', { + signal + }) + } + this.log('run self-query, look for %d peers timing out after %dms', this.count, this.queryTimeout) + const start = Date.now() const found = await pipe( this.peerRouting.getClosestPeers(this.components.peerId.toBytes(), { @@ -133,16 +137,16 @@ export class QuerySelf implements Startable { async (source) => length(source) ) - this.log('self-query ran successfully - found %d peers', found) + this.log('self-query found %d peers in %dms', found, Date.now() - start) + } catch (err: any) { + this.log.error('self-query error', err) + } finally { + signal.clear() if (this.initialQuerySelfHasRun != null) { this.initialQuerySelfHasRun.resolve() this.initialQuerySelfHasRun = undefined } - } catch (err: any) { - this.log.error('self-query error', err) - } finally { - signal.clear() } } diff --git a/packages/kad-dht/src/record/selectors.ts b/packages/kad-dht/src/record/selectors.ts index 7753bf3b03..825af34154 100644 --- a/packages/kad-dht/src/record/selectors.ts +++ b/packages/kad-dht/src/record/selectors.ts @@ -24,7 +24,7 @@ export function bestRecord (selectors: Selectors, k: Uint8Array, records: Uint8A const selector = selectors[parts[1].toString()] if (selector == null) { - const errMsg = `Unrecognized key prefix: ${parts[1]}` + const errMsg = `No selector function configured for key type "${parts[1]}"` throw new CodeError(errMsg, 'ERR_UNRECOGNIZED_KEY_PREFIX') } diff --git a/packages/kad-dht/src/record/validators.ts b/packages/kad-dht/src/record/validators.ts index 7302a083f5..a31e09b550 100644 --- a/packages/kad-dht/src/record/validators.ts +++ b/packages/kad-dht/src/record/validators.ts @@ -23,7 +23,7 @@ export async function verifyRecord (validators: Validators, record: Libp2pRecord const validator = validators[parts[1].toString()] if (validator == null) { - const errMsg = 'Invalid record keytype' + const errMsg = `No validator available for key type "${parts[1]}"` throw new CodeError(errMsg, 'ERR_INVALID_RECORD_KEY_TYPE') } diff --git a/packages/kad-dht/test/record/selection.spec.ts b/packages/kad-dht/test/record/selection.spec.ts index cf4b441943..49556941dd 100644 --- a/packages/kad-dht/test/record/selection.spec.ts +++ b/packages/kad-dht/test/record/selection.spec.ts @@ -13,26 +13,20 @@ describe('selection', () => { it('throws no records given when no records received', () => { expect( () => selection.bestRecord({}, uint8ArrayFromString('/'), []) - ).to.throw( - /No records given/ - ) + ).to.throw().with.property('code', 'ERR_NO_RECORDS_RECEIVED') }) it('throws on missing selector in the record key', () => { expect( () => selection.bestRecord({}, uint8ArrayFromString('/'), records) - ).to.throw( - /Record key does not have a selector function/ - ) + ).to.throw().with.property('code', 'ERR_NO_SELECTOR_FUNCTION_FOR_RECORD_KEY') }) it('throws on unknown key prefix', () => { expect( // @ts-expect-error invalid input () => selection.bestRecord({ world () {} }, uint8ArrayFromString('/hello/'), records) - ).to.throw( - /Unrecognized key prefix: hello/ - ) + ).to.throw().with.property('code', 'ERR_UNRECOGNIZED_KEY_PREFIX') }) it('returns the index from the matching selector', () => { diff --git a/packages/kad-dht/test/record/validator.spec.ts b/packages/kad-dht/test/record/validator.spec.ts index 0fd7c2320b..86fbb1b21b 100644 --- a/packages/kad-dht/test/record/validator.spec.ts +++ b/packages/kad-dht/test/record/validator.spec.ts @@ -86,9 +86,7 @@ describe('validator', () => { } } await expect(validator.verifyRecord(validators, rec)) - .to.eventually.rejectedWith( - /Invalid record keytype/ - ) + .to.eventually.rejected.with.property('code', 'ERR_INVALID_RECORD_KEY_TYPE') }) })