refactor(webconnectivitylte): use NewHTTPTransportWithOptions (#1494)

There's no need to use the older NewHTTPTransport factory for creating a new HTTP transport, because this codebase doesn't need to use any quirk implemented by such a transport. While there, move TODOs around the codebase. Part of ooni/probe#2669.
ooni · Feb 8, 2024 · 49382fe · 49382fe
1 parent 7a71438
commit 49382fe
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 18 deletions.
diff --git a/internal/experiment/webconnectivitylte/cleartextflow.go b/internal/experiment/webconnectivitylte/cleartextflow.go
@@ -105,10 +105,6 @@ func (t *CleartextFlow) Run(parentCtx context.Context, index int64) error {
 	trace := measurexlite.NewTrace(index, t.ZeroTime, fmt.Sprintf("depth=%d", t.Depth),
 		fmt.Sprintf("fetch_body=%v", t.PrioSelector != nil))
 
-	// TODO(bassosimone): the DSL starts measuring for throttling when we start
-	// fetching the body while here we start immediately. We should come up with
-	// a consistent policy otherwise data won't be comparable.
-
 	// start measuring throttling
 	sampler := throttling.NewSampler(trace)
 	defer func() {
@@ -144,10 +140,7 @@ func (t *CleartextFlow) Run(parentCtx context.Context, index int64) error {
 	}
 
 	// create HTTP transport
-	// TODO(https://github.com/ooni/probe/issues/2534): here we're using the QUIRKY netxlite.NewHTTPTransport
-	// function, but we can probably avoid using it, given that this code is
-	// not using tracing and does not care about those quirks.
-	httpTransport := netxlite.NewHTTPTransport(
+	httpTransport := netxlite.NewHTTPTransportWithOptions(
 		t.Logger,
 		netxlite.NewSingleUseDialer(tcpConn),
 		netxlite.NewNullTLSDialer(),
@@ -188,7 +181,7 @@ func (t *CleartextFlow) Run(parentCtx context.Context, index int64) error {
 	// if enabled, follow possible redirects
 	t.maybeFollowRedirects(parentCtx, httpResp)
 
-	// TODO: insert here additional code if needed
+	// ignore the response body
 	_ = httpRespBody
 
 	// completed successfully

diff --git a/internal/experiment/webconnectivitylte/secureflow.go b/internal/experiment/webconnectivitylte/secureflow.go
@@ -112,10 +112,6 @@ func (t *SecureFlow) Run(parentCtx context.Context, index int64) error {
 	trace := measurexlite.NewTrace(index, t.ZeroTime, fmt.Sprintf("depth=%d", t.Depth),
 		fmt.Sprintf("fetch_body=%v", t.PrioSelector != nil))
 
-	// TODO(bassosimone): the DSL starts measuring for throttling when we start
-	// fetching the body while here we start immediately. We should come up with
-	// a consistent policy otherwise data won't be comparable.
-
 	// start measuring throttling
 	sampler := throttling.NewSampler(trace)
 	defer func() {
@@ -179,10 +175,7 @@ func (t *SecureFlow) Run(parentCtx context.Context, index int64) error {
 	}
 
 	// create HTTP transport
-	// TODO(https://github.com/ooni/probe/issues/2534): here we're using the QUIRKY netxlite.NewHTTPTransport
-	// function, but we can probably avoid using it, given that this code is
-	// not using tracing and does not care about those quirks.
-	httpTransport := netxlite.NewHTTPTransport(
+	httpTransport := netxlite.NewHTTPTransportWithOptions(
 		t.Logger,
 		netxlite.NewNullDialer(),
 		netxlite.NewSingleUseTLSDialer(tlsConn),
@@ -223,7 +216,7 @@ func (t *SecureFlow) Run(parentCtx context.Context, index int64) error {
 	// if enabled, follow possible redirects
 	t.maybeFollowRedirects(parentCtx, httpResp)
 
-	// TODO: insert here additional code if needed
+	// ignore the response body
 	_ = httpRespBody
 
 	// completed successfully

diff --git a/internal/x/dslvm/quic.go b/internal/x/dslvm/quic.go
@@ -158,6 +158,10 @@ func (sx *QUICHandshakeStage) handshake(ctx context.Context, rtx Runtime, endpoi
 		return
 	}
 
+	// TODO(https://github.com/ooni/probe/issues/2670).
+	//
+	// Start measuring for throttling here.
+
 	// handle success
 	sx.Output <- &QUICConnection{Conn: quicConn, tx: trace, tlsConfig: config}
 }

diff --git a/internal/x/dslvm/tcp.go b/internal/x/dslvm/tcp.go
@@ -143,6 +143,10 @@ func (sx *TCPConnectStage) connect(ctx context.Context, rtx Runtime, endpoint st
 		return
 	}
 
+	// TODO(https://github.com/ooni/probe/issues/2670).
+	//
+	// Start measuring for throttling here.
+
 	// handle success
 	sx.Output <- &TCPConnection{Conn: conn, tx: trace}
 }
diff --git a/internal/x/dslx/quic.go b/internal/x/dslx/quic.go
@@ -48,6 +48,10 @@ func QUICHandshake(rt Runtime, options ...TLSHandshakeOption) Func[*Endpoint, *Q
 		if quicConn != nil {
 			closerConn = &quicCloserConn{quicConn}
 			tlsState = quicConn.ConnectionState().TLS // only quicConn can be nil
+
+			// TODO(https://github.com/ooni/probe/issues/2670).
+			//
+			// Start measuring for throttling here.
 		}
 
 		// possibly track established conn for late close

diff --git a/internal/x/dslx/tcp.go b/internal/x/dslx/tcp.go
@@ -51,6 +51,10 @@ func TCPConnect(rt Runtime) Func[*Endpoint, *TCPConnection] {
 			return nil, err
 		}
 
+		// TODO(https://github.com/ooni/probe/issues/2670).
+		//
+		// Start measuring for throttling here.
+
 		// handle success
 		state := &TCPConnection{
 			Address: input.Address,