Skip to content

Commit

Permalink
chore: use Display instead of Debug for most errors (paradigmxyz#6777)
Browse files Browse the repository at this point in the history
  • Loading branch information
DaniPopes authored Feb 24, 2024
1 parent da3f469 commit 94cb6a8
Show file tree
Hide file tree
Showing 36 changed files with 106 additions and 105 deletions.
2 changes: 1 addition & 1 deletion crates/blockchain-tree/src/blockchain_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -837,7 +837,7 @@ impl<DB: Database, EVM: ExecutorFactory> BlockchainTree<DB, EVM> {
.try_insert_validated_block(block, BlockValidationKind::SkipStateRootValidation)
.map_err(|err| {
debug!(
target: "blockchain_tree", ?err,
target: "blockchain_tree", %err,
"Failed to insert buffered block",
);
err
Expand Down
4 changes: 2 additions & 2 deletions crates/consensus/auto-seal/src/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ where
}
}
Err(err) => {
error!(target: "consensus::auto", ?err, "Autoseal fork choice update failed");
error!(target: "consensus::auto", %err, "Autoseal fork choice update failed");
return None
}
}
Expand Down Expand Up @@ -219,7 +219,7 @@ where
.send(reth_provider::CanonStateNotification::Commit { new: chain });
}
Err(err) => {
warn!(target: "consensus::auto", ?err, "failed to execute block")
warn!(target: "consensus::auto", %err, "failed to execute block")
}
}

