Skip to content

Commit 89d76ec

Browse files
ai-and-iclaudekixelated
authored
Fix error propagation when connection is closed (#181)
* Propagate session close error to all in-progress operations Share Arc<OnceLock<SessionError>> with SendStream, RecvStream, and SessionAccept so that when a CloseWebTransportSession capsule is received (or close() is called), in-progress stream operations return the meaningful session error instead of a generic LocallyClosed or HTTP/3 teardown code. Each stream type gets a map_error() method that replaces connection-level errors and invalid HTTP/3 codes (InvalidReset/InvalidStopped) with the stored session error. Session-level methods (accept, open, datagram) use the existing Session::map_error(), which is tightened to only replace connection-level errors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Applied cargo fmt * Address PR review comments - Change map_error to accept impl Into<Error> to reduce boilerplate - Only remap SessionError variants, not InvalidReset/InvalidStopped - Remove unwrap() panic in received_reset for invalid reset codes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Luke Curley <kixelated@gmail.com>
1 parent 913fec1 commit 89d76ec

3 files changed

Lines changed: 133 additions & 40 deletions

File tree

web-transport-quinn/src/recv.rs

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use std::{
22
io,
33
pin::Pin,
4+
sync::{Arc, OnceLock},
45
task::{Context, Poll},
56
};
67

@@ -12,11 +13,26 @@ use crate::{ReadError, ReadExactError, ReadToEndError, SessionError};
1213
#[derive(Debug)]
1314
pub struct RecvStream {
1415
inner: quinn::RecvStream,
16+
error: Arc<OnceLock<SessionError>>,
1517
}
1618

1719
impl RecvStream {
18-
pub(crate) fn new(stream: quinn::RecvStream) -> Self {
19-
Self { inner: stream }
20+
pub(crate) fn new(stream: quinn::RecvStream, error: Arc<OnceLock<SessionError>>) -> Self {
21+
Self {
22+
inner: stream,
23+
error,
24+
}
25+
}
26+
27+
/// Replace connection-level errors with the stored session error if available.
28+
fn map_error(&self, e: impl Into<ReadError>) -> ReadError {
29+
let e = e.into();
30+
if let Some(err) = self.error.get() {
31+
if matches!(&e, ReadError::SessionError(_)) {
32+
return ReadError::SessionError(err.clone());
33+
}
34+
}
35+
e
2036
}
2137

2238
/// Tell the other end to stop sending data with the given error code. See [`quinn::RecvStream::stop`].
@@ -31,12 +47,15 @@ impl RecvStream {
3147

3248
/// Read some data into the buffer and return the amount read. See [`quinn::RecvStream::read`].
3349
pub async fn read(&mut self, buf: &mut [u8]) -> Result<Option<usize>, ReadError> {
34-
self.inner.read(buf).await.map_err(Into::into)
50+
self.inner.read(buf).await.map_err(|e| self.map_error(e))
3551
}
3652

3753
/// Fill the entire buffer with data. See [`quinn::RecvStream::read_exact`].
3854
pub async fn read_exact(&mut self, buf: &mut [u8]) -> Result<(), ReadExactError> {
39-
self.inner.read_exact(buf).await.map_err(Into::into)
55+
self.inner.read_exact(buf).await.map_err(|e| match e {
56+
quinn::ReadExactError::ReadError(e) => self.map_error(e).into(),
57+
e => e.into(),
58+
})
4059
}
4160

4261
/// Read a chunk of data from the stream. See [`quinn::RecvStream::read_chunk`].
@@ -48,17 +67,26 @@ impl RecvStream {
4867
self.inner
4968
.read_chunk(max_length, ordered)
5069
.await
51-
.map_err(Into::into)
70+
.map_err(|e| self.map_error(e))
5271
}
5372

5473
/// Read chunks of data from the stream. See [`quinn::RecvStream::read_chunks`].
5574
pub async fn read_chunks(&mut self, bufs: &mut [Bytes]) -> Result<Option<usize>, ReadError> {
56-
self.inner.read_chunks(bufs).await.map_err(Into::into)
75+
self.inner
76+
.read_chunks(bufs)
77+
.await
78+
.map_err(|e| self.map_error(e))
5779
}
5880

5981
/// Read until the end of the stream or the limit is hit. See [`quinn::RecvStream::read_to_end`].
6082
pub async fn read_to_end(&mut self, size_limit: usize) -> Result<Vec<u8>, ReadToEndError> {
61-
self.inner.read_to_end(size_limit).await.map_err(Into::into)
83+
self.inner
84+
.read_to_end(size_limit)
85+
.await
86+
.map_err(|e| match e {
87+
quinn::ReadToEndError::Read(e) => self.map_error(e).into(),
88+
e => e.into(),
89+
})
6290
}
6391

6492
/// Block until the stream has been reset and return the error code. See [`quinn::RecvStream::received_reset`].
@@ -67,10 +95,10 @@ impl RecvStream {
6795
pub async fn received_reset(&mut self) -> Result<Option<u32>, SessionError> {
6896
match self.inner.received_reset().await {
6997
Ok(None) => Ok(None),
70-
Ok(Some(code)) => Ok(Some(
71-
web_transport_proto::error_from_http3(code.into_inner()).unwrap(),
72-
)),
73-
Err(quinn::ResetError::ConnectionLost(conn_err)) => Err(conn_err.into()),
98+
Ok(Some(code)) => Ok(web_transport_proto::error_from_http3(code.into_inner())),
99+
Err(quinn::ResetError::ConnectionLost(conn_err)) => {
100+
Err(self.error.get().cloned().unwrap_or_else(|| conn_err.into()))
101+
}
74102
Err(quinn::ResetError::ZeroRttRejected) => unreachable!("0-RTT not supported"),
75103
}
76104
}

web-transport-quinn/src/send.rs

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use std::{
22
io,
33
pin::Pin,
4+
sync::{Arc, OnceLock},
45
task::{Context, Poll},
56
};
67

@@ -15,11 +16,23 @@ use crate::{ClosedStream, SessionError, WriteError};
1516
#[derive(Debug)]
1617
pub struct SendStream {
1718
stream: quinn::SendStream,
19+
error: Arc<OnceLock<SessionError>>,
1820
}
1921

2022
impl SendStream {
21-
pub(crate) fn new(stream: quinn::SendStream) -> Self {
22-
Self { stream }
23+
pub(crate) fn new(stream: quinn::SendStream, error: Arc<OnceLock<SessionError>>) -> Self {
24+
Self { stream, error }
25+
}
26+
27+
/// Replace connection-level errors with the stored session error if available.
28+
fn map_error(&self, e: impl Into<WriteError>) -> WriteError {
29+
let e = e.into();
30+
if let Some(err) = self.error.get() {
31+
if matches!(&e, WriteError::SessionError(_)) {
32+
return WriteError::SessionError(err.clone());
33+
}
34+
}
35+
e
2336
}
2437

2538
/// Abruptly reset the stream with the provided error code. See [`quinn::SendStream::reset`].
@@ -38,7 +51,9 @@ impl SendStream {
3851
match self.stream.stopped().await {
3952
Ok(Some(code)) => Ok(web_transport_proto::error_from_http3(code.into_inner())),
4053
Ok(None) => Ok(None),
41-
Err(quinn::StoppedError::ConnectionLost(conn_err)) => Err(conn_err.into()),
54+
Err(quinn::StoppedError::ConnectionLost(conn_err)) => {
55+
Err(self.error.get().cloned().unwrap_or_else(|| conn_err.into()))
56+
}
4257
Err(quinn::StoppedError::ZeroRttRejected) => unreachable!("0-RTT not supported"),
4358
}
4459
}
@@ -47,27 +62,39 @@ impl SendStream {
4762

4863
/// Write some data to the stream, returning the size written. See [`quinn::SendStream::write`].
4964
pub async fn write(&mut self, buf: &[u8]) -> Result<usize, WriteError> {
50-
self.stream.write(buf).await.map_err(Into::into)
65+
self.stream.write(buf).await.map_err(|e| self.map_error(e))
5166
}
5267

5368
/// Write all of the data to the stream. See [`quinn::SendStream::write_all`].
5469
pub async fn write_all(&mut self, buf: &[u8]) -> Result<(), WriteError> {
55-
self.stream.write_all(buf).await.map_err(Into::into)
70+
self.stream
71+
.write_all(buf)
72+
.await
73+
.map_err(|e| self.map_error(e))
5674
}
5775

5876
/// Write chunks of data to the stream. See [`quinn::SendStream::write_chunks`].
5977
pub async fn write_chunks(&mut self, bufs: &mut [Bytes]) -> Result<quinn::Written, WriteError> {
60-
self.stream.write_chunks(bufs).await.map_err(Into::into)
78+
self.stream
79+
.write_chunks(bufs)
80+
.await
81+
.map_err(|e| self.map_error(e))
6182
}
6283

6384
/// Write a chunk of data to the stream. See [`quinn::SendStream::write_chunk`].
6485
pub async fn write_chunk(&mut self, buf: Bytes) -> Result<(), WriteError> {
65-
self.stream.write_chunk(buf).await.map_err(Into::into)
86+
self.stream
87+
.write_chunk(buf)
88+
.await
89+
.map_err(|e| self.map_error(e))
6690
}
6791

6892
/// Write all of the chunks of data to the stream. See [`quinn::SendStream::write_all_chunks`].
6993
pub async fn write_all_chunks(&mut self, bufs: &mut [Bytes]) -> Result<(), WriteError> {
70-
self.stream.write_all_chunks(bufs).await.map_err(Into::into)
94+
self.stream
95+
.write_all_chunks(bufs)
96+
.await
97+
.map_err(|e| self.map_error(e))
7198
}
7299

73100
/// Mark the stream as finished, such that no more data can be written. See [`quinn::SendStream::finish`].

web-transport-quinn/src/session.rs

Lines changed: 59 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ impl Session {
8181
let error: Arc<OnceLock<SessionError>> = Arc::new(OnceLock::new());
8282

8383
// Accept logic is stateful, so use an Arc<Mutex> to share it.
84-
let accept = SessionAccept::new(conn.clone(), session_id);
84+
let accept = SessionAccept::new(conn.clone(), session_id, error.clone());
8585

8686
let this = Self {
8787
conn,
@@ -186,50 +186,69 @@ impl Session {
186186
/// Accept a new unidirectional stream. See [`quinn::Connection::accept_uni`].
187187
pub async fn accept_uni(&self) -> Result<RecvStream, SessionError> {
188188
if let Some(accept) = &self.accept {
189-
Ok(poll_fn(|cx| accept.lock().unwrap().poll_accept_uni(cx)).await?)
189+
poll_fn(|cx| accept.lock().unwrap().poll_accept_uni(cx))
190+
.await
191+
.map_err(|e| self.map_error(e))
190192
} else {
191-
Ok(RecvStream::new(self.conn.accept_uni().await?))
193+
let recv = self
194+
.conn
195+
.accept_uni()
196+
.await
197+
.map_err(|e| self.map_error(e))?;
198+
Ok(RecvStream::new(recv, self.error.clone()))
192199
}
193200
}
194201

195202
/// Accept a new bidirectional stream. See [`quinn::Connection::accept_bi`].
196203
pub async fn accept_bi(&self) -> Result<(SendStream, RecvStream), SessionError> {
197204
if let Some(accept) = &self.accept {
198-
Ok(poll_fn(|cx| accept.lock().unwrap().poll_accept_bi(cx)).await?)
205+
poll_fn(|cx| accept.lock().unwrap().poll_accept_bi(cx))
206+
.await
207+
.map_err(|e| self.map_error(e))
199208
} else {
200-
let (send, recv) = self.conn.accept_bi().await?;
201-
Ok((SendStream::new(send), RecvStream::new(recv)))
209+
let (send, recv) = self.conn.accept_bi().await.map_err(|e| self.map_error(e))?;
210+
Ok((
211+
SendStream::new(send, self.error.clone()),
212+
RecvStream::new(recv, self.error.clone()),
213+
))
202214
}
203215
}
204216

205217
/// Open a new unidirectional stream. See [`quinn::Connection::open_uni`].
206218
pub async fn open_uni(&self) -> Result<SendStream, SessionError> {
207-
let mut send = self.conn.open_uni().await?;
219+
let mut send = self.conn.open_uni().await.map_err(|e| self.map_error(e))?;
208220

209221
// Set the stream priority to max and then write the stream header.
210222
// Otherwise the application could write data with lower priority than the header, resulting in queuing.
211223
// Also the header is very important for determining the session ID without reliable reset.
212224
send.set_priority(i32::MAX).ok();
213-
Self::write_full(&mut send, &self.header_uni).await?;
225+
Self::write_full(&mut send, &self.header_uni)
226+
.await
227+
.map_err(|e| self.map_error(e))?;
214228

215229
// Reset the stream priority back to the default of 0.
216230
send.set_priority(0).ok();
217-
Ok(SendStream::new(send))
231+
Ok(SendStream::new(send, self.error.clone()))
218232
}
219233

220234
/// Open a new bidirectional stream. See [`quinn::Connection::open_bi`].
221235
pub async fn open_bi(&self) -> Result<(SendStream, RecvStream), SessionError> {
222-
let (mut send, recv) = self.conn.open_bi().await?;
236+
let (mut send, recv) = self.conn.open_bi().await.map_err(|e| self.map_error(e))?;
223237

224238
// Set the stream priority to max and then write the stream header.
225239
// Otherwise the application could write data with lower priority than the header, resulting in queuing.
226240
// Also the header is very important for determining the session ID without reliable reset.
227241
send.set_priority(i32::MAX).ok();
228-
Self::write_full(&mut send, &self.header_bi).await?;
242+
Self::write_full(&mut send, &self.header_bi)
243+
.await
244+
.map_err(|e| self.map_error(e))?;
229245

230246
// Reset the stream priority back to the default of 0.
231247
send.set_priority(0).ok();
232-
Ok((SendStream::new(send), RecvStream::new(recv)))
248+
Ok((
249+
SendStream::new(send, self.error.clone()),
250+
RecvStream::new(recv, self.error.clone()),
251+
))
233252
}
234253

235254
/// Asynchronously receives an application datagram from the remote peer.
@@ -238,7 +257,11 @@ impl Session {
238257
/// peer over the connection.
239258
/// It waits for a datagram to become available and returns the received bytes.
240259
pub async fn read_datagram(&self) -> Result<Bytes, SessionError> {
241-
let mut datagram = self.conn.read_datagram().await?;
260+
let mut datagram = self
261+
.conn
262+
.read_datagram()
263+
.await
264+
.map_err(|e| self.map_error(e))?;
242265

243266
let mut cursor = Cursor::new(&datagram);
244267

@@ -276,7 +299,7 @@ impl Session {
276299
self.conn.send_datagram(data)
277300
};
278301

279-
result?;
302+
result.map_err(|e| self.map_error(e))?;
280303
Ok(())
281304
}
282305

@@ -397,12 +420,19 @@ impl Session {
397420
self.conn.close_reason().map(|e| self.map_error(e))
398421
}
399422

400-
/// Convert an error to `SessionError`, using the stored session error if set.
423+
/// Replace connection-level errors with the stored session error if available.
401424
fn map_error(&self, e: impl Into<SessionError>) -> SessionError {
425+
let e = e.into();
402426
if let Some(err) = self.error.get() {
403-
return err.clone();
427+
if matches!(
428+
&e,
429+
SessionError::ConnectionError(_)
430+
| SessionError::SendDatagramError(quinn::SendDatagramError::ConnectionLost(_))
431+
) {
432+
return err.clone();
433+
}
404434
}
405-
e.into()
435+
e
406436
}
407437

408438
async fn write_full(send: &mut quinn::SendStream, buf: &[u8]) -> Result<(), SessionError> {
@@ -488,6 +518,9 @@ type PendingBi = dyn Future<Output = Result<Option<(quinn::SendStream, quinn::Re
488518
pub struct SessionAccept {
489519
session_id: VarInt,
490520

521+
// Shared session error for propagation to accepted streams.
522+
error: Arc<OnceLock<SessionError>>,
523+
491524
// We also need to keep a reference to the qpack streams if the endpoint (incorrectly) creates them.
492525
// Again, this is just so they don't get closed until we drop the session.
493526
qpack_encoder: Option<quinn::RecvStream>,
@@ -508,7 +541,11 @@ pub struct SessionAccept {
508541
}
509542

510543
impl SessionAccept {
511-
pub(crate) fn new(conn: quinn::Connection, session_id: VarInt) -> Self {
544+
pub(crate) fn new(
545+
conn: quinn::Connection,
546+
session_id: VarInt,
547+
error: Arc<OnceLock<SessionError>>,
548+
) -> Self {
512549
// Create a stream that just outputs new streams, so it's easy to call from poll.
513550
let accept_uni = Box::pin(futures::stream::unfold(conn.clone(), |conn| async {
514551
Some((conn.accept_uni().await, conn))
@@ -520,6 +557,7 @@ impl SessionAccept {
520557

521558
Self {
522559
session_id,
560+
error,
523561

524562
qpack_decoder: None,
525563
qpack_encoder: None,
@@ -580,7 +618,7 @@ impl SessionAccept {
580618
// Decide if we keep looping based on the type.
581619
match typ {
582620
StreamUni::WEBTRANSPORT => {
583-
let recv = RecvStream::new(recv);
621+
let recv = RecvStream::new(recv, self.error.clone());
584622
for waker in self.uni_wakers.drain(..) {
585623
waker.wake();
586624
}
@@ -666,8 +704,8 @@ impl SessionAccept {
666704

667705
if let Some((send, recv)) = res {
668706
// Wrap the streams in our own types for correct error codes.
669-
let send = SendStream::new(send);
670-
let recv = RecvStream::new(recv);
707+
let send = SendStream::new(send, self.error.clone());
708+
let recv = RecvStream::new(recv, self.error.clone());
671709
for waker in self.bi_wakers.drain(..) {
672710
waker.wake();
673711
}

0 commit comments

Comments
 (0)