Skip to content

Commit f76661f

Browse files
committed
fix(client): race in connection errors propagation
Fix a race condition in the legacy HTTP client's connection setup where connection errors (e.g., TLS failures, unexpected server responses) were discarded, resulting in vague ChannelClosed errors. seanmonstar/reqwest#2649
1 parent 6c29abb commit f76661f

File tree

1 file changed

+31
-6
lines changed

1 file changed

+31
-6
lines changed

src/client/legacy/client.rs

+31-6
Original file line numberDiff line numberDiff line change
@@ -582,16 +582,41 @@ where
582582
trace!(
583583
"http1 handshake complete, spawning background dispatcher task"
584584
);
585+
let (err_tx, err_rx) = tokio::sync::oneshot::channel();
585586
executor.execute(
586587
conn.with_upgrades()
587-
.map_err(|e| debug!("client connection error: {}", e))
588+
.map_err(|e| {
589+
debug!("client connection error: {:?}", e);
590+
trace!("sending connection error to error channel");
591+
let _ =err_tx.send(e);
592+
})
588593
.map(|_| ()),
589594
);
590-
591-
// Wait for 'conn' to ready up before we
592-
// declare this tx as usable
593-
tx.ready().await.map_err(Error::tx)?;
594-
PoolTx::Http1(tx)
595+
trace!("waiting for connection to be ready");
596+
match tx.ready().await {
597+
Ok(_) => {
598+
trace!("connection is ready");
599+
drop(err_rx);
600+
PoolTx::Http1(tx)
601+
}
602+
Err(e) if e.is_closed() => {
603+
trace!("connection channel closed, checking for connection error");
604+
match err_rx.await {
605+
Ok(err) => {
606+
trace!("received connection error: {:?}", err);
607+
return Err(Error::tx(err));
608+
}
609+
Err(_) => {
610+
trace!("error channel closed, returning the vague ChannelClosed error");
611+
return Err(Error::tx(e));
612+
}
613+
}
614+
}
615+
Err(e) => {
616+
trace!("connection readiness failed: {:?}", e);
617+
return Err(Error::tx(e));
618+
}
619+
}
595620
}
596621
#[cfg(not(feature = "http1"))] {
597622
panic!("http1 feature is not enabled");

0 commit comments

Comments
 (0)