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

Tracker Checker: handle UDP Tracker timeouts #682

Closed
Tracked by #669 ...
josecelano opened this issue Feb 2, 2024 · 1 comment
Closed
Tracked by #669 ...

Tracker Checker: handle UDP Tracker timeouts #682

josecelano opened this issue Feb 2, 2024 · 1 comment
Assignees
Labels
- Developer - Torrust Improvement Experience Code Cleanup / Refactoring Tidying and Making Neat Testing Checking Torrust
Milestone

Comments

@josecelano
Copy link
Member

josecelano commented Feb 2, 2024

Parent issue: #677
Depends on: #681

You can run a Tracker Checker with:

TORRUST_CHECKER_CONFIG='{
    "udp_trackers": ["127.0.0.1:6969"],
    "http_trackers": [],
    "health_checks": []
}' cargo run --bin tracker_checker

The UDP Tracker client does have a timeout for sending and receiving responses. However, it panics when times out. See #681.

After implementing the issue #681 we should bubble the error up to this point:

(torrust_tracker::console::clients::udp::checker::Client)

pub struct Client {
    local_binding_address: Option<SocketAddr>,
    local_bound_address: Option<SocketAddr>,
    remote_socket: Option<SocketAddr>,
    udp_tracker_client: Option<UdpTrackerClient>,
}

impl Client {
    pub async fn bind_and_connect(&mut self, local_port: u16, remote_socket_addr: &SocketAddr) -> anyhow::Result<SocketAddr> {
        let bound_to = self.bind(local_port).await?;
        self.connect(remote_socket_addr).await?;
        Ok(bound_to)
    }

    async fn bind(&mut self, local_port: u16) -> anyhow::Result<SocketAddr> {
        let local_bind_to = format!("0.0.0.0:{local_port}");
        let binding_address = local_bind_to.parse().context("binding local address")?;

        debug!("Binding to: {local_bind_to}");
        let udp_client = UdpClient::bind(&local_bind_to).await;

        let bound_to = udp_client.socket.local_addr().context("bound local address")?;
        debug!("Bound to: {bound_to}");

        self.local_binding_address = Some(binding_address);
        self.local_bound_address = Some(bound_to);

        self.udp_tracker_client = Some(UdpTrackerClient { udp_client });

        Ok(bound_to)
    }

    async fn connect(&mut self, tracker_socket_addr: &SocketAddr) -> anyhow::Result<()> {
        debug!("Connecting to tracker: udp://{tracker_socket_addr}");

        match &self.udp_tracker_client {
            Some(client) => {
                client.udp_client.connect(&tracker_socket_addr.to_string()).await;
                self.remote_socket = Some(*tracker_socket_addr);
                Ok(())
            }
            None => Err(ClientError::NotBound.into()),
        }
    }

    pub async fn send_connection_request(&self, transaction_id: TransactionId) -> anyhow::Result<ConnectionId> {
        debug!("Sending connection request with transaction id: {transaction_id:#?}");

        let connect_request = ConnectRequest { transaction_id };

        match &self.udp_tracker_client {
            Some(client) => {
                client.send(connect_request.into()).await;

                let response = client.receive().await;

                debug!("connection request response:\n{response:#?}");

                match response {
                    Response::Connect(connect_response) => Ok(connect_response.connection_id),
                    _ => Err(ClientError::UnexpectedConnectionResponse.into()),
                }
            }
            None => Err(ClientError::NotConnected.into()),
        }
    }

    pub async fn send_announce_request(
        &self,
        connection_id: ConnectionId,
        transaction_id: TransactionId,
        info_hash: TorrustInfoHash,
        client_port: Port,
    ) -> anyhow::Result<Response> {
        debug!("Sending announce request with transaction id: {transaction_id:#?}");

        let announce_request = AnnounceRequest {
            connection_id,
            transaction_id,
            info_hash: InfoHash(info_hash.bytes()),
            peer_id: PeerId(*b"-qB00000000000000001"),
            bytes_downloaded: NumberOfBytes(0i64),
            bytes_uploaded: NumberOfBytes(0i64),
            bytes_left: NumberOfBytes(0i64),
            event: AnnounceEvent::Started,
            ip_address: Some(Ipv4Addr::new(0, 0, 0, 0)),
            key: PeerKey(0u32),
            peers_wanted: NumberOfPeers(1i32),
            port: client_port,
        };

        match &self.udp_tracker_client {
            Some(client) => {
                client.send(announce_request.into()).await;

                let response = client.receive().await;

                debug!("announce request response:\n{response:#?}");

                Ok(response)
            }
            None => Err(ClientError::NotConnected.into()),
        }
    }

