Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add error handling for P2P requests to return None to requester and log error #1342

Merged
merged 2 commits into from
Sep 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,20 @@ 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.
- Enables debugger GraphQL Endpoints.
- 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.
51 changes: 39 additions & 12 deletions crates/services/p2p/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
},
Expand Down
Loading