From f06e548a9554dfbd3e9d5ed9a17f3237e4528949 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dar=C3=ADo=20Kondratiuk?= Date: Fri, 19 Dec 2025 15:16:03 -0300 Subject: [PATCH 1/2] improve waits --- .../TestExpectations.local.json | 15 ---------- .../NavigationTests/PageGotoTests.cs | 8 ++--- lib/PuppeteerSharp/Bidi/BidiFrame.cs | 4 ++- lib/PuppeteerSharp/Cdp/LifecycleWatcher.cs | 5 +++- lib/PuppeteerSharp/Helpers/TaskHelper.cs | 30 +++++++++++++++++++ 5 files changed, 41 insertions(+), 21 deletions(-) diff --git a/lib/PuppeteerSharp.Nunit/TestExpectations/TestExpectations.local.json b/lib/PuppeteerSharp.Nunit/TestExpectations/TestExpectations.local.json index 35cd27cb8..9158ec170 100644 --- a/lib/PuppeteerSharp.Nunit/TestExpectations/TestExpectations.local.json +++ b/lib/PuppeteerSharp.Nunit/TestExpectations/TestExpectations.local.json @@ -228,21 +228,6 @@ "FAIL" ] }, - { - "comment": "This is part of organizing the webdriver bidi implementation, We will remove it one by one", - "testIdPattern": "[navigation.spec] *timeout*", - "platforms": [ - "darwin", - "linux", - "win32" - ], - "parameters": [ - "webDriverBiDi" - ], - "expectations": [ - "FAIL" - ] - }, { "comment": "This is part of organizing the webdriver bidi implementation, We will remove it one by one", "testIdPattern": "[ariaqueryhandler.spec] *", diff --git a/lib/PuppeteerSharp.Tests/NavigationTests/PageGotoTests.cs b/lib/PuppeteerSharp.Tests/NavigationTests/PageGotoTests.cs index ed7ea1006..194679e38 100644 --- a/lib/PuppeteerSharp.Tests/NavigationTests/PageGotoTests.cs +++ b/lib/PuppeteerSharp.Tests/NavigationTests/PageGotoTests.cs @@ -223,7 +223,7 @@ public void ShouldFailWhenExceedingMaximumNavigationTimeout() var exception = Assert.ThrowsAsync(async () => await Page.GoToAsync(TestConstants.EmptyPage, new NavigationOptions { Timeout = 1 })); - Assert.That(exception.Message, Does.Contain("Timeout of 1 ms exceeded")); + Assert.That(exception.Message, Does.Contain("Navigation timeout of 1 ms exceeded")); } [Test, PuppeteerTest("navigation.spec", "navigation Page.goto", "should fail when exceeding default maximum navigation timeout")] @@ -233,7 +233,7 @@ public void ShouldFailWhenExceedingDefaultMaximumNavigationTimeout() Page.DefaultNavigationTimeout = 1; var exception = Assert.ThrowsAsync(async () => await Page.GoToAsync(TestConstants.EmptyPage)); - Assert.That(exception.Message, Does.Contain("Timeout of 1 ms exceeded")); + Assert.That(exception.Message, Does.Contain("Navigation timeout of 1 ms exceeded")); } [Test, PuppeteerTest("navigation.spec", "navigation Page.goto", "should fail when exceeding default maximum timeout")] @@ -243,7 +243,7 @@ public void ShouldFailWhenExceedingDefaultMaximumTimeout() Server.SetRoute("/empty.html", _ => Task.Delay(-1)); Page.DefaultTimeout = 1; var exception = Assert.ThrowsAsync(async () => await Page.GoToAsync(TestConstants.EmptyPage)); - Assert.That(exception.Message, Does.Contain("Timeout of 1 ms exceeded")); + Assert.That(exception.Message, Does.Contain("Navigation timeout of 1 ms exceeded")); } [Test, PuppeteerTest("navigation.spec", "navigation Page.goto", "should prioritize default navigation timeout over default timeout")] @@ -254,7 +254,7 @@ public void ShouldPrioritizeDefaultNavigationTimeoutOverDefaultTimeout() Page.DefaultTimeout = 0; Page.DefaultNavigationTimeout = 1; var exception = Assert.ThrowsAsync(async () => await Page.GoToAsync(TestConstants.EmptyPage)); - Assert.That(exception.Message, Does.Contain("Timeout of 1 ms exceeded")); + Assert.That(exception.Message, Does.Contain("Navigation timeout of 1 ms exceeded")); } [Test, PuppeteerTest("navigation.spec", "navigation Page.goto", "should disable timeout when its set to 0")] diff --git a/lib/PuppeteerSharp/Bidi/BidiFrame.cs b/lib/PuppeteerSharp/Bidi/BidiFrame.cs index 037ea2f11..8704170a1 100644 --- a/lib/PuppeteerSharp/Bidi/BidiFrame.cs +++ b/lib/PuppeteerSharp/Bidi/BidiFrame.cs @@ -126,7 +126,9 @@ public override async Task GoToAsync(string url, NavigationOptions op try { - await Task.WhenAll(waitForNavigationTask, navigationTask).ConfigureAwait(false); + // - If any task fails/is canceled, immediately throw (don't wait for other tasks) + // - If all tasks succeed, return when all have completed + await new[] { waitForNavigationTask, navigationTask }.WhenAllFailFast().ConfigureAwait(false); } catch (NavigationException) { diff --git a/lib/PuppeteerSharp/Cdp/LifecycleWatcher.cs b/lib/PuppeteerSharp/Cdp/LifecycleWatcher.cs index 02a2ba0b6..d115f611b 100644 --- a/lib/PuppeteerSharp/Cdp/LifecycleWatcher.cs +++ b/lib/PuppeteerSharp/Cdp/LifecycleWatcher.cs @@ -62,7 +62,10 @@ public LifecycleWatcher( public CdpHttpResponse NavigationResponse => (CdpHttpResponse)_navigationRequest?.Response; - public Task TerminationTask => _terminationTaskWrapper.Task.WithTimeout(_timeout, cancellationToken: _terminationCancellationToken.Token); + public Task TerminationTask => _terminationTaskWrapper.Task.WithTimeout( + _timeout, + t => new TimeoutException($"Navigation timeout of {t.TotalMilliseconds} ms exceeded"), + _terminationCancellationToken.Token); public Task LifecycleTask => _lifecycleTaskWrapper.Task; diff --git a/lib/PuppeteerSharp/Helpers/TaskHelper.cs b/lib/PuppeteerSharp/Helpers/TaskHelper.cs index 7a6ecaa01..1f306d498 100644 --- a/lib/PuppeteerSharp/Helpers/TaskHelper.cs +++ b/lib/PuppeteerSharp/Helpers/TaskHelper.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; @@ -188,6 +189,35 @@ public static async Task WithTimeout(this Task task, TimeSpan timeout, return await task.ConfigureAwait(false); } + /// + /// Awaits all tasks, but fails immediately if any task fails (like JavaScript's Promise.all). + /// Unlike which waits for all tasks to complete before throwing, + /// this method throws immediately when any task fails or is canceled. + /// + /// The tasks to await. + /// A task that completes when all tasks complete successfully. + public static async Task WhenAllFailFast(this Task[] tasks) + { + if (tasks == null) + { + throw new ArgumentNullException(nameof(tasks)); + } + + var remaining = new HashSet(tasks); + + while (remaining.Count > 0) + { + var completed = await Task.WhenAny(remaining).ConfigureAwait(false); + remaining.Remove(completed); + + // If the task failed or was canceled, throw immediately (don't wait for others) + if (completed.IsFaulted || completed.IsCanceled) + { + await completed.ConfigureAwait(false); + } + } + } + private static async Task TimeoutTask(Task task, TimeSpan timeout) { if (timeout <= TimeSpan.Zero) From 5431354582e4ab00f7fa6aff7ba79360705a7610 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dar=C3=ADo=20Kondratiuk?= Date: Mon, 22 Dec 2025 10:14:12 -0300 Subject: [PATCH 2/2] fix some tests --- lib/PuppeteerSharp.Tests/PageTests/SetContentTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/PuppeteerSharp.Tests/PageTests/SetContentTests.cs b/lib/PuppeteerSharp.Tests/PageTests/SetContentTests.cs index efd10544e..d31c7dd9a 100644 --- a/lib/PuppeteerSharp.Tests/PageTests/SetContentTests.cs +++ b/lib/PuppeteerSharp.Tests/PageTests/SetContentTests.cs @@ -65,7 +65,7 @@ public async Task ShouldRespectTimeout() Timeout = 1 })); - Assert.That(exception!.Message, Does.Contain("Timeout of 1 ms exceeded")); + Assert.That(exception!.Message, Does.Contain("timeout of 1 ms exceeded")); } [Test, PuppeteerTest("page.spec", "Page Page.setContent", "should respect default navigation timeout")] @@ -79,7 +79,7 @@ public async Task ShouldRespectDefaultTimeout() var exception = Assert.ThrowsAsync(async () => await Page.SetContentAsync($"")); - Assert.That(exception!.Message, Does.Contain("Timeout of 1 ms exceeded")); + Assert.That(exception!.Message, Does.Contain("timeout of 1 ms exceeded")); } [Test, PuppeteerTest("page.spec", "Page Page.setContent", "should await resources to load")]