    pub async fn send_scrape_request(
        &self,
        connection_id: ConnectionId,
        transaction_id: TransactionId,
        info_hashes: Vec<TorrustInfoHash>,
    ) -> anyhow::Result<Response> {
        debug!("Sending scrape request with transaction id: {transaction_id:#?}");

        let scrape_request = ScrapeRequest {
            connection_id,
            transaction_id,
            info_hashes: info_hashes
                .iter()
                .map(|torrust_info_hash| InfoHash(torrust_info_hash.bytes()))
                .collect(),
        };

        match &self.udp_tracker_client {
            Some(client) => {
                client.send(scrape_request.into()).await;

                let response = client.receive().await;

                debug!("scrape request response:\n{response:#?}");

                Ok(response)
            }
            None => Err(ClientError::NotConnected.into()),
        }
    }
}

We have to catch the error and add new types of errors:

#[derive(Error, Debug)]
pub enum ClientError {
    #[error("Local socket address is not bound yet. Try binding before connecting.")]
    NotBound,
    #[error("Not connected to remote tracker UDP socket. Try connecting before making requests.")]
    NotConnected,
    #[error("Unexpected response while connecting the the remote server.")]
    UnexpectedConnectionResponse,
}

At the top level the Tracker Checker should return a check error in the final report instead of panicking when a UDP client panics.

@josecelano josecelano added Code Cleanup / Refactoring Tidying and Making Neat - Developer - Torrust Improvement Experience Testing Checking Torrust labels Feb 2, 2024
@josecelano josecelano added this to the v3.1.0 milestone Feb 2, 2024
@da2ce7 da2ce7 self-assigned this Mar 26, 2024
hungfnt added a commit to hungfnt/torrust-tracker that referenced this issue May 3, 2024
@josecelano josecelano linked a pull request May 3, 2024 that will close this issue
hungfnt added a commit to hungfnt/torrust-tracker that referenced this issue May 11, 2024
@josecelano
Copy link
Member Author

This was implemented by @da2ce7. I've tested it with a "sleep" in the handler.

Announce:

2024-09-11T08:28:58.921545Z  INFO torrust_tracker::console::clients::checker::service: Running checks for trackers ...
[
  {
    "Udp": {
      "Err": {
        "remote_addr": "127.0.0.1:6969",
        "results": [
          [
            "Setup",
            {
              "Ok": null
            }
          ],
          [
            "Connect",
            {
              "Ok": null
            }
          ],
          [
            "Announce",
            {
              "Err": "Failed to receive a announce response, with error: Timeout while trying to receive data."
            }
          ],
          [
            "Announce",
            {
              "Ok": null
            }
          ]
        ]
      }
    }
  }
]

Scrape:

2024-09-11T08:31:26.362237Z  INFO torrust_tracker::console::clients::checker::service: Running checks for trackers ...
[
  {
    "Udp": {
      "Err": {
        "remote_addr": "127.0.0.1:6969",
        "results": [
          [
            "Setup",
            {
              "Ok": null
            }
          ],
          [
            "Connect",
            {
              "Ok": null
            }
          ],
          [
            "Announce",
            {
              "Ok": null
            }
          ],
          [
            "Announce",
            {
              "Err": "Failed to receive a scrape response, with error: Timeout while trying to receive data."
            }
          ]
        ]
      }
    }
  }
]

I just realized the operation name in the array is wrong. Announce is duplicated. The second one should be "Scrape".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- Developer - Torrust Improvement Experience Code Cleanup / Refactoring Tidying and Making Neat Testing Checking Torrust
Projects
Status: Done
2 participants