Introduce Page.SetDefaultTimeout (#938)

kblok · web-flow · commit 2e59b1eef049 · 2019-02-28T21:36:46.000-03:00
* Some progress

* Feature complete

* Tests fixes

* CodeFactor
diff --git a/lib/PuppeteerSharp.Tests/FrameTests/WaitForFunctionTests.cs b/lib/PuppeteerSharp.Tests/FrameTests/WaitForFunctionTests.cs
@@ -85,15 +85,11 @@ public async Task ShouldThrowNegativePollingInterval()
 
         [Fact]
         public async Task ShouldReturnTheSuccessValueAsAJSHandle()
-        {
-            Assert.Equal(5, await (await Page.WaitForFunctionAsync("() => 5")).JsonValueAsync<int>());
-        }
+            => Assert.Equal(5, await (await Page.WaitForFunctionAsync("() => 5")).JsonValueAsync<int>());
 
         [Fact]
         public async Task ShouldReturnTheWindowAsASuccessValue()
-        {
-            Assert.NotNull(await Page.WaitForFunctionAsync("() => window"));
-        }
+            => Assert.NotNull(await Page.WaitForFunctionAsync("() => window"));
 
         [Fact]
         public async Task ShouldAcceptElementHandleArguments()
@@ -117,6 +113,16 @@ public async Task ShouldRespectTimeout()
             Assert.Contains("waiting for function failed: timeout", exception.Message);
         }
 
+        [Fact]
+        public async Task ShouldRespectDefaultTimeout()
+        {
+            Page.DefaultTimeout = 1;
+            var exception = await Assert.ThrowsAsync<WaitTaskTimeoutException>(()
+                => Page.WaitForExpressionAsync("false"));
+
+            Assert.Contains("waiting for function failed: timeout", exception.Message);
+        }
+
         [Fact]
         public async Task ShouldDisableTimeoutWhenItsSetTo0()
         {
diff --git a/lib/PuppeteerSharp.Tests/PageTests/GotoTests.cs b/lib/PuppeteerSharp.Tests/PageTests/GotoTests.cs
@@ -139,6 +139,27 @@ public async Task ShouldFailWhenExceedingDefaultMaximumNavigationTimeout()
             Assert.Contains("Timeout Exceeded: 1ms", exception.Message);
         }
 
+        [Fact]
+        public async Task ShouldFailWhenExceedingDefaultMaximumTimeout()
+        {
+            // Hang for request to the empty.html
+            Server.SetRoute("/empty.html", context => Task.Delay(-1));
+            Page.DefaultTimeout = 1;
+            var exception = await Assert.ThrowsAnyAsync<Exception>(async () => await Page.GoToAsync(TestConstants.EmptyPage));
+            Assert.Contains("Timeout Exceeded: 1ms", exception.Message);
+        }
+
+        [Fact]
+        public async Task ShouldPrioritizeDefaultNavigationTimeoutOverDefaultTimeout()
+        {
+            // Hang for request to the empty.html
+            Server.SetRoute("/empty.html", context => Task.Delay(-1));
+            Page.DefaultTimeout = 0;
+            Page.DefaultNavigationTimeout = 1;
+            var exception = await Assert.ThrowsAnyAsync<Exception>(async () => await Page.GoToAsync(TestConstants.EmptyPage));
+            Assert.Contains("Timeout Exceeded: 1ms", exception.Message);
+        }
+
         [Fact]
         public async Task ShouldDisableTimeoutWhenItsSetTo0()
         {
diff --git a/lib/PuppeteerSharp.Tests/PageTests/SetContentTests.cs b/lib/PuppeteerSharp.Tests/PageTests/SetContentTests.cs
@@ -1,4 +1,5 @@
-﻿using System.Threading.Tasks;
+﻿using System;
+using System.Threading.Tasks;
 using Xunit;
 using Xunit.Abstractions;
 
@@ -45,6 +46,36 @@ public async Task ShouldWorkWithHtml4Doctype()
             Assert.Equal($"{doctype}{ExpectedOutput}", result);
         }
 
