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

fix: log an error when inbound fails #13

Open
wants to merge 1 commit into
base: mainnet-staging
Choose a base branch
from

Conversation

joske
Copy link

@joske joske commented Jun 7, 2024

Currently when self.inbound() fails, sometimes, nothing is logged, and you don't see that anything went wrong.

This tripped me up when diagnosing sync issues.

Currently when `inbound()` fails, nothing is logged, and you don't see
what went wrong
@joske joske requested a review from ljedrz June 7, 2024 07:46
@@ -117,6 +117,7 @@ impl<N: Network, C: ConsensusStorage<N>> Reading for Client<N, C> {
async fn process_message(&self, peer_addr: SocketAddr, message: Self::Message) -> io::Result<()> {
// Process the message. Disconnect if the peer violated the protocol.
if let Err(error) = self.inbound(peer_addr, message).await {
error!("Failed to process inbound message from '{peer_addr}' - {error}");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This log would be almost the same as the one displayed if the peer is still connected; I'm in favor of reporting the error regardless, but I think we may want to either remove the preexisting WARN or to change its message to no longer state the error (instead it could state sth like Disconnecting from '{peer_ip}' for protocol violation).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants