Skip to content

Commit 7a5b155

Browse files
kblokkblok
authored
Browser closing/disconnecting should abort navigations (#699)
1 parent c51fab4 commit 7a5b155

File tree

6 files changed

+115
-31
lines changed

6 files changed

+115
-31
lines changed

‎lib/PuppeteerSharp.Tests/BrowserTests/Events/DisconnectedTests.cs‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,5 +47,50 @@ await Task.WhenAll(
4747
Assert.Equal(1, disconnectedRemote1);
4848
Assert.Equal(1, disconnectedRemote2);
4949
}
50+
51+
[Fact]
52+
public async Task ShouldRejectNavigationWhenBrowserCloses()
53+
{
54+
Server.SetRoute("/one-style.css", context => Task.Delay(10000));
55+
56+
using (var browser = await Puppeteer.LaunchAsync(TestConstants.DefaultBrowserOptions()))
57+
{
58+
var remote = await Puppeteer.ConnectAsync(new ConnectOptions
59+
{
60+
BrowserWSEndpoint = browser.WebSocketEndpoint
61+
});
62+
var page = await remote.NewPageAsync();
63+
var navigationTask = page.GoToAsync(TestConstants.ServerUrl + "/one-style.html", new NavigationOptions
64+
{
65+
Timeout = 60000
66+
});
67+
await Server.WaitForRequest("/one-style.css");
68+
remote.Disconnect();
69+
var exception = await Assert.ThrowsAsync<NavigationException>(() => navigationTask);
70+
Assert.Equal("Navigation failed because browser has disconnected!", exception.Message);
71+
}
72+
}
73+
74+
[Fact]
75+
public async Task ShouldRejectWaitForSelectorWhenBrowserCloses()
76+
{
77+
Server.SetRoute("/empty.html", context => Task.Delay(10000));
78+
79+
using (var browser = await Puppeteer.LaunchAsync(TestConstants.DefaultBrowserOptions()))
80+
{
81+
var remote = await Puppeteer.ConnectAsync(new ConnectOptions
82+
{
83+
BrowserWSEndpoint = browser.WebSocketEndpoint
84+
});
85+
var page = await remote.NewPageAsync();
86+
var watchdog = page.WaitForSelectorAsync("div", new WaitForSelectorOptions { Timeout = 60000 });
87+
remote.Disconnect();
88+
var exception = await Assert.ThrowsAsync<EvaluationFailedException>(() => watchdog);
89+
//Using the type instead of the message because the exception could come
90+
//Whether from the Connection rejecting a message from the CDPSession
91+
//or from the CDPSession trying to send a message to a closed connection
92+
Assert.IsType<TargetClosedException>(exception.InnerException);
93+
}
94+
}
5095
}
5196
}

‎lib/PuppeteerSharp/CDPSession.cs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,10 @@ internal CDPSession(IConnection connection, TargetType targetType, string sessio
8282
/// </summary>
8383
public event EventHandler<TracingCompleteEventArgs> TracingComplete;
8484
/// <summary>
85+
/// Occurs when the connection is closed.
86+
/// </summary>
87+
public event EventHandler Closed;
88+
/// <summary>
8589
/// Gets or sets a value indicating whether this <see cref="CDPSession"/> is closed.
8690
/// </summary>
8791
/// <value><c>true</c> if is closed; otherwise, <c>false</c>.</value>
@@ -280,7 +284,7 @@ internal void OnClosed()
280284
));
281285
}
282286
_callbacks.Clear();
283-
287+
Closed?.Invoke(this, EventArgs.Empty);
284288
Connection = null;
285289
}
286290

@@ -297,6 +301,7 @@ internal CDPSession CreateSession(TargetType targetType, string sessionId)
297301
bool IConnection.IsClosed => IsClosed;
298302
Task<JObject> IConnection.SendAsync(string method, dynamic args, bool waitForCallback)
299303
=> SendAsync(method, args, waitForCallback);
304+
IConnection IConnection.Connection => Connection;
300305
#endregion
301306
}
302307
}

‎lib/PuppeteerSharp/Connection.cs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,15 @@ private void OnClose()
166166
_callbacks.Clear();
167167
}
168168

169+
internal static IConnection FromSession(CDPSession session)
170+
{
171+
var connection = session.Connection;
172+
while (connection is CDPSession)
173+
{
174+
connection = connection.Connection;
175+
}
176+
return connection;
177+
}
169178
#region Private Methods
170179

171180
/// <summary>
@@ -342,6 +351,7 @@ public void Dispose()
342351
bool IConnection.IsClosed => IsClosed;
343352
Task<JObject> IConnection.SendAsync(string method, dynamic args, bool waitForCallback)
344353
=> SendAsync(method, args, waitForCallback);
354+
IConnection IConnection.Connection => null;
345355
#endregion
346356
}
347357
}

‎lib/PuppeteerSharp/IConnection.cs‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
using System.Threading.Tasks;
1+
using System;
2+
using System.Threading.Tasks;
23
using Microsoft.Extensions.Logging;
34
using Newtonsoft.Json.Linq;
45

@@ -24,11 +25,19 @@ internal interface IConnection
2425
/// </summary>
2526
/// <returns>The async.</returns>
2627
/// <param name="method">Method to call.</param>
27-
/// <param name="args">Method arguments.</param>
28+
/// <param name="args">Method arguments.</param>
2829
/// <param name="waitForCallback">
2930
/// If <c>true</c> the method will return a task to be completed when the message is confirmed by Chromium.
3031
/// If <c>false</c> the task will be considered complete after sending the message to Chromium.
3132
/// </param>
32-
Task<JObject> SendAsync(string method, dynamic args = null, bool waitForCallback = true);
33+
Task<JObject> SendAsync(string method, dynamic args = null, bool waitForCallback = true);
34+
/// <summary>
35+
/// Gets the parent connection
36+
/// </summary>
37+
IConnection Connection { get; }
38+
/// <summary>
39+
/// Occurs when the connection is closed.
40+
/// </summary>
41+
event EventHandler Closed;
3342
}
3443
}

‎lib/PuppeteerSharp/NavigatorWatcher.cs‎

Lines changed: 28 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
using System.Collections.Generic;
2-
using System.Threading.Tasks;
3-
using System.Linq;
4-
using System.Diagnostics.Contracts;
5-
using PuppeteerSharp.Helpers;
6-
2+
using System.Threading.Tasks;
3+
using System.Linq;
4+
using System.Diagnostics.Contracts;
5+
using PuppeteerSharp.Helpers;
6+
77
namespace PuppeteerSharp
88
{
99
internal class NavigatorWatcher
@@ -24,9 +24,14 @@ internal class NavigatorWatcher
2424
private readonly int _timeout;
2525
private readonly string _initialLoaderId;
2626
private Request _navigationRequest;
27-
private bool _hasSameDocumentNavigation;
27+
private bool _hasSameDocumentNavigation;
28+
private TaskCompletionSource<bool> _newDocumentNavigationTaskWrapper;
29+
private TaskCompletionSource<bool> _sameDocumentNavigationTaskWrapper;
30+
private TaskCompletionSource<bool> _terminationTaskWrapper;
31+
private Task _timeoutTask;
2832

2933
public NavigatorWatcher(
34+
CDPSession client,
3035
FrameManager frameManager,
3136
Frame mainFrame,
3237
NetworkManager networkManager,
@@ -58,20 +63,22 @@ public NavigatorWatcher(
5863
frameManager.FrameNavigatedWithinDocument += NavigatedWithinDocument;
5964
frameManager.FrameDetached += CheckLifecycleComplete;
6065
networkManager.Request += OnRequest;
66+
Connection.FromSession(client).Closed += (sender, e)
67+
=> Terminate(new TargetClosedException("Navigation failed because browser has disconnected!"));
68+
69+
_sameDocumentNavigationTaskWrapper = new TaskCompletionSource<bool>();
70+
_newDocumentNavigationTaskWrapper = new TaskCompletionSource<bool>();
71+
_terminationTaskWrapper = new TaskCompletionSource<bool>();
72+
_timeoutTask = TaskHelper.CreateTimeoutTask(timeout);
73+
}
6174

62-
SameDocumentNavigationTaskWrapper = new TaskCompletionSource<bool>();
63-
NewDocumentNavigationTaskWrapper = new TaskCompletionSource<bool>();
64-
TimeoutTask = TaskHelper.CreateTimeoutTask(timeout);
65-
}
66-
6775
#region Properties
6876
public Task<Task> NavigationTask { get; internal set; }
69-
public Task<bool> SameDocumentNavigationTask => SameDocumentNavigationTaskWrapper.Task;
70-
public TaskCompletionSource<bool> SameDocumentNavigationTaskWrapper { get; }
71-
public Task<bool> NewDocumentNavigationTask => NewDocumentNavigationTaskWrapper.Task;
72-
public TaskCompletionSource<bool> NewDocumentNavigationTaskWrapper { get; }
77+
public Task<bool> SameDocumentNavigationTask => _sameDocumentNavigationTaskWrapper.Task;
78+
public Task<bool> NewDocumentNavigationTask => _newDocumentNavigationTaskWrapper.Task;
7379
public Response NavigationResponse => _navigationRequest?.Response;
74-
public Task TimeoutTask { get; }
80+
public Task<Task> TimeoutOrTerminationTask => Task.WhenAny(_timeoutTask, _terminationTaskWrapper.Task);
81+
7582
#endregion
7683

7784
#region Private methods
@@ -90,13 +97,15 @@ private void CheckLifecycleComplete(object sender, FrameEventArgs e)
9097

9198
if (_hasSameDocumentNavigation)
9299
{
93-
SameDocumentNavigationTaskWrapper.TrySetResult(true);
100+
_sameDocumentNavigationTaskWrapper.TrySetResult(true);
94101
}
95102
if (_frame.LoaderId != _initialLoaderId)
96103
{
97-
NewDocumentNavigationTaskWrapper.TrySetResult(true);
104+
_newDocumentNavigationTaskWrapper.TrySetResult(true);
98105
}
99-
}
106+
}
107+
108+
private void Terminate(PuppeteerException ex) => _terminationTaskWrapper.TrySetException(ex);
100109

