upnp: Panics when defaulting for Toggle

[dariusc93]

upnp: Panics when defaulting for Toggle

[jxs]

Hi Darius, and thanks for reporting this, nice catch.
The proposed solution is very elegant, the only downside would be that if someone can still poll the behaviour manually and then drop it right after triggering the panics. We could just address the panics themselves:

diff --git a/protocols/upnp/src/tokio.rs b/protocols/upnp/src/tokio.rs
index c6a40182b..9c8b2cafe 100644
--- a/protocols/upnp/src/tokio.rs
+++ b/protocols/upnp/src/tokio.rs
@@ -100,9 +100,7 @@ pub(crate) fn search_gateway() -> oneshot::Receiver<Result<Gateway, Box<dyn Erro
         let gateway = match igd_next::aio::tokio::search_gateway(SearchOptions::default()).await {
             Ok(gateway) => gateway,
             Err(err) => {
-                search_result_sender
-                    .send(Err(err.into()))
-                    .expect("receiver shouldn't have been dropped");
+                let _ = search_result_sender.send(Err(err.into()));
                 return;
             }
         };
@@ -110,20 +108,22 @@ pub(crate) fn search_gateway() -> oneshot::Receiver<Result<Gateway, Box<dyn Erro
         let external_addr = match gateway.get_external_ip().await {
             Ok(addr) => addr,
             Err(err) => {
-                search_result_sender
-                    .send(Err(err.into()))
-                    .expect("receiver shouldn't have been dropped");
+                let _ = search_result_sender.send(Err(err.into()));
                 return;
             }
         };
 
-        search_result_sender
+        // Check if receiver dropped.
+        if search_result_sender
             .send(Ok(Gateway {
                 sender: events_sender,
                 receiver: events_queue,
                 external_addr,
             }))
-            .expect("receiver shouldn't have been dropped");
+            .is_err()
+        {
+            return;
+        }
 
         loop {
             // The task sender has dropped so we can return.

wdyt?

[dariusc93]

Hi Darius, and thanks for reporting this, nice catch. The proposed solution is very elegant, the only downside would be that if someone can still poll the behaviour manually and then drop it right after triggering the panics. We could just address the panics themselves:

diff --git a/protocols/upnp/src/tokio.rs b/protocols/upnp/src/tokio.rs
index c6a40182b..9c8b2cafe 100644
--- a/protocols/upnp/src/tokio.rs
+++ b/protocols/upnp/src/tokio.rs
@@ -100,9 +100,7 @@ pub(crate) fn search_gateway() -> oneshot::Receiver<Result<Gateway, Box<dyn Erro
         let gateway = match igd_next::aio::tokio::search_gateway(SearchOptions::default()).await {
             Ok(gateway) => gateway,
             Err(err) => {
-                search_result_sender
-                    .send(Err(err.into()))
-                    .expect("receiver shouldn't have been dropped");
+                let _ = search_result_sender.send(Err(err.into()));
                 return;
             }
         };
@@ -110,20 +108,22 @@ pub(crate) fn search_gateway() -> oneshot::Receiver<Result<Gateway, Box<dyn Erro
         let external_addr = match gateway.get_external_ip().await {
             Ok(addr) => addr,
             Err(err) => {
-                search_result_sender
-                    .send(Err(err.into()))
-                    .expect("receiver shouldn't have been dropped");
+                let _ = search_result_sender.send(Err(err.into()));
                 return;
             }
         };
 
-        search_result_sender
+        // Check if receiver dropped.
+        if search_result_sender
             .send(Ok(Gateway {
                 sender: events_sender,
                 receiver: events_queue,
                 external_addr,
             }))
-            .expect("receiver shouldn't have been dropped");
+            .is_err()
+        {
+            return;
+        }
 
         loop {
             // The task sender has dropped so we can return.

wdyt?

Looks good to me :) Although i dont know of a case where one would poll it manually, however I do like the idea of addressing it directly