+        [Fact]
+        public async Task ShouldRespectTimeout()
+        {
+            const string imgPath = "/img.png";
+            Server.SetRoute(imgPath, context => Task.Delay(-1));
+
+            await Page.GoToAsync(TestConstants.EmptyPage);
+            var exception = await Assert.ThrowsAnyAsync<TimeoutException>(async () =>
+                await Page.SetContentAsync($"<img src='{TestConstants.ServerUrl + imgPath}'></img>", new NavigationOptions
+                {
+                    Timeout = 1
+                }));
+
+            Assert.Contains("Timeout Exceeded: 1ms", exception.Message);
+        }
+
+        [Fact]
+        public async Task ShouldRespectDefaultTimeout()
+        {
+            const string imgPath = "/img.png";
+            Server.SetRoute(imgPath, context => Task.Delay(-1));
+
+            await Page.GoToAsync(TestConstants.EmptyPage);
+            Page.DefaultTimeout = 1;
+            var exception = await Assert.ThrowsAnyAsync<TimeoutException>(async () =>
+                await Page.SetContentAsync($"<img src='{TestConstants.ServerUrl + imgPath}'></img>"));
+
+            Assert.Contains("Timeout Exceeded: 1ms", exception.Message);
+        }
+
         [Fact]
         public async Task ShouldAwaitResourcesToLoad()
         {
diff --git a/lib/PuppeteerSharp.Tests/PageTests/WaitForRequestTests.cs b/lib/PuppeteerSharp.Tests/PageTests/WaitForRequestTests.cs
@@ -1,4 +1,5 @@
-﻿using System.Net;
+﻿using System;
+using System.Net;
 using System.Threading.Tasks;
 using Xunit;
 using Xunit.Abstractions;
@@ -46,6 +47,30 @@ await Task.WhenAll(
             Assert.Equal(TestConstants.ServerUrl + "/digits/2.png", task.Result.Url);
         }
 
+        [Fact]
+        public async Task ShouldRespectTimeout()
+        {
+            await Page.GoToAsync(TestConstants.EmptyPage);
+            var exception = await Assert.ThrowsAnyAsync<TimeoutException>(async () =>
+                await Page.WaitForRequestAsync(request => false, new WaitForOptions
+                {
+                    Timeout = 1
+                }));
+
+            Assert.Contains("Timeout Exceeded: 1ms", exception.Message);
+        }
+
+        [Fact]
+        public async Task ShouldRespectDefaultTimeout()
+        {
+            await Page.GoToAsync(TestConstants.EmptyPage);
+            Page.DefaultTimeout = 1;
+            var exception = await Assert.ThrowsAnyAsync<TimeoutException>(async () =>
+                await Page.WaitForRequestAsync(request => false));
+
+            Assert.Contains("Timeout Exceeded: 1ms", exception.Message);
+        }
+
         [Fact]
         public async Task ShouldWorkWithNoTimeout()
         {
diff --git a/lib/PuppeteerSharp.Tests/PageTests/WaitForResponseTests.cs b/lib/PuppeteerSharp.Tests/PageTests/WaitForResponseTests.cs
@@ -1,4 +1,5 @@
-﻿using System.Net;
+﻿using System;
+using System.Net;
 using System.Threading.Tasks;
 using Xunit;
 using Xunit.Abstractions;
@@ -46,6 +47,30 @@ await Task.WhenAll(
             Assert.Equal(TestConstants.ServerUrl + "/digits/2.png", task.Result.Url);
         }
 
+        [Fact]
+        public async Task ShouldRespectTimeout()
+        {
+            await Page.GoToAsync(TestConstants.EmptyPage);
+            var exception = await Assert.ThrowsAnyAsync<TimeoutException>(async () =>
+                await Page.WaitForResponseAsync(request => false, new WaitForOptions
+                {
+                    Timeout = 1
+                }));
+
+            Assert.Contains("Timeout Exceeded: 1ms", exception.Message);
+        }
+
+        [Fact]
+        public async Task ShouldRespectDefaultTimeout()
+        {
+            await Page.GoToAsync(TestConstants.EmptyPage);
+            Page.DefaultTimeout = 1;
+            var exception = await Assert.ThrowsAnyAsync<TimeoutException>(async () =>
+                await Page.WaitForResponseAsync(request => false));
+
+            Assert.Contains("Timeout Exceeded: 1ms", exception.Message);
+        }
+
         [Fact]
         public async Task ShouldWorkWithNoTimeout()
         {
diff --git a/lib/PuppeteerSharp/DOMWorld.cs b/lib/PuppeteerSharp/DOMWorld.cs
@@ -10,17 +10,19 @@ namespace PuppeteerSharp
     internal class DOMWorld
     {
         private readonly FrameManager _frameManager;
+        private readonly TimeoutSettings _timeoutSettings;
         private bool _detached;
         private TaskCompletionSource<ExecutionContext> _contextResolveTaskWrapper;
         private TaskCompletionSource<ElementHandle> _documentCompletionSource;
 
         internal List<WaitTask> WaitTasks;
         internal Frame Frame { get; }
 
-        public DOMWorld(FrameManager frameManager, Frame frame)
+        public DOMWorld(FrameManager frameManager, Frame frame, TimeoutSettings timeoutSettings)
         {
             _frameManager = frameManager;
             Frame = frame;
+            _timeoutSettings = timeoutSettings;
 
             SetContext(null);
 
@@ -130,7 +132,7 @@ internal Task<string> GetContentAsync() => EvaluateFunctionAsync<string>(@"() =>
         internal async Task SetContentAsync(string html, NavigationOptions options = null)
         {
             var waitUntil = options?.WaitUntil ?? new[] { WaitUntilNavigation.Load };
-            var timeout = options?.Timeout ?? Puppeteer.DefaultTimeout;
+            var timeout = options?.Timeout ?? _timeoutSettings.NavigationTimeout;
 
             // We rely upon the fact that document.open() will reset frame lifecycle with "init"
             // lifecycle event. @see https://crrev.com/608658
@@ -348,10 +350,25 @@ internal Task<ElementHandle> WaitForXPathAsync(string xpath, WaitForSelectorOpti
             => WaitForSelectorOrXPathAsync(xpath, true, options);
 
         internal Task<JSHandle> WaitForFunctionAsync(string script, WaitForFunctionOptions options, params object[] args)
-            => new WaitTask(this, script, false, "function", options.Polling, options.PollingInterval, options.Timeout, args).Task;
+            => new WaitTask(
+                this,
+                script,
+                false,
+                "function",
+                options.Polling,
+                options.PollingInterval,
+                options.Timeout ?? _timeoutSettings.Timeout,
+                args).Task;
 
         internal Task<JSHandle> WaitForExpressionAsync(string script, WaitForFunctionOptions options)
-            => new WaitTask(this, script, true, "function", options.Polling, options.PollingInterval, options.Timeout).Task;
+            => new WaitTask(
+                this,
+                script,
+                true,
+                "function",
+                options.Polling,
+                options.PollingInterval,
+                options.Timeout ?? _timeoutSettings.Timeout).Task;
 
         internal Task<string> GetTitleAsync() => EvaluateExpressionAsync<string>("document.title");
 
diff --git a/lib/PuppeteerSharp/Frame.cs b/lib/PuppeteerSharp/Frame.cs
@@ -63,8 +63,8 @@ internal Frame(FrameManager frameManager, CDPSession client, Frame parentFrame,
             WaitTasks = new List<WaitTask>();
             LifecycleEvents = new List<string>();
 
-            MainWorld = new DOMWorld(FrameManager, this);
-            SecondaryWorld = new DOMWorld(FrameManager, this);
+            MainWorld = new DOMWorld(FrameManager, this, FrameManager.TimeoutSettings);
+            SecondaryWorld = new DOMWorld(FrameManager, this, FrameManager.TimeoutSettings);
         }
 
         #region Properties
diff --git a/lib/PuppeteerSharp/FrameManager.cs b/lib/PuppeteerSharp/FrameManager.cs
@@ -20,17 +20,17 @@ internal class FrameManager
         private const string RefererHeaderName = "referer";
         private readonly AsyncDictionaryHelper<string, Frame> _asyncFrames;
         private readonly List<string> _isolatedWorlds = new List<string>();
-
         private const string UtilityWorldName = "__puppeteer_utility_world__";
 
-        private FrameManager(CDPSession client, Page page, NetworkManager networkManager)
+        private FrameManager(CDPSession client, Page page, NetworkManager networkManager, TimeoutSettings timeoutSettings)
         {
             Client = client;
             Page = page;
             _frames = new ConcurrentDictionary<string, Frame>();
             _contextIdToContext = new Dictionary<int, ExecutionContext>();
             _logger = Client.Connection.LoggerFactory.CreateLogger<FrameManager>();
             NetworkManager = networkManager;
+            TimeoutSettings = timeoutSettings;
             _asyncFrames = new AsyncDictionaryHelper<string, Frame>(_frames, "Frame {0} not found");
 
             Client.MessageReceived += Client_MessageReceived;
@@ -48,14 +48,18 @@ private FrameManager(CDPSession client, Page page, NetworkManager networkManager
         internal NetworkManager NetworkManager { get; }
         internal Frame MainFrame { get; set; }
         internal Page Page { get; }
-        internal int DefaultNavigationTimeout { get; set; } = 30000;
-
+        internal TimeoutSettings TimeoutSettings { get; }
         #endregion
 
         #region Public Methods
-        internal static async Task<FrameManager> CreateFrameManagerAsync(CDPSession client, Page page, NetworkManager networkManager, FrameTree frameTree)
+        internal static async Task<FrameManager> CreateFrameManagerAsync(
+            CDPSession client,
+            Page page,
+            NetworkManager networkManager,
+            FrameTree frameTree,
+            TimeoutSettings timeoutSettings)
         {
-            var frameManager = new FrameManager(client, page, networkManager);
+            var frameManager = new FrameManager(client, page, networkManager, timeoutSettings);
             await frameManager.HandleFrameTreeAsync(frameTree).ConfigureAwait(false);
             return frameManager;
         }
@@ -77,7 +81,7 @@ public async Task<Response> NavigateFrameAsync(Frame frame, string url, Navigati
                ? NetworkManager.ExtraHTTPHeaders?.GetValueOrDefault(RefererHeaderName)
                : options.Referer;
             var requests = new Dictionary<string, Request>();
-            var timeout = options?.Timeout ?? DefaultNavigationTimeout;
+            var timeout = options?.Timeout ?? TimeoutSettings.NavigationTimeout;
 
             using (var watcher = new LifecycleWatcher(this, frame, options?.WaitUntil, timeout))
             {
@@ -125,7 +129,7 @@ private async Task NavigateAsync(CDPSession client, string url, string referrer,
 
         public async Task<Response> WaitForFrameNavigationAsync(Frame frame, NavigationOptions options = null)
         {
-            var timeout = options?.Timeout ?? DefaultNavigationTimeout;
+            var timeout = options?.Timeout ?? TimeoutSettings.NavigationTimeout;
             using (var watcher = new LifecycleWatcher(this, frame, options?.WaitUntil, timeout))
             {
                 var raceTask = await Task.WhenAny(
diff --git a/lib/PuppeteerSharp/NavigationOptions.cs b/lib/PuppeteerSharp/NavigationOptions.cs
@@ -9,7 +9,7 @@ public class NavigationOptions
         /// Maximum navigation time in milliseconds, defaults to 30 seconds, pass <c>0</c> to disable timeout. 
         /// </summary>
         /// <remarks>
-        /// The default value can be changed by setting the <see cref="Page.DefaultNavigationTimeout"/> property.
+        /// The default value can be changed by setting the <see cref="Page.DefaultNavigationTimeout"/> or <see cref="Page.DefaultTimeout"/> property.
         /// </remarks>
         public int? Timeout { get; set; }
 
diff --git a/lib/PuppeteerSharp/Page.cs b/lib/PuppeteerSharp/Page.cs
diff --git a/lib/PuppeteerSharp/TimeoutSettings.cs b/lib/PuppeteerSharp/TimeoutSettings.cs
diff --git a/lib/PuppeteerSharp/WaitForFunctionOptions.cs b/lib/PuppeteerSharp/WaitForFunctionOptions.cs
diff --git a/lib/PuppeteerSharp/WaitForOptions.cs b/lib/PuppeteerSharp/WaitForOptions.cs

Original file line number	Diff line number	Diff line change
`@@ -63,8 +63,8 @@ internal Frame(FrameManager frameManager, CDPSession client, Frame parentFrame,`
`63`	`63`	`WaitTasks = new List<WaitTask>();`
`64`	`64`	`LifecycleEvents = new List<string>();`
`65`	`65`
`66`		`- MainWorld = new DOMWorld(FrameManager, this);`
`67`		`- SecondaryWorld = new DOMWorld(FrameManager, this);`
	`66`	`+ MainWorld = new DOMWorld(FrameManager, this, FrameManager.TimeoutSettings);`
	`67`	`+ SecondaryWorld = new DOMWorld(FrameManager, this, FrameManager.TimeoutSettings);`
`68`	`68`	`}`
`69`	`69`
`70`	`70`	`#region Properties`