From 00264b2740f392dd15e61dc9644d21b1842a0d34 Mon Sep 17 00:00:00 2001 From: Turner Date: Fri, 1 Sep 2023 14:32:15 -0700 Subject: [PATCH 1/2] Add error handling that returns `None` and logs error --- crates/services/p2p/src/service.rs | 51 +++++++++++++++++++++++------- 1 file changed, 39 insertions(+), 12 deletions(-) diff --git a/crates/services/p2p/src/service.rs b/crates/services/p2p/src/service.rs index 136b0f7e758..1f97cb05690 100644 --- a/crates/services/p2p/src/service.rs +++ b/crates/services/p2p/src/service.rs @@ -287,27 +287,54 @@ where Some(FuelP2PEvent::RequestMessage { request_message, request_id }) => { match request_message { RequestMessage::Block(block_height) => { - // TODO: Process `StorageError` somehow. - let block_response = self.db.get_sealed_block(&block_height)? - .map(Arc::new); - let _ = self.p2p_service.send_response_msg(request_id, OutboundResponse::Block(block_response)); + match self.db.get_sealed_block(&block_height) { + Ok(maybe_block) => { + let response = maybe_block.map(Arc::new); + let _ = self.p2p_service.send_response_msg(request_id, OutboundResponse::Block(response)); + }, + Err(e) => { + tracing::error!("Failed to get block at height {:?}: {:?}", block_height, e); + let response = None; + let _ = self.p2p_service.send_response_msg(request_id, OutboundResponse::Block(response)); + return Err(e.into()) + } + } } RequestMessage::Transactions(block_id) => { - let transactions_response = self.db.get_transactions(&block_id)? - .map(Arc::new); - - let _ = self.p2p_service.send_response_msg(request_id, OutboundResponse::Transactions(transactions_response)); + match self.db.get_transactions(&block_id) { + Ok(maybe_transactions) => { + let response = maybe_transactions.map(Arc::new); + let _ = self.p2p_service.send_response_msg(request_id, OutboundResponse::Transactions(response)); + }, + Err(e) => { + tracing::error!("Failed to get transactions for block {:?}: {:?}", block_id, e); + let response = None; + let _ = self.p2p_service.send_response_msg(request_id, OutboundResponse::Transactions(response)); + return Err(e.into()) + } + } } RequestMessage::SealedHeaders(range) => { let max_len = self.max_headers_per_request.try_into().expect("u32 should always fit into usize"); - let response = if range.len() > max_len { + if range.len() > max_len { tracing::error!("Requested range of sealed headers is too big. Requested length: {:?}, Max length: {:?}", range.len(), max_len); // TODO: Return helpful error message to requester. https://github.com/FuelLabs/fuel-core/issues/1311 - None + let response = None; + let _ = self.p2p_service.send_response_msg(request_id, OutboundResponse::SealedHeaders(response)); } else { - Some(self.db.get_sealed_headers(range)?) + match self.db.get_sealed_headers(range.clone()) { + Ok(headers) => { + let response = Some(headers); + let _ = self.p2p_service.send_response_msg(request_id, OutboundResponse::SealedHeaders(response)); + }, + Err(e) => { + tracing::error!("Failed to get sealed headers for range {:?}: {:?}", range, &e); + let response = None; + let _ = self.p2p_service.send_response_msg(request_id, OutboundResponse::SealedHeaders(response)); + return Err(e.into()) + } + } }; - let _ = self.p2p_service.send_response_msg(request_id, OutboundResponse::SealedHeaders(response)); } } }, From 6944c30c1859e4b05ccdd94c40ed153981c725d3 Mon Sep 17 00:00:00 2001 From: Turner Date: Fri, 1 Sep 2023 14:41:06 -0700 Subject: [PATCH 2/2] Update CHANGELOG, replace `Breaking` in `Removed` section --- CHANGELOG.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9bb14791f5e..1bdbd931655 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,13 +27,13 @@ Description of the upcoming release here. It simplifies the maintenance and updating of the events, requiring only putting the event definition into the codebase of the relayer. - [#1293](https://github.com/FuelLabs/fuel-core/issues/1293): Parallelized the `estimate_predicates` endpoint to utilize all available threads. - [#1270](https://github.com/FuelLabs/fuel-core/pull/1270): Modify the way block headers are retrieved from peers to be done in batches. +- [#1342](https://github.com/FuelLabs/fuel-core/pull/1342): Add error handling for P2P requests to return `None` to requester and log error -#### Breaking +### Breaking - [#1318](https://github.com/FuelLabs/fuel-core/pull/1318): Removed the `--sync-max-header-batch-requests` CLI argument, and renamed `--sync-max-get-txns` to `--sync-block-stream-buffer-size` to better represent the current behavior in the import. - [#1290](https://github.com/FuelLabs/fuel-core/pull/1290): Standardize CLI args to use `-` instead of `_`. - [#1279](https://github.com/FuelLabs/fuel-core/pull/1279): Added a new CLI flag to enable the Relayer service `--enable-relayer`, and disabled the Relayer service by default. When supplying the `--enable-relayer` flag, the `--relayer` argument becomes mandatory, and omitting it is an error. Similarly, providing a `--relayer` argument without the `--enable-relayer` flag is an error. Lastly, providing the `--keypair` or `--network` arguments will also produce an error if the `--enable-p2p` flag is not set. - [#1262](https://github.com/FuelLabs/fuel-core/pull/1262): The `ConsensusParameters` aggregates all configuration data related to the consensus. It contains many fields that are segregated by the usage. The API of some functions was affected to use lesser types instead the whole `ConsensusParameters`. It is a huge breaking change requiring repetitively monotonically updating all places that use the `ConsensusParameters`. But during updating, consider that maybe you can use lesser types. Usage of them may simplify signatures of methods and make them more user-friendly and transparent. -- [#1322](https://github.com/FuelLabs/fuel-core/pull/1322): The `manual_blocks_enabled` flag is removed from the CLI. The analog is a `debug` flag. - [#1322](https://github.com/FuelLabs/fuel-core/pull/1322): The `debug` flag is added to the CLI. The flag should be used for local development only. Enabling debug mode: - Allows GraphQL Endpoints to arbitrarily advance blocks. @@ -41,3 +41,6 @@ Description of the upcoming release here. - Allows setting `utxo_validation` to `false`. ### Removed + +#### Breaking +- [#1322](https://github.com/FuelLabs/fuel-core/pull/1322): The `manual_blocks_enabled` flag is removed from the CLI. The analog is a `debug` flag.