Skip to content

Commit 785df52

Browse files
authoredAug 2, 2023
fix: improve connection manager error handling + edge cases (#1450)
* increase TTL on discovery classes * refactor dialPeer to handle edge cases * address comment
1 parent 0b8936f commit 785df52

File tree

3 files changed

+60
-53
lines changed

3 files changed

+60
-53
lines changed
 

‎packages/core/src/lib/connection_manager.ts

+58-51
Original file line numberDiff line numberDiff line change
@@ -174,51 +174,67 @@ export class ConnectionManager
174174
private async dialPeer(peerId: PeerId): Promise<void> {
175175
this.currentActiveDialCount += 1;
176176
let dialAttempt = 0;
177-
while (dialAttempt <= this.options.maxDialAttemptsForPeer) {
177+
while (dialAttempt < this.options.maxDialAttemptsForPeer) {
178178
try {
179-
log(`Dialing peer ${peerId.toString()}`);
179+
log(`Dialing peer ${peerId.toString()} on attempt ${dialAttempt + 1}`);
180180
await this.libp2p.dial(peerId);
181181

182182
const tags = await this.getTagNamesForPeer(peerId);
183183
// add tag to connection describing discovery mechanism
184184
// don't add duplicate tags
185-
this.libp2p
186-
.getConnections(peerId)
187-
.forEach(
188-
(conn) => (conn.tags = Array.from(new Set([...conn.tags, ...tags])))
189-
);
185+
this.libp2p.getConnections(peerId).forEach((conn) => {
186+
conn.tags = Array.from(new Set([...conn.tags, ...tags]));
187+
});
190188

191189
this.dialAttemptsForPeer.delete(peerId.toString());
192-
return;
193-
} catch (e) {
194-
const error = e as AggregateError;
195-
190+
// Dialing succeeded, break the loop
191+
break;
192+
} catch (error) {
193+
if (error instanceof AggregateError) {
194+
// Handle AggregateError
195+
log(`Error dialing peer ${peerId.toString()} - ${error.errors}`);
196+
} else {
197+
// Handle generic error
198+
log(
199+
`Error dialing peer ${peerId.toString()} - ${
200+
(error as any).message
201+
}`
202+
);
203+
}
196204
this.dialErrorsForPeer.set(peerId.toString(), error);
197-
log(`Error dialing peer ${peerId.toString()} - ${error.errors}`);
198205

199-
dialAttempt = this.dialAttemptsForPeer.get(peerId.toString()) ?? 1;
200-
this.dialAttemptsForPeer.set(peerId.toString(), dialAttempt + 1);
201-
202-
if (dialAttempt <= this.options.maxDialAttemptsForPeer) {
203-
log(`Reattempting dial (${dialAttempt})`);
204-
}
206+
dialAttempt++;
207+
this.dialAttemptsForPeer.set(peerId.toString(), dialAttempt);
205208
}
206209
}
207210

208-
try {
209-
log(
210-
`Deleting undialable peer ${peerId.toString()} from peer store. Error: ${JSON.stringify(
211-
this.dialErrorsForPeer.get(peerId.toString()).errors[0]
212-
)}
213-
}`
214-
);
215-
this.dialErrorsForPeer.delete(peerId.toString());
216-
return await this.libp2p.peerStore.delete(peerId);
217-
} catch (error) {
218-
throw `Error deleting undialable peer ${peerId.toString()} from peer store - ${error}`;
219-
} finally {
220-
this.currentActiveDialCount -= 1;
221-
this.processDialQueue();
211+
// Always decrease the active dial count and process the dial queue
212+
this.currentActiveDialCount--;
213+
this.processDialQueue();
214+
215+
// If max dial attempts reached and dialing failed, delete the peer
216+
if (dialAttempt === this.options.maxDialAttemptsForPeer) {
217+
try {
218+
const error = this.dialErrorsForPeer.get(peerId.toString());
219+
220+
let errorMessage;
221+
if (error instanceof AggregateError) {
222+
errorMessage = JSON.stringify(error.errors[0]);
223+
} else {
224+
errorMessage = error.message;
225+
}
226+
227+
log(
228+
`Deleting undialable peer ${peerId.toString()} from peer store. Error: ${errorMessage}`
229+
);
230+
231+
this.dialErrorsForPeer.delete(peerId.toString());
232+
await this.libp2p.peerStore.delete(peerId);
233+
} catch (error) {
234+
throw new Error(
235+
`Error deleting undialable peer ${peerId.toString()} from peer store - ${error}`
236+
);
237+
}
222238
}
223239
}
224240

@@ -302,25 +318,16 @@ export class ConnectionManager
302318
Tags.BOOTSTRAP
303319
);
304320

305-
if (isBootstrap) {
306-
this.dispatchEvent(
307-
new CustomEvent<PeerId>(
308-
EPeersByDiscoveryEvents.PEER_DISCOVERY_BOOTSTRAP,
309-
{
310-
detail: peerId,
311-
}
312-
)
313-
);
314-
} else {
315-
this.dispatchEvent(
316-
new CustomEvent<PeerId>(
317-
EPeersByDiscoveryEvents.PEER_DISCOVERY_PEER_EXCHANGE,
318-
{
319-
detail: peerId,
320-
}
321-
)
322-
);
323-
}
321+
this.dispatchEvent(
322+
new CustomEvent<PeerId>(
323+
isBootstrap
324+
? EPeersByDiscoveryEvents.PEER_DISCOVERY_BOOTSTRAP
325+
: EPeersByDiscoveryEvents.PEER_DISCOVERY_PEER_EXCHANGE,
326+
{
327+
detail: peerId,
328+
}
329+
)
330+
);
324331

325332
try {
326333
await this.attemptDial(peerId);

‎packages/dns-discovery/src/index.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ const enrTree = {
2222

2323
const DEFAULT_BOOTSTRAP_TAG_NAME = "bootstrap";
2424
const DEFAULT_BOOTSTRAP_TAG_VALUE = 50;
25-
const DEFAULT_BOOTSTRAP_TAG_TTL = 120000;
25+
const DEFAULT_BOOTSTRAP_TAG_TTL = 100_000_000;
2626

2727
export interface DnsDiscoveryComponents {
2828
peerStore: PeerStore;

‎packages/peer-exchange/src/waku_peer_exchange_discovery.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ export interface Options {
4747

4848
export const DEFAULT_PEER_EXCHANGE_TAG_NAME = Tags.PEER_EXCHANGE;
4949
const DEFAULT_PEER_EXCHANGE_TAG_VALUE = 50;
50-
const DEFAULT_PEER_EXCHANGE_TAG_TTL = 120000;
50+
const DEFAULT_PEER_EXCHANGE_TAG_TTL = 100_000_000;
5151

5252
export class PeerExchangeDiscovery
5353
extends EventEmitter<PeerDiscoveryEvents>

0 commit comments

Comments
 (0)