From 02b9573315ef139af0df6c69de7d37f6d2f4ea55 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Thu, 9 Jan 2025 12:29:06 +0200
Subject: [PATCH 1/4] chore: Enable the `unused_result_ok` lint

This also exposed an unneeded `Result` being returned from `stream_fairness`,
which this PR also removes.
---
 Cargo.toml                                    |  1 +
 neqo-http3/src/connection.rs                  | 17 ++++-------------
 neqo-http3/src/connection_client.rs           | 12 ++----------
 neqo-transport/src/connection/mod.rs          |  9 +++------
 neqo-transport/src/connection/tests/stream.rs |  4 ++--
 neqo-transport/src/send_stream.rs             | 14 ++++++--------
 neqo-transport/src/streams.rs                 |  6 ++----
 neqo-transport/tests/connection.rs            |  4 ++--
 8 files changed, 22 insertions(+), 45 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 7899277091..c676bf52d4 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -45,6 +45,7 @@ pedantic = { level = "warn", priority = -1 }
 if_then_some_else_none = "warn"
 get_unwrap = "warn"
 pathbuf_init_then_push = "warn"
+unused_result_ok = "warn"
 
 # Optimize build dependencies, because bindgen and proc macros / style
 # compilation take more to run than to build otherwise.
diff --git a/neqo-http3/src/connection.rs b/neqo-http3/src/connection.rs
index a59b6908e7..37ef1ccbee 100644
--- a/neqo-http3/src/connection.rs
+++ b/neqo-http3/src/connection.rs
@@ -738,7 +738,7 @@ impl Http3Connection {
                     return Ok(ReceiveOutput::NoOutput);
                 }
                 // set incoming WebTransport streams to be fair (share bandwidth)