Expand Down
28 changes: 14 additions & 14 deletions crates/consensus/beacon/src/engine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ where
Err(error) => {
if let RethError::Canonical(ref err) = error {
if err.is_fatal() {
tracing::error!(target: "consensus::engine", ?err, "Encountered fatal error");
tracing::error!(target: "consensus::engine", %err, "Encountered fatal error");
return Err(error)
}
}
Expand Down Expand Up @@ -658,7 +658,7 @@ where
// skip the pipeline run
match self.blockchain.header_by_hash_or_number(state.finalized_block_hash.into()) {
Err(err) => {
warn!(target: "consensus::engine", ?err, "Failed to get finalized block header");
warn!(target: "consensus::engine", %err, "Failed to get finalized block header");
}
Ok(None) => {
// we don't have the block yet and the distance exceeds the allowed
Expand Down Expand Up @@ -984,16 +984,16 @@ where
// check if the new head was previously invalidated, if so then we deem this FCU
// as invalid
if let Some(invalid_ancestor) = self.check_invalid_ancestor(state.head_block_hash) {
warn!(target: "consensus::engine", ?error, ?state, ?invalid_ancestor, head=?state.head_block_hash, "Failed to canonicalize the head hash, head is also considered invalid");
debug!(target: "consensus::engine", head=?state.head_block_hash, current_error=?error, "Head was previously marked as invalid");
warn!(target: "consensus::engine", %error, ?state, ?invalid_ancestor, head=?state.head_block_hash, "Failed to canonicalize the head hash, head is also considered invalid");
debug!(target: "consensus::engine", head=?state.head_block_hash, current_error=%error, "Head was previously marked as invalid");
return invalid_ancestor
}

match &error {
RethError::Canonical(
error @ CanonicalError::Validation(BlockValidationError::BlockPreMerge { .. }),
) => {
warn!(target: "consensus::engine", ?error, ?state, "Failed to canonicalize the head hash");
warn!(target: "consensus::engine", %error, ?state, "Failed to canonicalize the head hash");
return PayloadStatus::from_status(PayloadStatusEnum::Invalid {
validation_error: error.to_string(),
})
Expand All @@ -1007,7 +1007,7 @@ where
// to a new target and is considered normal operation during sync
}
_ => {
warn!(target: "consensus::engine", ?error, ?state, "Failed to canonicalize the head hash");
warn!(target: "consensus::engine", %error, ?state, "Failed to canonicalize the head hash");
// TODO(mattsse) better error handling before attempting to sync (FCU could be
// invalid): only trigger sync if we can't determine whether the FCU is invalid
}
Expand Down Expand Up @@ -1127,7 +1127,7 @@ where
Ok(status)
}
Err(error) => {
warn!(target: "consensus::engine", ?error, "Error while processing payload");
warn!(target: "consensus::engine", %error, "Error while processing payload");
self.map_insert_error(error)
}
};
Expand Down Expand Up @@ -1170,7 +1170,7 @@ where
match self.payload_validator.ensure_well_formed_payload(payload, cancun_fields.into()) {
Ok(block) => Ok(block),
Err(error) => {
error!(target: "consensus::engine", ?error, "Invalid payload");
error!(target: "consensus::engine", %error, "Invalid payload");
// we need to convert the error to a payload status (response to the CL)

let latest_valid_hash =
Expand Down Expand Up @@ -1322,7 +1322,7 @@ where
let (block, error) = err.split();

if error.is_invalid_block() {
warn!(target: "consensus::engine", invalid_hash=?block.hash(), invalid_number=?block.number, ?error, "Invalid block error on new payload");
warn!(target: "consensus::engine", invalid_hash=?block.hash(), invalid_number=?block.number, %error, "Invalid block error on new payload");

// all of these occurred if the payload is invalid
let parent_hash = block.parent_hash;
Expand Down Expand Up @@ -1412,10 +1412,10 @@ where
}
}
Err(err) => {
warn!(target: "consensus::engine", ?err, "Failed to insert downloaded block");
warn!(target: "consensus::engine", %err, "Failed to insert downloaded block");
if err.kind().is_invalid_block() {
let (block, err) = err.split();
warn!(target: "consensus::engine", invalid_number=?block.number, invalid_hash=?block.hash(), ?err, "Marking block as invalid");
warn!(target: "consensus::engine", invalid_number=?block.number, invalid_hash=?block.hash(), %err, "Marking block as invalid");

self.invalid_headers.insert(block.header);
}
Expand Down Expand Up @@ -1608,7 +1608,7 @@ where
}
},
Err(error) => {
error!(target: "consensus::engine", ?error, "Error getting canonical header for continuous sync");
error!(target: "consensus::engine", %error, "Error getting canonical header for continuous sync");
return Some(Err(RethError::Provider(error).into()))
}
};
Expand Down Expand Up @@ -1690,7 +1690,7 @@ where
}
}
Err(error) => {
error!(target: "consensus::engine", ?error, "Error restoring blockchain tree state");
error!(target: "consensus::engine", %error, "Error restoring blockchain tree state");
return Some(Err(error.into()))
}
};
Expand Down Expand Up @@ -1724,7 +1724,7 @@ where
if let Err(error) =
self.blockchain.connect_buffered_blocks_to_canonical_hashes()
{
error!(target: "consensus::engine", ?error, "Error connecting buffered blocks to canonical hashes on hook result");
error!(target: "consensus::engine", %error, "Error connecting buffered blocks to canonical hashes on hook result");
return Err(error.into())
}
}
Expand Down
6 changes: 3 additions & 3 deletions crates/interfaces/src/p2p/full_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ where
BodyResponse::PendingValidation(resp) => {
// ensure the block is valid, else retry
if let Err(err) = ensure_valid_body_response(&header, resp.data()) {
debug!(target: "downloaders", ?err, hash=?header.hash(), "Received wrong body");
debug!(target: "downloaders", %err, hash=?header.hash(), "Received wrong body");
self.client.report_bad_message(resp.peer_id());
self.header = Some(header);
self.request.body = Some(self.client.get_block_body(self.hash));
Expand All @@ -164,7 +164,7 @@ where
fn on_block_response(&mut self, resp: WithPeerId<BlockBody>) {
if let Some(ref header) = self.header {
if let Err(err) = ensure_valid_body_response(header, resp.data()) {
debug!(target: "downloaders", ?err, hash=?header.hash(), "Received wrong body");
debug!(target: "downloaders", %err, hash=?header.hash(), "Received wrong body");
self.client.report_bad_message(resp.peer_id());
return
}
Expand Down Expand Up @@ -438,7 +438,7 @@ where
BodyResponse::PendingValidation(resp) => {
// ensure the block is valid, else retry
if let Err(err) = ensure_valid_body_response(header, resp.data()) {
debug!(target: "downloaders", ?err, hash=?header.hash(), "Received wrong body in range response");
debug!(target: "downloaders", %err, hash=?header.hash(), "Received wrong body in range response");
self.client.report_bad_message(resp.peer_id());

// get body that doesn't match, put back into vecdeque, and retry it
Expand Down
8 changes: 4 additions & 4 deletions crates/net/discv4/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1639,7 +1639,7 @@ impl Discv4Service {
debug!(target: "discv4", %err, "failed to read datagram");
}
IngressEvent::BadPacket(from, err, data) => {
debug!(target: "discv4", ?from, ?err, packet=?hex::encode(&data), "bad packet");
debug!(target: "discv4", ?from, %err, packet=?hex::encode(&data), "bad packet");
}
IngressEvent::Packet(remote_addr, Packet { msg, node_id, hash }) => {
trace!(target: "discv4", r#type=?msg.msg_type(), from=?remote_addr,"received packet");
Expand Down Expand Up @@ -1765,7 +1765,7 @@ pub(crate) async fn send_loop(udp: Arc<UdpSocket>, rx: EgressReceiver) {
trace!(target: "discv4", ?to, ?size,"sent payload");
}
Err(err) => {
debug!(target: "discv4", ?to, ?err,"Failed to send datagram.");
debug!(target: "discv4", ?to, %err,"Failed to send datagram.");
}
}
}
Expand All @@ -1788,7 +1788,7 @@ pub(crate) async fn receive_loop(udp: Arc<UdpSocket>, tx: IngressSender, local_i
let res = udp.recv_from(&mut buf).await;
match res {
Err(err) => {
debug!(target: "discv4", ?err, "Failed to read datagram.");
debug!(target: "discv4", %err, "Failed to read datagram.");
send(IngressEvent::RecvError(err)).await;
}
Ok((read, remote_addr)) => {
Expand All @@ -1803,7 +1803,7 @@ pub(crate) async fn receive_loop(udp: Arc<UdpSocket>, tx: IngressSender, local_i
send(IngressEvent::Packet(remote_addr, packet)).await;
}
Err(err) => {
debug!(target: "discv4", ?err,"Failed to decode packet");
debug!(target: "discv4", %err,"Failed to decode packet");
send(IngressEvent::BadPacket(remote_addr, err, packet.to_vec())).await
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/net/discv4/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ impl Stream for MockDiscovery {
match event {
IngressEvent::RecvError(_) => {}
IngressEvent::BadPacket(from, err, data) => {
debug!(target: "discv4", ?from, ?err, packet=?hex::encode(&data), "bad packet");
debug!(target: "discv4", ?from, %err, packet=?hex::encode(&data), "bad packet");
}
IngressEvent::Packet(remote_addr, Packet { msg, node_id, hash }) => match msg {
Message::Ping(ping) => {
Expand Down
4 changes: 2 additions & 2 deletions crates/net/dns/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ impl<R: Resolver> DnsDiscoveryService<R> {
}
},
Err((err, link)) => {
debug!(target: "disc::dns",?err, ?link, "Failed to lookup root")
debug!(target: "disc::dns",%err, ?link, "Failed to lookup root")
}
}
}
Expand All @@ -251,7 +251,7 @@ impl<R: Resolver> DnsDiscoveryService<R> {

match entry {
Some(Err(err)) => {
debug!(target: "disc::dns",?err, domain=%link.domain, ?hash, "Failed to lookup entry")
debug!(target: "disc::dns",%err, domain=%link.domain, ?hash, "Failed to lookup entry")
}
None => {
debug!(target: "disc::dns",domain=%link.domain, ?hash, "No dns entry")
Expand Down
2 changes: 1 addition & 1 deletion crates/net/dns/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ impl<P: ConnectionProvider> Resolver for AsyncResolver<P> {
let fqn = if query.ends_with('.') { query.to_string() } else { format!("{query}.") };
match self.txt_lookup(fqn).await {
Err(err) => {
trace!(target: "disc::dns", ?err, ?query, "dns lookup failed");
trace!(target: "disc::dns", %err, ?query, "dns lookup failed");
None
}
Ok(lookup) => {
Expand Down
4 changes: 2 additions & 2 deletions crates/net/downloaders/src/bodies/bodies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ where
this.buffer_bodies_response(response);
}
Err(error) => {
tracing::debug!(target: "downloaders::bodies", ?error, "Request failed");
tracing::debug!(target: "downloaders::bodies", %error, "Request failed");
this.clear();
return Poll::Ready(Some(Err(error)))
}
Expand All @@ -386,7 +386,7 @@ where
}
Ok(None) => break 'inner,
Err(error) => {
tracing::error!(target: "downloaders::bodies", ?error, "Failed to download from next request");
tracing::error!(target: "downloaders::bodies", %error, "Failed to download from next request");
this.clear();
return Poll::Ready(Some(Err(error)))
}
Expand Down
2 changes: 1 addition & 1 deletion crates/net/downloaders/src/bodies/task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ impl<T: BodyDownloader> Future for SpawnedDownloader<T> {
while let Poll::Ready(update) = this.updates.poll_next_unpin(cx) {
if let Some(range) = update {
if let Err(err) = this.downloader.set_download_range(range) {
tracing::error!(target: "downloaders::bodies", ?err, "Failed to set bodies download range");
tracing::error!(target: "downloaders::bodies", %err, "Failed to set bodies download range");

// Clone the sender ensure its availability. See [PollSender::clone].
let mut bodies_tx = this.bodies_tx.clone();
Expand Down
10 changes: 5 additions & 5 deletions crates/net/downloaders/src/headers/reverse_headers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ where
validated.last().or_else(|| self.lowest_validated_header())
{
if let Err(error) = self.validate(validated_header, &parent) {
trace!(target: "downloaders::headers", ?error ,"Failed to validate header");
trace!(target: "downloaders::headers", %error ,"Failed to validate header");
return Err(
HeadersResponseError { request, peer_id: Some(peer_id), error }.into()
)
Expand All @@ -278,7 +278,7 @@ where
{
// Every header must be valid on its own
if let Err(error) = self.consensus.validate_header(last_header) {
trace!(target: "downloaders::headers", ?error, "Failed to validate header");
trace!(target: "downloaders::headers", %error, "Failed to validate header");
return Err(HeadersResponseError {
request,
peer_id: Some(peer_id),
Expand All @@ -294,7 +294,7 @@ where
// detached head error.
if let Err(error) = self.consensus.validate_header_against_parent(last_header, head) {
// Replace the last header with a detached variant
error!(target: "downloaders::headers", ?error, number = last_header.number, hash = ?last_header.hash(), "Header cannot be attached to known canonical chain");
error!(target: "downloaders::headers", %error, number = last_header.number, hash = ?last_header.hash(), "Header cannot be attached to known canonical chain");
return Err(HeadersDownloaderError::DetachedHead {
local_head: Box::new(head.clone()),
header: Box::new(last_header.clone()),
Expand Down Expand Up @@ -529,7 +529,7 @@ where
fn penalize_peer(&self, peer_id: Option<PeerId>, error: &DownloadError) {
// Penalize the peer for bad response
if let Some(peer_id) = peer_id {
trace!(target: "downloaders::headers", ?peer_id, ?error, "Penalizing peer");
trace!(target: "downloaders::headers", ?peer_id, %error, "Penalizing peer");
self.client.report_bad_message(peer_id);
}
}
Expand Down Expand Up @@ -775,7 +775,7 @@ where
match this.on_sync_target_outcome(outcome) {
Ok(()) => break,
Err(ReverseHeadersDownloaderError::Response(error)) => {
trace!(target: "downloaders::headers", ?error, "invalid sync target response");
trace!(target: "downloaders::headers", %error, "invalid sync target response");
if error.is_channel_closed() {
// download channel closed which means the network was dropped
return Poll::Ready(None)
Expand Down
1 change: 0 additions & 1 deletion crates/net/eth-wire/src/errors/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use std::io;

/// Errors when sending/receiving messages
#[derive(thiserror::Error, Debug)]

pub enum EthStreamError {
#[error(transparent)]
/// Error of the underlying P2P connection.
Expand Down
Loading

0 comments on commit 94cb6a8

Please sign in to comment.