refactor(enginenetx): assign InitialDelay when dialing (#1555)

This diff refactors enginenetx to assign InitialDelay only when dialing.
It is pointless to do that before. Also, take advantage of algorithms
introduced by #1556 to make the
code more compact.

Part of ooni/probe#2704.

internal/enginenetx/bridgespolicy.go

@@ -27,33 +27,18 @@ var _ httpsDialerPolicy = &bridgesPolicy{}
 
 // LookupTactics implements httpsDialerPolicy.
 func (p *bridgesPolicy) LookupTactics(ctx context.Context, domain, port string) <-chan *httpsDialerTactic {
-	out := make(chan *httpsDialerTactic)
-
-	go func() {
-		defer close(out) // tell the parent when we're done
-		index := 0
-
+	return mixSequentially(
 		// emit bridges related tactics first which are empty if there are
 		// no bridges for the givend domain and port
-		for tx := range p.bridgesTacticsForDomain(domain, port) {
-			tx.InitialDelay = happyEyeballsDelay(index)
-			index += 1
-			out <- tx
-		}
+		p.bridgesTacticsForDomain(domain, port),
 
 		// now fallback to get more tactics (typically here the fallback
 		// uses the DNS and obtains some extra tactics)
 		//
 		// we wrap whatever the underlying policy returns us with some
 		// extra logic for better communicating with test helpers
-		for tx := range p.maybeRewriteTestHelpersTactics(p.Fallback.LookupTactics(ctx, domain, port)) {
-			tx.InitialDelay = happyEyeballsDelay(index)
-			index += 1
-			out <- tx
-		}
-	}()
-
-	return out
+		p.maybeRewriteTestHelpersTactics(p.Fallback.LookupTactics(ctx, domain, port)),
+	)
 }
 
 var bridgesPolicyTestHelpersDomains = []string{
@@ -92,7 +77,7 @@ func (p *bridgesPolicy) maybeRewriteTestHelpersTactics(input <-chan *httpsDialer
 			for _, sni := range p.bridgesDomainsInRandomOrder() {
 				out <- &httpsDialerTactic{
 					Address:        tactic.Address,
-					InitialDelay:   0,
+					InitialDelay:   0, // set when dialing
 					Port:           tactic.Port,
 					SNI:            sni,
 					VerifyHostname: tactic.VerifyHostname,
@@ -119,7 +104,7 @@ func (p *bridgesPolicy) bridgesTacticsForDomain(domain, port string) <-chan *htt
 			for _, sni := range p.bridgesDomainsInRandomOrder() {
 				out <- &httpsDialerTactic{
 					Address:        ipAddr,
-					InitialDelay:   0,
+					InitialDelay:   0, // set when dialing
 					Port:           port,
 					SNI:            sni,
 					VerifyHostname: domain,

internal/enginenetx/bridgespolicy_test.go

@@ -62,6 +62,10 @@ func TestBridgesPolicy(t *testing.T) {
 				t.Fatal("the host should always be 93.184.216.34")
 			}
 
+			if tactic.InitialDelay != 0 {
+				t.Fatal("unexpected InitialDelay")
+			}
+
 			if tactic.SNI != "www.example.com" {
 				t.Fatal("the SNI field should always be like `www.example.com`")
 			}
@@ -104,6 +108,10 @@ func TestBridgesPolicy(t *testing.T) {
 				t.Fatal("the host should always be 162.55.247.208")
 			}
 
+			if tactic.InitialDelay != 0 {
+				t.Fatal("unexpected InitialDelay")
+			}
+
 			if tactic.SNI == "api.ooni.io" {
 				t.Fatal("we should not see the `api.ooni.io` SNI on the wire")
 			}
@@ -173,6 +181,10 @@ func TestBridgesPolicy(t *testing.T) {
 				bridgesCount++
 			}
 
+			if tactic.InitialDelay != 0 {
+				t.Fatal("unexpected InitialDelay")
+			}
+
 			if tactic.VerifyHostname != "api.ooni.io" {
 				t.Fatal("the VerifyHostname field should always be like `api.ooni.io`")
 			}
@@ -206,27 +218,25 @@ func TestBridgesPolicy(t *testing.T) {
 				}
 
 				ctx := context.Background()
-				index := 0
-				for tactics := range p.LookupTactics(ctx, domain, "443") {
+				for tactic := range p.LookupTactics(ctx, domain, "443") {
 
-					if tactics.Address != "164.92.180.7" {
+					if tactic.Address != "164.92.180.7" {
 						t.Fatal("unexpected .Address")
 					}
 
-					if tactics.InitialDelay != happyEyeballsDelay(index) {
+					if tactic.InitialDelay != 0 {
 						t.Fatal("unexpected .InitialDelay")
 					}
-					index++
 
-					if tactics.Port != "443" {
+					if tactic.Port != "443" {
 						t.Fatal("unexpected .Port")
 					}
 
-					if tactics.SNI == domain {
+					if tactic.SNI == domain {
 						t.Fatal("unexpected .Domain")
 					}
 
-					if tactics.VerifyHostname != domain {
+					if tactic.VerifyHostname != domain {
 						t.Fatal("unexpected .VerifyHostname")
 					}
 				}

internal/enginenetx/dnspolicy.go

@@ -56,10 +56,10 @@ func (p *dnsPolicy) LookupTactics(
 		}
 
 		// The tactics we generate here have SNI == VerifyHostname == domain
-		for idx, addr := range addrs {
+		for _, addr := range addrs {
 			tactic := &httpsDialerTactic{
 				Address:        addr,
-				InitialDelay:   happyEyeballsDelay(idx),
+				InitialDelay:   0, // set when dialing
 				Port:           port,
 				SNI:            domain,
 				VerifyHostname: domain,

internal/enginenetx/dnspolicy_test.go

@@ -54,6 +54,9 @@ func TestDNSPolicy(t *testing.T) {
 			if tactic.Address != "130.192.91.211" {
 				t.Fatal("invalid endpoint address")
 			}
+			if tactic.InitialDelay != 0 {
+				t.Fatal("unexpected .InitialDelay")
+			}
 			if tactic.Port != "443" {
 				t.Fatal("invalid endpoint port")
 			}

internal/enginenetx/httpsdialer.go

@@ -209,7 +209,7 @@ func (hd *httpsDialer) DialTLSContext(ctx context.Context, network string, endpo
 
 	// The emitter will emit tactics and then close the channel when done. We spawn 16 workers
 	// that handle tactics in parallel and post results on the collector channel.
-	emitter := hd.policy.LookupTactics(ctx, hostname, port)
+	emitter := httpsDialerFilterTactics(hd.policy.LookupTactics(ctx, hostname, port))
 	collector := make(chan *httpsDialerErrorOrConn)
 	joiner := make(chan any)
 	const parallelism = 16
@@ -247,6 +247,20 @@ func (hd *httpsDialer) DialTLSContext(ctx context.Context, network string, endpo
 	return httpsDialerReduceResult(connv, errorv)
 }
 
+// httpsDialerFilterTactics filters the tactics to:
+//
+// 1. be paranoid and filter out nil tactics if any;
+//
+// 2. avoid emitting duplicate tactics as part of the same run;
+//
+// 3. rewrite the happy eyeball delays.
+//
+// This function returns a channel where we emit the edited
+// tactics, and which we clone when we're done.
+func httpsDialerFilterTactics(input <-chan *httpsDialerTactic) <-chan *httpsDialerTactic {
+	return filterAssignInitialDelays(filterOnlyKeepUniqueTactics(filterOutNilTactics(input)))
+}
+
 // httpsDialerReduceResult returns either an established conn or an error, using [errDNSNoAnswer] in
 // case the list of connections and the list of errors are empty.
 func httpsDialerReduceResult(connv []model.TLSConn, errorv []error) (model.TLSConn, error) {