-                conn.stream_fairness(stream_id, true).ok();
+                conn.stream_fairness(stream_id, true);
                 qinfo!(
                     [self],
                     "A new WebTransport stream {} for session {}.",
@@ -1024,17 +1024,8 @@ impl Http3Connection {
     /// Set the stream Fairness.   Fair streams will share bandwidth with other
     /// streams of the same sendOrder group (or the unordered group).  Unfair streams
     /// will give bandwidth preferentially to the lowest streamId with data to send.
-    ///
-    /// # Errors
-    ///
-    /// Returns `InvalidStreamId` if the stream id doesn't exist
-    pub fn stream_set_fairness(
-        conn: &mut Connection,
-        stream_id: StreamId,
-        fairness: bool,
-    ) -> Res<()> {
-        conn.stream_fairness(stream_id, fairness)
-            .map_err(|_| Error::InvalidStreamId)
+    pub fn stream_set_fairness(conn: &mut Connection, stream_id: StreamId, fairness: bool) {
+        conn.stream_fairness(stream_id, fairness);
     }
 
     pub fn cancel_fetch(
@@ -1284,7 +1275,7 @@ impl Http3Connection {
             .map_err(|e| Error::map_stream_create_errors(&e))?;
         // Set outgoing WebTransport streams to be fair (share bandwidth)
         // This really can't fail, panics if it does
-        conn.stream_fairness(stream_id, true).unwrap();
+        conn.stream_fairness(stream_id, true);
 
         self.webtransport_create_stream_internal(
             wt,
diff --git a/neqo-http3/src/connection_client.rs b/neqo-http3/src/connection_client.rs
index b9f7db52a4..2dc25c008b 100644
--- a/neqo-http3/src/connection_client.rs
+++ b/neqo-http3/src/connection_client.rs
@@ -808,16 +808,8 @@ impl Http3Client {
     }
 
     /// Sets the `Fairness` for a given stream
-    ///
-    /// # Errors
-    ///
-    /// It may return `InvalidStreamId` if a stream does not exist anymore.
-    ///
-    /// # Panics
-    ///
-    /// This cannot panic.
-    pub fn webtransport_set_fairness(&mut self, stream_id: StreamId, fairness: bool) -> Res<()> {
-        Http3Connection::stream_set_fairness(&mut self.conn, stream_id, fairness)
+    pub fn webtransport_set_fairness(&mut self, stream_id: StreamId, fairness: bool) {
+        Http3Connection::stream_set_fairness(&mut self.conn, stream_id, fairness);
     }
 
     /// Returns the current `SendStreamStats` of a `WebTransportSendStream`.
diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs
index 8ef0ad8e55..763db54efd 100644
--- a/neqo-transport/src/connection/mod.rs
+++ b/neqo-transport/src/connection/mod.rs
@@ -1559,7 +1559,7 @@ impl Connection {
         );
         path.borrow_mut().add_received(d.len());
         let res = self.input_path(&path, d, received);
-        self.capture_error(Some(path), now, 0, res).ok();
+        _ = self.capture_error(Some(path), now, 0, res);
     }
 
     fn input_path(
@@ -3331,11 +3331,8 @@ impl Connection {
     }
 
     /// Set the Fairness of a stream
-    ///
-    /// # Errors
-    /// When the stream does not exist.
-    pub fn stream_fairness(&mut self, stream_id: StreamId, fairness: bool) -> Res<()> {
-        self.streams.set_fairness(stream_id, fairness)
+    pub fn stream_fairness(&mut self, stream_id: StreamId, fairness: bool) {
+        self.streams.set_fairness(stream_id, fairness);
     }
 
     /// # Errors
diff --git a/neqo-transport/src/connection/tests/stream.rs b/neqo-transport/src/connection/tests/stream.rs
index 89e1773dae..a74c12d4d0 100644
--- a/neqo-transport/src/connection/tests/stream.rs
+++ b/neqo-transport/src/connection/tests/stream.rs
@@ -131,8 +131,8 @@ fn sendorder_test(order_of_sendorder: &[Option<SendOrder>]) {
         streams.push(id);
         ordered.push((id, *sendorder));
         // must be set before sendorder
-        client.streams.set_fairness(id, true).ok();
-        client.streams.set_sendorder(id, *sendorder).ok();
+        client.streams.set_fairness(id, true);
+        client.streams.set_sendorder(id, *sendorder).unwrap();
     }
     // Write some data to all the streams
     for stream_id in streams {
diff --git a/neqo-transport/src/send_stream.rs b/neqo-transport/src/send_stream.rs
index 323215cd85..31308ab655 100644
--- a/neqo-transport/src/send_stream.rs
+++ b/neqo-transport/src/send_stream.rs
@@ -1565,10 +1565,8 @@ impl SendStreams {
         }
     }
 
-    #[allow(clippy::missing_panics_doc)]
-    #[allow(clippy::missing_errors_doc)]
     pub fn set_sendorder(&mut self, stream_id: StreamId, sendorder: Option<SendOrder>) -> Res<()> {
-        self.set_fairness(stream_id, true)?;
+        self.set_fairness(stream_id, true);
         if let Some(stream) = self.map.get_mut(&stream_id) {
             // don't grab stream here; causes borrow errors
             let old_sendorder = stream.sendorder();
@@ -1591,10 +1589,11 @@ impl SendStreams {
         }
     }
 
-    #[allow(clippy::missing_panics_doc)]
-    #[allow(clippy::missing_errors_doc)]
-    pub fn set_fairness(&mut self, stream_id: StreamId, make_fair: bool) -> Res<()> {
-        let stream: &mut SendStream = self.map.get_mut(&stream_id).ok_or(Error::InvalidStreamId)?;
+    pub fn set_fairness(&mut self, stream_id: StreamId, make_fair: bool) {
+        let Some(stream) = self.map.get_mut(&stream_id) else {
+            // We can get called with an invalid stream ID, and that is OK.
+            return;
+        };
         let was_fair = stream.fair;
         stream.set_fairness(make_fair);
         if !was_fair && make_fair {
@@ -1626,7 +1625,6 @@ impl SendStreams {
             };
             group.remove(stream_id);
         }
-        Ok(())
     }
 
     pub fn acked(&mut self, token: &SendStreamRecoveryToken) {
diff --git a/neqo-transport/src/streams.rs b/neqo-transport/src/streams.rs
index 059607be08..df6ef5e15d 100644
--- a/neqo-transport/src/streams.rs
+++ b/neqo-transport/src/streams.rs
@@ -429,10 +429,8 @@ impl Streams {
         self.send.set_sendorder(stream_id, sendorder)
     }
 
-    /// # Errors
-    /// When the stream does not exist.
-    pub fn set_fairness(&mut self, stream_id: StreamId, fairness: bool) -> Res<()> {
-        self.send.set_fairness(stream_id, fairness)
+    pub fn set_fairness(&mut self, stream_id: StreamId, fairness: bool) {
+        self.send.set_fairness(stream_id, fairness);
     }
 
     /// # Errors
diff --git a/neqo-transport/tests/connection.rs b/neqo-transport/tests/connection.rs
index fcbcf228e5..12b4829732 100644
--- a/neqo-transport/tests/connection.rs
+++ b/neqo-transport/tests/connection.rs
@@ -283,8 +283,8 @@ fn handshake_mlkem768x25519() {
 
     client
         .set_groups(&[neqo_crypto::TLS_GRP_KEM_MLKEM768X25519])
-        .ok();
-    client.send_additional_key_shares(0).ok();
+        .unwrap();
+    client.send_additional_key_shares(0).unwrap();
 
     test_fixture::handshake(&mut client, &mut server);
     assert_eq!(*client.state(), State::Confirmed);

From 8661a97fdf9012f3e1116637e6713068f373ce52 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Thu, 9 Jan 2025 15:17:28 +0200
Subject: [PATCH 2/4] Restore error handling around `stream_fairness`

---
 neqo-http3/src/connection.rs                  | 23 +++++++++++++++----
 neqo-http3/src/connection_client.rs           |  8 +++++--
 neqo-transport/src/connection/mod.rs          |  7 ++++--
 neqo-transport/src/connection/tests/stream.rs |  2 +-
 neqo-transport/src/send_stream.rs             | 10 ++++----
 neqo-transport/src/streams.rs                 |  6 +++--
 6 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/neqo-http3/src/connection.rs b/neqo-http3/src/connection.rs
index 37ef1ccbee..ab4c1ad5e5 100644
--- a/neqo-http3/src/connection.rs
+++ b/neqo-http3/src/connection.rs
@@ -737,8 +737,12 @@ impl Http3Connection {
                     conn.stream_stop_sending(stream_id, Error::HttpStreamCreation.code())?;
                     return Ok(ReceiveOutput::NoOutput);
                 }
-                // set incoming WebTransport streams to be fair (share bandwidth)
-                conn.stream_fairness(stream_id, true);
+                // Set incoming WebTransport streams to be fair (share bandwidth).
+                // We may call this with an invalid stream ID, so ignore that error.
+                match conn.stream_fairness(stream_id, true) {
+                    Ok(()) | Err(neqo_transport::Error::InvalidStreamId) => (),
+                    Err(e) => return Err(Error::from(e)),
+                };
                 qinfo!(
                     [self],
                     "A new WebTransport stream {} for session {}.",
@@ -1024,8 +1028,17 @@ impl Http3Connection {
     /// Set the stream Fairness.   Fair streams will share bandwidth with other
     /// streams of the same sendOrder group (or the unordered group).  Unfair streams
     /// will give bandwidth preferentially to the lowest streamId with data to send.
-    pub fn stream_set_fairness(conn: &mut Connection, stream_id: StreamId, fairness: bool) {
-        conn.stream_fairness(stream_id, fairness);
+    ///
+    /// # Errors
+    ///
+    /// Returns `InvalidStreamId` if the stream id doesn't exist
+    pub fn stream_set_fairness(
+        conn: &mut Connection,
+        stream_id: StreamId,
+        fairness: bool,
+    ) -> Res<()> {
+        conn.stream_fairness(stream_id, fairness)
+            .map_err(|_| Error::InvalidStreamId)
     }
 
     pub fn cancel_fetch(
@@ -1275,7 +1288,7 @@ impl Http3Connection {
             .map_err(|e| Error::map_stream_create_errors(&e))?;
         // Set outgoing WebTransport streams to be fair (share bandwidth)
         // This really can't fail, panics if it does
-        conn.stream_fairness(stream_id, true);
+        conn.stream_fairness(stream_id, true)?;
 
         self.webtransport_create_stream_internal(
             wt,
diff --git a/neqo-http3/src/connection_client.rs b/neqo-http3/src/connection_client.rs
index 2dc25c008b..e922450911 100644
--- a/neqo-http3/src/connection_client.rs
+++ b/neqo-http3/src/connection_client.rs
@@ -808,8 +808,12 @@ impl Http3Client {
     }
 
     /// Sets the `Fairness` for a given stream
-    pub fn webtransport_set_fairness(&mut self, stream_id: StreamId, fairness: bool) {
-        Http3Connection::stream_set_fairness(&mut self.conn, stream_id, fairness);
+    ///
+    /// # Errors
+    ///
+    /// It may return `InvalidStreamId` if a stream does not exist anymore.
+    pub fn webtransport_set_fairness(&mut self, stream_id: StreamId, fairness: bool) -> Res<()> {
+        Http3Connection::stream_set_fairness(&mut self.conn, stream_id, fairness)
     }
 
     /// Returns the current `SendStreamStats` of a `WebTransportSendStream`.
diff --git a/neqo-transport/src/connection/mod.rs b/neqo-transport/src/connection/mod.rs
index 763db54efd..cc0267e6ca 100644
--- a/neqo-transport/src/connection/mod.rs
+++ b/neqo-transport/src/connection/mod.rs
@@ -3331,8 +3331,11 @@ impl Connection {
     }
 
     /// Set the Fairness of a stream
-    pub fn stream_fairness(&mut self, stream_id: StreamId, fairness: bool) {
-        self.streams.set_fairness(stream_id, fairness);
+    ///
+    /// # Errors
+    /// When the stream does not exist.
+    pub fn stream_fairness(&mut self, stream_id: StreamId, fairness: bool) -> Res<()> {
+        self.streams.set_fairness(stream_id, fairness)
     }
 
     /// # Errors
diff --git a/neqo-transport/src/connection/tests/stream.rs b/neqo-transport/src/connection/tests/stream.rs
index a74c12d4d0..a92d3be9c2 100644
--- a/neqo-transport/src/connection/tests/stream.rs
+++ b/neqo-transport/src/connection/tests/stream.rs
@@ -131,7 +131,7 @@ fn sendorder_test(order_of_sendorder: &[Option<SendOrder>]) {
         streams.push(id);
         ordered.push((id, *sendorder));
         // must be set before sendorder
-        client.streams.set_fairness(id, true);
+        client.streams.set_fairness(id, true).unwrap();
         client.streams.set_sendorder(id, *sendorder).unwrap();
     }
     // Write some data to all the streams
diff --git a/neqo-transport/src/send_stream.rs b/neqo-transport/src/send_stream.rs
index 31308ab655..0543f53015 100644
--- a/neqo-transport/src/send_stream.rs
+++ b/neqo-transport/src/send_stream.rs
@@ -1566,7 +1566,7 @@ impl SendStreams {
     }
 
     pub fn set_sendorder(&mut self, stream_id: StreamId, sendorder: Option<SendOrder>) -> Res<()> {
-        self.set_fairness(stream_id, true);
+        self.set_fairness(stream_id, true)?;
         if let Some(stream) = self.map.get_mut(&stream_id) {
             // don't grab stream here; causes borrow errors
             let old_sendorder = stream.sendorder();
@@ -1589,11 +1589,8 @@ impl SendStreams {
         }
     }
 
-    pub fn set_fairness(&mut self, stream_id: StreamId, make_fair: bool) {
-        let Some(stream) = self.map.get_mut(&stream_id) else {
-            // We can get called with an invalid stream ID, and that is OK.
-            return;
-        };
+    pub fn set_fairness(&mut self, stream_id: StreamId, make_fair: bool) -> Res<()> {
+        let stream: &mut SendStream = self.map.get_mut(&stream_id).ok_or(Error::InvalidStreamId)?;
         let was_fair = stream.fair;
         stream.set_fairness(make_fair);
         if !was_fair && make_fair {
@@ -1625,6 +1622,7 @@ impl SendStreams {
             };
             group.remove(stream_id);
         }
+        Ok(())
     }
 
     pub fn acked(&mut self, token: &SendStreamRecoveryToken) {
diff --git a/neqo-transport/src/streams.rs b/neqo-transport/src/streams.rs
index df6ef5e15d..059607be08 100644
--- a/neqo-transport/src/streams.rs
+++ b/neqo-transport/src/streams.rs
@@ -429,8 +429,10 @@ impl Streams {
         self.send.set_sendorder(stream_id, sendorder)
     }
 
-    pub fn set_fairness(&mut self, stream_id: StreamId, fairness: bool) {
-        self.send.set_fairness(stream_id, fairness);
+    /// # Errors
+    /// When the stream does not exist.
+    pub fn set_fairness(&mut self, stream_id: StreamId, fairness: bool) -> Res<()> {
+        self.send.set_fairness(stream_id, fairness)
     }
 
     /// # Errors

From 1bf9fb882b66b25badb5616d624e3824f9f99e40 Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Thu, 9 Jan 2025 18:20:31 +0200
Subject: [PATCH 3/4] Fix

---
 neqo-transport/src/send_stream.rs | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/neqo-transport/src/send_stream.rs b/neqo-transport/src/send_stream.rs
index 0543f53015..323215cd85 100644
--- a/neqo-transport/src/send_stream.rs
+++ b/neqo-transport/src/send_stream.rs
@@ -1565,6 +1565,8 @@ impl SendStreams {
         }
     }
 
+    #[allow(clippy::missing_panics_doc)]
+    #[allow(clippy::missing_errors_doc)]
     pub fn set_sendorder(&mut self, stream_id: StreamId, sendorder: Option<SendOrder>) -> Res<()> {
         self.set_fairness(stream_id, true)?;
         if let Some(stream) = self.map.get_mut(&stream_id) {
@@ -1589,6 +1591,8 @@ impl SendStreams {
         }
     }
 
+    #[allow(clippy::missing_panics_doc)]
+    #[allow(clippy::missing_errors_doc)]
     pub fn set_fairness(&mut self, stream_id: StreamId, make_fair: bool) -> Res<()> {
         let stream: &mut SendStream = self.map.get_mut(&stream_id).ok_or(Error::InvalidStreamId)?;
         let was_fair = stream.fair;

From f59c3150c481919ad238285aef4e12e84bec0eaf Mon Sep 17 00:00:00 2001
From: Lars Eggert <lars@eggert.org>
Date: Fri, 10 Jan 2025 11:06:34 +0200
Subject: [PATCH 4/4] Remove old comment

---
 neqo-http3/src/connection.rs | 1 -
 1 file changed, 1 deletion(-)

diff --git a/neqo-http3/src/connection.rs b/neqo-http3/src/connection.rs
index d851a3e264..011cae82b1 100644
--- a/neqo-http3/src/connection.rs
+++ b/neqo-http3/src/connection.rs
@@ -1287,7 +1287,6 @@ impl Http3Connection {
             .stream_create(stream_type)
             .map_err(|e| Error::map_stream_create_errors(&e))?;
         // Set outgoing WebTransport streams to be fair (share bandwidth)
-        // This really can't fail, panics if it does
         conn.stream_fairness(stream_id, true)?;
 
         self.webtransport_create_stream_internal(