Skip to content

Commit

Permalink
fix(p2p): remove error event from Peer
Browse files Browse the repository at this point in the history
This removes the `error` event from `Peer`. Previously it was only used
to log the error from the listener in `Pool`, however this was
unnecessary and instead the logging can be done directly from `Peer`.

This simplifies the code and also resolves a mysterious crash condition
where an unhandled error from `Peer` would crash xud after a failed
inbound connection via tor.

Closes #1129.
  • Loading branch information
sangaman committed Oct 17, 2019
1 parent 24b4eb1 commit 8927a09
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 25 deletions.
19 changes: 2 additions & 17 deletions lib/p2p/Peer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ type PeerInfo = {

interface Peer {
on(event: 'packet', listener: (packet: Packet) => void): this;
on(event: 'error', listener: (err: Error) => void): this;
on(event: 'reputation', listener: (event: ReputationEvent) => void): this;
/** Adds a listener to be called when the peer's advertised but inactive pairs should be verified. */
on(event: 'verifyPairs', listener: () => void): this;
Expand All @@ -44,7 +43,6 @@ interface Peer {
emit(event: 'connect'): boolean;
emit(event: 'reputation', reputationEvent: ReputationEvent): boolean;
emit(event: 'close'): boolean;
emit(event: 'error', err: Error): boolean;
emit(event: 'packet', packet: Packet): boolean;
/** Notifies listeners that the peer's advertised but inactive pairs should be verified. */
emit(event: 'verifyPairs'): boolean;
Expand Down Expand Up @@ -576,7 +574,7 @@ class Peer extends EventEmitter {
if (now > entry.timeout) {
const request = PacketType[parseInt(packetId, 10)] || packetId;
const err = errors.RESPONSE_TIMEOUT(request);
this.emitError(err.message);
this.logger.error(`Peer timed out waiting for response to packet ${packetId}`);
entry.reject(err);
await this.close(DisconnectionReason.ResponseStalling, packetId);
}
Expand Down Expand Up @@ -648,8 +646,7 @@ class Peer extends EventEmitter {
assert(this.socket);

this.socket!.once('error', (err) => {
this.emitError(err);
// socket close event will be called immediately after the socket error
this.logger.error(`Peer (${this.label}) error`, err);
});

this.socket!.once('close', async (hadError) => {
Expand Down Expand Up @@ -740,18 +737,6 @@ class Peer extends EventEmitter {
}
}

private emitError = (err: Error | string): void => {
if (this.closed) {
return;
}

if (err instanceof Error) {
this.emit('error', err);
} else {
this.emit('error', new Error(err));
}
}

/**
* Authenticates the identity of a peer with a [[SessionInitPacket]] and sets the peer's node state.
* Throws an error and closes the peer if authentication fails.
Expand Down
8 changes: 0 additions & 8 deletions lib/p2p/Pool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -846,14 +846,6 @@ class Pool extends EventEmitter {
this.emit('peer.nodeStateUpdate', peer);
});

peer.on('error', (err) => {
// The only situation in which the node should be connected to itself is the
// reachability check of the advertised addresses and we don't have to log that
if (peer.nodePubKey !== this.nodePubKey) {
this.logger.error(`Peer (${peer.label}): error: ${err.message}`);
}
});

peer.once('close', () => this.handlePeerClose(peer));

peer.on('reputation', async (event) => {
Expand Down

0 comments on commit 8927a09

Please sign in to comment.