Repository: quic-go/quic-go
Pull request #4573 (2024-06-23 03:58:20)

http3: allow re-dialing of connection after a dial error

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/quic-go/quic-go/pull/4573?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) Report
Attention: Patch coverage is `50.00000%` with `1 line` in your changes missing coverage. Please review.
> Project coverage is 85.19%. Comparing base [(`4d0562d`)](https://app.codecov.io/gh/quic-go/quic-go/commit/4d0562d13c5a55ff04b05d1e4db1c67ce7159df4?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) to head [(`d2deffb`)](https://app.codecov.io/gh/quic-go/quic-go/commit/d2deffb8fa91b563ed51253b7a57bd44ad894bc8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).
> Report is 4 commits behind head on master.

> :exclamation: **Current head d2deffb differs from pull request most recent head 17839b6**
> 
> Please [upload](https://docs.codecov.com/docs/codecov-uploader) reports for the commit 17839b6 to get more accurate results.

| [Files](https://app.codecov.io/gh/quic-go/quic-go/pull/4573?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) | Patch % | Lines |
|---|---|---|
| [http3/roundtrip.go](https://app.codecov.io/gh/quic-go/quic-go/pull/4573?src=pr&el=tree&filepath=http3%2Froundtrip.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go#diff-aHR0cDMvcm91bmR0cmlwLmdv) | 50.00% | [1 Missing :warning: ](https://app.codecov.io/gh/quic-go/quic-go/pull/4573?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #4573      +/-   ##
==========================================
+ Coverage   85.14%   85.19%   +0.04%     
==========================================
  Files         154      154              
  Lines       14843    14959     +116     
==========================================
+ Hits        12638    12743     +105     
- Misses       1698     1710      +12     
+ Partials      507      506       -1     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/quic-go/quic-go/pull/4573?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).


Patch:
diff --git a/http3/roundtrip.go b/http3/roundtrip.go
index 148e3373..a9b169ee 100644
--- a/http3/roundtrip.go
+++ b/http3/roundtrip.go
@@ -166,6 +166,7 @@ func (r *RoundTripper) RoundTripOpt(req *http.Request, opt RoundTripOpt) (*http.
 	}
 
 	if cl.dialErr != nil {
+		r.removeClient(hostname)
 		return nil, cl.dialErr
 	}
 	defer cl.useCount.Add(-1)
@@ -258,6 +259,7 @@ func (r *RoundTripper) getClient(ctx context.Context, hostname string, onlyCache
 	select {
 	case <-cl.dialing:
 		if cl.dialErr != nil {
+			delete(r.clients, hostname)
 			return nil, false, cl.dialErr
 		}
 		select {


Test patch:
diff --git a/http3/roundtrip_test.go b/http3/roundtrip_test.go
index fb081abe..c8472e04 100644
--- a/http3/roundtrip_test.go
+++ b/http3/roundtrip_test.go
@@ -265,6 +265,37 @@ var _ = Describe("RoundTripper", func() {
 			Expect(count).To(Equal(1))
 		})
 
+		It("redials a connection if dialing failed", func() {
+			cl1 := NewMockSingleRoundTripper(mockCtrl)
+			clientChan <- cl1
+
+			req1, err := http.NewRequest("GET", "https://quic-go.net/foo.html", nil)
+			Expect(err).ToNot(HaveOccurred())
+			req2, err := http.NewRequest("GET", "https://quic-go.net/bar.html", nil)
+			Expect(err).ToNot(HaveOccurred())
+
+			testErr := errors.New("handshake error")
+			conn := mockquic.NewMockEarlyConnection(mockCtrl)
+			var count int
+			rt.Dial = func(context.Context, string, *tls.Config, *quic.Config) (quic.EarlyConnection, error) {
+				count++
+				if count == 1 {
+					return nil, testErr
+				}
+				return conn, nil
+			}
+			handshakeChan := make(chan struct{})
+			close(handshakeChan)
+			conn.EXPECT().HandshakeComplete().Return(handshakeChan).MaxTimes(2)
+			cl1.EXPECT().RoundTrip(req2).Return(&http.Response{Request: req2}, nil)
+			_, err = rt.RoundTrip(req1)
+			Expect(err).To(MatchError(testErr))
+			rsp, err := rt.RoundTrip(req2)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(rsp.Request).To(Equal(req2))
+			Expect(count).To(Equal(2))
+		})
+
 		It("immediately removes a clients when a request errored", func() {
 			cl1 := NewMockSingleRoundTripper(mockCtrl)
 			clientChan <- cl1
diff --git a/integrationtests/self/http_test.go b/integrationtests/self/http_test.go
index 0688a437..e2cf5fb2 100644
--- a/integrationtests/self/http_test.go
+++ b/integrationtests/self/http_test.go
@@ -150,6 +150,29 @@ var _ = Describe("HTTP tests", func() {
 		Expect(resp.Header.Get("Content-Length")).To(Equal("6"))
 	})
 
+	It("re-establishes a QUIC connection after a dial error", func() {
+		var dialCounter int
+		testErr := errors.New("test error")
+		cl := http.Client{
+			Transport: &http3.RoundTripper{
+				TLSClientConfig: getTLSClientConfig(),
+				Dial: func(ctx context.Context, addr string, tlsConf *tls.Config, conf *quic.Config) (quic.EarlyConnection, error) {
+					dialCounter++
+					if dialCounter == 1 { // make the first dial fail
+						return nil, testErr
+					}
+					return quic.DialAddrEarly(ctx, addr, tlsConf, conf)
+				},
+			},
+		}
+		defer cl.Transport.(io.Closer).Close()
+		_, err := cl.Get(fmt.Sprintf("https://localhost:%d/hello", port))
+		Expect(err).To(MatchError(testErr))
+		resp, err := cl.Get(fmt.Sprintf("https://localhost:%d/hello", port))
+		Expect(err).ToNot(HaveOccurred())
+		Expect(resp.StatusCode).To(Equal(http.StatusOK))
+	})
+
 	It("detects stream errors when server panics when writing response", func() {
 		respChan := make(chan struct{})
 		mux.HandleFunc("/writing_and_panicking", func(w http.ResponseWriter, r *http.Request) {