lisk-sdk icon indicating copy to clipboard operation
lisk-sdk copied to clipboard

[network.ts] event handlers related code should be put into respective private method

Open sitetester opened this issue 3 years ago • 0 comments

Description

  1. We have a large code block inside init method, which is just mixed with other code. We should move this into a private method something like registerEventHandlers to make it distinct & explicit about it's functionality.
// ---- START: Bind event handlers ----
this._p2p.on(EVENT_NETWORK_READY, () => {
	this._logger.debug('Node connected to the network');
	this.events.emit(ENGINE_EVENT_NETWORK_READY);
});

this._p2p.on(
	EVENT_CLOSE_OUTBOUND,
	({ peerInfo, code, reason }: liskP2P.p2pTypes.P2PClosePacket) => {
		this._logger.debug(
			{
				...peerInfo,
				code,
				reason,
			},
			'EVENT_CLOSE_OUTBOUND: Close outbound peer connection',
		);
	},
);

this._p2p.on(
	EVENT_CLOSE_INBOUND,
	({ peerInfo, code, reason }: liskP2P.p2pTypes.P2PClosePacket) => {
		this._logger.debug(
			{
				...peerInfo,
				code,
				reason,
			},
			'EVENT_CLOSE_INBOUND: Close inbound peer connection',
		);
	},
);

this._p2p.on(EVENT_CONNECT_OUTBOUND, peerInfo => {
	this._logger.debug(
		{
			...peerInfo,
		},
		'EVENT_CONNECT_OUTBOUND: Outbound peer connection',
	);
});

this._p2p.on(EVENT_DISCOVERED_PEER, peerInfo => {
	this._logger.trace(
		{
			...peerInfo,
		},
		'EVENT_DISCOVERED_PEER: Discovered peer connection',
	);
});

this._p2p.on(EVENT_NEW_INBOUND_PEER, peerInfo => {
	this._logger.debug(
		{
			...peerInfo,
		},
		'EVENT_NEW_INBOUND_PEER: Inbound peer connection',
	);
});

this._p2p.on(EVENT_FAILED_TO_FETCH_PEER_INFO, (error: Error) => {
	this._logger.error(
		{ err: error },
		'EVENT_FAILED_TO_FETCH_PEER_INFO: Failed to fetch peer info',
	);
});

this._p2p.on(EVENT_FAILED_TO_PUSH_NODE_INFO, (error: Error) => {
	this._logger.trace(
		{ err: error },
		'EVENT_FAILED_TO_PUSH_NODE_INFO: Failed to push node info',
	);
});

this._p2p.on(EVENT_OUTBOUND_SOCKET_ERROR, (error: Error) => {
	this._logger.debug({ err: error }, 'EVENT_OUTBOUND_SOCKET_ERROR: Outbound socket error');
});

this._p2p.on(EVENT_INBOUND_SOCKET_ERROR, (error: Error) => {
	this._logger.debug({ err: error }, 'EVENT_INBOUND_SOCKET_ERROR: Inbound socket error');
});

this._p2p.on(EVENT_UPDATED_PEER_INFO, peerInfo => {
	this._logger.trace(
		{
			...peerInfo,
		},
		'EVENT_UPDATED_PEER_INFO: Update peer info',
	);
});

this._p2p.on(EVENT_FAILED_PEER_INFO_UPDATE, (error: Error) => {
	this._logger.error({ err: error }, 'EVENT_FAILED_PEER_INFO_UPDATE: Failed peer update');
});

// eslint-disable-next-line @typescript-eslint/no-misused-promises
this._p2p.on(EVENT_REQUEST_RECEIVED, async (request: P2PRequest) => {
	this._logger.trace(
		{ procedure: request.procedure },
		'EVENT_REQUEST_RECEIVED: Received inbound request for procedure',
	);

	// If the request has already been handled internally by the P2P library, we ignore.
	if (request.wasResponseSent) {
		return;
	}

	if (!Object.keys(this._endpoints).includes(request.procedure)) {
		const error = new Error(`Requested procedure "${request.procedure}" is not permitted.`);
		this._logger.error(
			{ err: error, procedure: request.procedure },
			'Peer request not fulfilled event: Requested procedure is not permitted.',
		);

		// Ban peer on if non-permitted procedure is requested
		this._p2p.applyPenalty({ peerId: request.peerId, penalty: 100 });

		// Send an error back to the peer.
		request.error(error);
		return;
	}

	try {
		const result = await this._endpoints[request.procedure]({
			data: request.data,
			peerId: request.peerId,
		});
		this._logger.trace(
			{ procedure: request.procedure },
			'Peer request fulfilled event: Responded to peer request',
		);
		request.end(result as object); // Send the response back to the peer.
	} catch (error) {
		this._logger.error(
			{ err: error as Error, procedure: request.procedure },
			'Peer request not fulfilled event: Could not respond to peer request',
		);
		request.error(error as Error); // Send an error back to the peer.
	}
});

this._p2p.on(
	EVENT_MESSAGE_RECEIVED,
	(packet: {
		readonly peerId: string;
		readonly event: string;
		readonly data: Buffer | undefined;
	}) => {
		if (!Object.keys(this._eventHandlers).includes(packet.event)) {
			const error = new Error(`Sent event "${packet.event}" is not permitted.`);
			this._logger.error(
				{ err: error, event: packet.event },
				'Peer request not fulfilled. Sent event is not permitted.',
			);
			// Ban peer on if non-permitted procedure is requested
			this._p2p.applyPenalty({ peerId: packet.peerId, penalty: 100 });

			return;
		}
		this._logger.trace(
			{
				peerId: packet.peerId,
				event: packet.event,
			},
			'EVENT_MESSAGE_RECEIVED: Received inbound message',
		);
		try {
			this._eventHandlers[packet.event](packet);
		} catch (error) {
			this._logger.error(
				{ err: error as Error, event: packet.event },
				'Peer request not fulfilled event: Fail to handle event',
			);
			this._p2p.applyPenalty({ peerId: packet.peerId, penalty: 100 });
		}
	},
);
this._p2p.on(EVENT_BAN_PEER, (peerId: string) => {
	this._logger.error({ peerId }, 'EVENT_MESSAGE_RECEIVED: Peer has been banned temporarily');
});
  1. Additionally we can put some of them together in separate methods also (based on common pattern), e.g.,
  • all (error: Error) related logs should be together
  • & all peerInfo, code, reason could be together
  • all error messages corresponding to each type could be put into an array with even type as KEY, so we will not need to write separate logs for each even type

Motivation

  • This code is doing only one thing, so for for easier code management & not to mix it with other code, it would be more wise to put it into a private method.
  • All relevant dependencies should be passed to this new method.

Additional Information

framework/src/engine/network/network.ts

sitetester avatar Aug 20 '22 16:08 sitetester