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

Close bad connection as notified by Heartbeat in P2P code #1567

Open
MitchTurner opened this issue Dec 19, 2023 · 3 comments
Open

Close bad connection as notified by Heartbeat in P2P code #1567

MitchTurner opened this issue Dec 19, 2023 · 3 comments
Assignees

Comments

@MitchTurner
Copy link
Member

Context

During poll our heartbeat::Handler previously would count failed connection and close the connection after a set number of failures.

            if self.failure_count >= self.config.max_failures.into() {
                // Request from `Swarm` to close the faulty connection
                return Poll::Ready(ConnectionHandlerEvent::Close(
                    HeartbeatFailure::Timeout,
                ))
            }

They have since removed the ability for handlers to close connections: libp2p/rust-libp2p#4755

We still want bad connections to be closed and reopened, so we need a new solution.

Requirements

  • Write a test that shows when connections with a peer fail more than the max_failures, it closes the connection.
  • Implement code to pass that test 👍
@AurelienFT
Copy link
Contributor

@MitchTurner Is it ok if the test just test that the handler send a notification to disconnect or we want ot test it at service level ? Because I'm having difficulties testing this at service level.

@MitchTurner
Copy link
Member Author

It might be the case that I was being overly prescriptive. It reads like typical TDD propaganda :). You have the context now to decide the best means of testing it, whether that is keeping the current arch or refactoring to a more testible design.

@AurelienFT
Copy link
Contributor

I always prefer to get with test first too and I prefer testing it at service level but I don't find a way to make the failure_count increase in tests at service level. There is too much process going on etc... so I wll propably end up doing it at handler level even if it's not my preference.

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

No branches or pull requests

2 participants