101110
private void OnRequest(object sender, RequestEventArgs e)
102111
{

‎lib/PuppeteerSharp/Page.cs‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -730,12 +730,12 @@ public async Task<Response> GoToAsync(string url, NavigationOptions options)
730730

731731
var mainFrame = _frameManager.MainFrame;
732732
var timeout = options?.Timeout ?? DefaultNavigationTimeout;
733+
var watcher = new NavigatorWatcher(Client, _frameManager, mainFrame, _networkManager, timeout, options);
733734

734-
var watcher = new NavigatorWatcher(_frameManager, mainFrame, _networkManager, timeout, options);
735735
var navigateTask = Navigate(Client, url, referrer);
736736

737737
await Task.WhenAny(
738-
watcher.TimeoutTask,
738+
watcher.TimeoutOrTerminationTask,
739739
navigateTask).ConfigureAwait(false);
740740

741741
AggregateException exception = null;
@@ -747,13 +747,13 @@ await Task.WhenAny(
747747
else
748748
{
749749
await Task.WhenAny(
750-
watcher.TimeoutTask,
750+
watcher.TimeoutOrTerminationTask,
751751
_ensureNewDocumentNavigation ? watcher.NewDocumentNavigationTask : watcher.SameDocumentNavigationTask
752752
).ConfigureAwait(false);
753753

754-
if (watcher.TimeoutTask.IsFaulted)
754+
if (watcher.TimeoutOrTerminationTask.IsCompleted && watcher.TimeoutOrTerminationTask.Result.IsFaulted)
755755
{
756-
exception = watcher.TimeoutTask.Exception;
756+
exception = watcher.TimeoutOrTerminationTask.Result.Exception;
757757
}
758758
}
759759

@@ -1414,15 +1414,21 @@ public async Task<Response> WaitForNavigationAsync(NavigationOptions options = n
14141414
{
14151415
var mainFrame = _frameManager.MainFrame;
14161416
var timeout = options?.Timeout ?? DefaultNavigationTimeout;
1417-
var watcher = new NavigatorWatcher(_frameManager, mainFrame, _networkManager, timeout, options);
1417+
var watcher = new NavigatorWatcher(Client, _frameManager, mainFrame, _networkManager, timeout, options);
14181418

14191419
var raceTask = await Task.WhenAny(
14201420
watcher.NewDocumentNavigationTask,
14211421
watcher.SameDocumentNavigationTask,
1422-
watcher.TimeoutTask
1422+
watcher.TimeoutOrTerminationTask
14231423
).ConfigureAwait(false);
14241424

14251425
var exception = raceTask.Exception;
1426+
if (exception == null &&
1427+
watcher.TimeoutOrTerminationTask.IsCompleted &&
1428+
watcher.TimeoutOrTerminationTask.Result.IsFaulted)
1429+
{
1430+
exception = watcher.TimeoutOrTerminationTask.Result.Exception;
1431+
}
14261432
if (exception != null)
14271433
{
14281434
throw new NavigationException(exception.Message, exception);
@@ -1837,7 +1843,7 @@ private async Task OnBindingCalled(BindingCalledResponse e)
18371843
{
18381844
expression,
18391845
contextId = e.ExecutionContextId
1840-
}).ConfigureAwait(false);
1846+
});
18411847
}
18421848

18431849
private async Task<object> ExecuteBinding(BindingCalledResponse e)

0 commit comments

Comments
 (0)