Honor socket timeout from connect in browser downloads

src/PlaywrightNative.Tests/Library/NetworkTimeout/NetworkTimeoutTests.cs

@@ -0,0 +1,114 @@
+/*
+ * Copyright (c) Microsoft Corporation.
+ * Modifications copyright (c) Microsoft Corporation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+using System;
+using System.Diagnostics;
+using System.Globalization;
+using System.Net;
+using System.Net.Http;
+using System.Net.Sockets;
+using System.Threading.Tasks;
+using NUnit.Framework;
+using PlaywrightNative.Helpers;
+using PlaywrightNative.NUnit;
+
+namespace PlaywrightNative.Tests
+{
+    /// <summary>
+    /// Official <c>library/network-timeout.spec.ts</c> parity. Upstream drives
+    /// the internal <c>utils.httpRequest</c>; the .NET twin is
+    /// <see cref="HttpRequestHelper"/>, which the browser fetcher uses.
+    /// File-level <c>mode !== 'default'</c> does not apply here.
+    /// </summary>
+    [TestFixture]
+    public class NetworkTimeoutTests : PlaywrightTestEx
+    {
+        [PlaywrightTest("network-timeout.spec.ts", "httpRequest should honor socketTimeout while connecting")]
+        [Test]
+        [Timeout(TestConstants.DefaultTestTimeout)]
+        public async Task HttpRequestShouldHonorSocketTimeoutWhileConnecting()
+        {
+            // 203.0.113.1 is TEST-NET-3 (RFC 5737), guaranteed to be unroutable, so the
+            // TCP connection stalls instead of failing. Make sure the requested timeout
+            // governs from the start.
+            const int socketTimeout = 8000;
+            Stopwatch stopwatch = Stopwatch.StartNew();
+            Exception error = await HttpRequestErrorAsync("http://203.0.113.1/index.html", socketTimeout).ConfigureAwait(false);
+            long elapsed = stopwatch.ElapsedMilliseconds;
+            if (!error.Message.Contains("timed out after", StringComparison.Ordinal))
+            {
+                // Some networks actively refuse TEST-NET addresses instead of blackholing them.
+                Assert.Ignore($"environment does not stall connections to TEST-NET: {error.Message}");
+            }
+
+            Assert.That(error.Message, Does.Contain($"timed out after {socketTimeout}ms"));
+            Assert.That(elapsed, Is.GreaterThanOrEqualTo(socketTimeout - 1000));
+        }
+
+        [PlaywrightTest("network-timeout.spec.ts", "httpRequest should honor socketTimeout for stalled responses")]
+        [Test]
+        [Timeout(TestConstants.DefaultTestTimeout)]
+        public async Task HttpRequestShouldHonorSocketTimeoutForStalledResponses()
+        {
+            // Accepts connections but never responds.
+            TcpListener server = new TcpListener(IPAddress.Loopback, 0);
+            server.Start();
+            int port = ((IPEndPoint)server.LocalEndpoint).Port;
+            const int socketTimeout = 1500;
+            Stopwatch stopwatch = Stopwatch.StartNew();
+            try
+            {
+                Exception error = await HttpRequestErrorAsync(
+                    "http://127.0.0.1:" + port.ToString(CultureInfo.InvariantCulture) + "/index.html",
+                    socketTimeout).ConfigureAwait(false);
+                long elapsed = stopwatch.ElapsedMilliseconds;
+                Assert.That(error.Message, Does.Contain($"timed out after {socketTimeout}ms"));
+                Assert.That(elapsed, Is.GreaterThanOrEqualTo(socketTimeout - 500));
+                Assert.That(elapsed, Is.LessThan(socketTimeout * 3));
+            }
+            finally
+            {
+                server.Stop();
+            }
+        }
+
+        [PlaywrightTest("network-timeout.spec.ts", "httpRequest should honor ipv4first result order")]
+        [Test]
+        [Timeout(TestConstants.DefaultTestTimeout)]
+        public void HttpRequestShouldHonorIpv4firstResultOrder()
+        {
+            Assert.Ignore("Node dns.promises.lookup / dns.setDefaultResultOrder hook");
+        }
+
+        private static async Task<Exception> HttpRequestErrorAsync(string url, int socketTimeout)
+        {
+            using HttpClient client = new HttpClient(HttpRequestHelper.CreateHandler(socketTimeout), disposeHandler: true)
+            {
+                Timeout = System.Threading.Timeout.InfiniteTimeSpan,
+            };
+            using HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Get, url);
+            try
+            {
+                using HttpResponseMessage response = await HttpRequestHelper.SendAsync(client, request, HttpCompletionOption.ResponseHeadersRead).ConfigureAwait(false);
+                return new InvalidOperationException("unexpected response");
+            }
+            catch (HttpRequestException ex)
+            {
+                return ex;
+            }
+        }
+    }
+}

src/PlaywrightNative.Tests/LibraryBrowserContextFetchHappyEyeballsParityTests.cs

@@ -25,10 +25,8 @@ namespace PlaywrightNative.Tests
 {
     /// <summary>
     /// Official <c>library/browsercontext-fetch-happy-eyeballs.spec.ts</c>
-    /// titles that do not need Node <c>__testHookLookup</c>. Skipped
-    /// (Node-only internals): <c>get should work</c>,
-    /// <c>get should work on request fixture</c>,
-    /// <c>https post should work with ignoreHTTPSErrors option</c>.
+    /// parity. Titles that drive the Node <c>__testHookLookup</c> DNS hook
+    /// are ignored.
     /// </summary>
     [TestFixture]
     [NonParallelizable]
@@ -79,6 +77,30 @@ public async Task StopOwnedServerAsync()
             }
         }
 
+        [PlaywrightTest("browsercontext-fetch-happy-eyeballs.spec.ts", "get should work")]
+        [Test]
+        [Timeout(TestConstants.DefaultTestTimeout)]
+        public void GetShouldWork()
+        {
+            Assert.Ignore("Node __testHookLookup DNS hook");
+        }
+
+        [PlaywrightTest("browsercontext-fetch-happy-eyeballs.spec.ts", "get should work on request fixture")]
+        [Test]
+        [Timeout(TestConstants.DefaultTestTimeout)]
+        public void GetShouldWorkOnRequestFixture()
+        {
+            Assert.Ignore("Node __testHookLookup DNS hook");
+        }
+
+        [PlaywrightTest("browsercontext-fetch-happy-eyeballs.spec.ts", "https post should work with ignoreHTTPSErrors option")]
+        [Test]
+        [Timeout(TestConstants.DefaultTestTimeout)]
+        public void HttpsPostShouldWorkWithIgnoreHTTPSErrorsOption()
+        {
+            Assert.Ignore("Node __testHookLookup DNS hook");
+        }
+
         [PlaywrightTest("browsercontext-fetch-happy-eyeballs.spec.ts", "should work with ip6 and port as the host")]
         [Test]
         [Timeout(TestConstants.DefaultTestTimeout)]

src/PlaywrightNative/BrowserFetcher.cs

@@ -11,9 +11,12 @@
 using System.Net;
 using System.Net.Http;
 using System.Runtime.InteropServices;
+using System.Security.Cryptography.X509Certificates;
+using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Extensions.Logging;
 using Microsoft.Playwright;
+using PlaywrightNative.Helpers;
 
 namespace PlaywrightNative
 {
@@ -92,7 +95,7 @@ public async Task<bool> CanDownloadAsync(string buildId)
                 try
                 {
                     using HttpRequestMessage request = new(HttpMethod.Head, url);
-                    using HttpResponseMessage response = await client.SendAsync(request).ConfigureAwait(false);
+                    using HttpResponseMessage response = await HttpRequestHelper.SendAsync(client, request, HttpCompletionOption.ResponseContentRead).ConfigureAwait(false);
                     if (response.IsSuccessStatusCode)
                     {
                         return true;
@@ -335,31 +338,27 @@ private HttpClient GetHttpClient()
 
         private HttpClient BuildHttpClient()
         {
-            HttpClientHandler handler = new()
-            {
-                CheckCertificateRevocationList = true,
-            };
+            // PLAYWRIGHT_DOWNLOAD_CONNECTION_TIMEOUT is a misnomer, it actually controls the socket's
+            // max idle timeout. Unfortunately, we cannot rename it without breaking existing user workflows.
+            string timeoutEnv = Environment.GetEnvironmentVariable(ConnectionTimeoutEnvVar);
+            int socketTimeout = !string.IsNullOrEmpty(timeoutEnv) && int.TryParse(timeoutEnv, out int timeoutMs) && timeoutMs > 0
+                ? timeoutMs
+                : HttpRequestHelper.NetDefaultTimeout;
+
+            SocketsHttpHandler handler = HttpRequestHelper.CreateHandler(socketTimeout);
+            handler.SslOptions.CertificateRevocationCheckMode = X509RevocationMode.Online;
 
             if (WebProxy != null)
             {
                 handler.Proxy = WebProxy;
                 handler.UseProxy = true;
             }
 
-            HttpClient client = new(handler, disposeHandler: true);
-
-            string timeoutEnv = Environment.GetEnvironmentVariable(ConnectionTimeoutEnvVar);
-            if (!string.IsNullOrEmpty(timeoutEnv) && int.TryParse(timeoutEnv, out int timeoutMs) && timeoutMs > 0)
+            // The socket idle timeout governs; large downloads must not hit a whole-request timeout.
+            return new HttpClient(handler, disposeHandler: true)
             {
-                client.Timeout = TimeSpan.FromMilliseconds(timeoutMs);
-            }
-            else
-            {
-                // Downloads are large; allow plenty of time by default.
-                client.Timeout = TimeSpan.FromMinutes(30);
-            }
-
-            return client;
+                Timeout = Timeout.InfiniteTimeSpan,
+            };
         }
 
         private string[] DownloadUrls(string buildId)
@@ -384,7 +383,8 @@ private async Task DownloadToFileAsync(string buildId, string zipPath, ILogger l
                     {
                         log?.LogInformation("Downloading {Browser} {BuildId} from {Url} (attempt {Attempt})", Browser, buildId, url, attempt + 1);
 
-                        using HttpResponseMessage response = await client.GetAsync(new Uri(url), HttpCompletionOption.ResponseHeadersRead).ConfigureAwait(false);
+                        using HttpRequestMessage request = new(HttpMethod.Get, url);
+                        using HttpResponseMessage response = await HttpRequestHelper.SendAsync(client, request, HttpCompletionOption.ResponseHeadersRead).ConfigureAwait(false);
                         if (!response.IsSuccessStatusCode)
                         {
                             int code = (int)response.StatusCode;