Skip to content

Commit 1361f65

Browse files
authored
Support timeouts per CDP command (#2500)
1 parent daffca3 commit 1361f65

File tree

13 files changed

+91
-64
lines changed

13 files changed

+91
-64
lines changed

lib/PuppeteerSharp.Tests/CDPSessionTests/CreateCDPSessionTests.cs

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ await Task.WhenAll(
5858
Page.EvaluateExpressionAsync("//# sourceURL=foo.js")
5959
);
6060
// expect events to be dispatched.
61-
Assert.AreEqual("foo.js", eventTask.Result["url"].Value<string>());
61+
Assert.AreEqual("foo.js", eventTask.Result["url"]!.Value<string>());
6262
}
6363

6464
[Test, Retry(2), PuppeteerTest("CDPSession.spec", "Target.createCDPSession", "should be able to detach session")]
@@ -71,7 +71,7 @@ public async Task ShouldBeAbleToDetachSession()
7171
Expression = "1 + 2",
7272
ReturnByValue = true
7373
});
74-
Assert.AreEqual(3, evalResponse["result"]["value"].ToObject<int>());
74+
Assert.AreEqual(3, evalResponse["result"]!["value"]!.ToObject<int>());
7575
await client.DetachAsync();
7676

7777
var exception = Assert.ThrowsAsync<TargetClosedException>(()
@@ -80,7 +80,7 @@ public async Task ShouldBeAbleToDetachSession()
8080
Expression = "3 + 1",
8181
ReturnByValue = true
8282
}));
83-
StringAssert.Contains("Session closed.", exception.Message);
83+
StringAssert.Contains("Session closed.", exception!.Message);
8484
}
8585

8686
[Test, Retry(2), PuppeteerTest("CDPSession.spec", "Target.createCDPSession", "should not report created targets for custom CDP sessions")]
@@ -112,10 +112,32 @@ public async Task ShouldThrowNiceErrors()
112112
{
113113
await TheSourceOfTheProblems();
114114
});
115-
StringAssert.Contains("TheSourceOfTheProblems", exception.StackTrace);
115+
StringAssert.Contains("TheSourceOfTheProblems", exception!.StackTrace);
116116
StringAssert.Contains("ThisCommand.DoesNotExist", exception.Message);
117117
}
118118

119+
[Test, Retry(2), PuppeteerTest("CDPSession.spec", "Target.createCDPSession", "should respect custom timeout")]
120+
public async Task ShouldRespectCustomTimeout()
121+
{
122+
var client = await Page.CreateCDPSessionAsync();
123+
var exception = Assert.ThrowsAsync<TimeoutException>(async () =>
124+
{
125+
await client.SendAsync(
126+
"Runtime.evaluate",
127+
new RuntimeEvaluateRequest()
128+
{
129+
Expression = "new Promise(x => {})",
130+
AwaitPromise = true,
131+
},
132+
true,
133+
new CommandOptions
134+
{
135+
Timeout = 50
136+
});
137+
});
138+
StringAssert.Contains("Timeout of 50 ms exceeded", exception!.Message);
139+
}
140+
119141
[Test, Retry(2), PuppeteerTest("CDPSession.spec", "Target.createCDPSession", "should expose the underlying connection")]
120142
public async Task ShouldExposeTheUnderlyingConnection()
121143
=> Assert.NotNull(await Page.CreateCDPSessionAsync());

lib/PuppeteerSharp.Tests/DeviceRequestPromptTests/MockCDPSession.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,10 @@ public class MockCDPSession : ICDPSession
3232
{
3333
public event EventHandler<MessageEventArgs> MessageReceived;
3434

35-
public Task<JObject> SendAsync(string method, object args = null, bool waitForCallback = true)
36-
=> SendAsync<JObject>(method, args);
35+
public Task<JObject> SendAsync(string method, object args = null, bool waitForCallback = true, CommandOptions options = null)
36+
=> SendAsync<JObject>(method, args, options);
3737

38-
public Task<T> SendAsync<T>(string method, object args = null)
38+
public Task<T> SendAsync<T>(string method, object args = null, CommandOptions options = null)
3939
{
4040
Console.WriteLine($"Mock Session send: {method} {args?.ToString() ?? string.Empty}");
4141
return Task.FromResult<T>(default);

lib/PuppeteerSharp/CDPSession.cs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Threading.Tasks;
77
using Microsoft.Extensions.Logging;
88
using Newtonsoft.Json.Linq;
9+
using PuppeteerSharp.Helpers;
910
using PuppeteerSharp.Helpers.Json;
1011
using PuppeteerSharp.Messaging;
1112

@@ -65,14 +66,14 @@ internal CDPSession ParentSession
6566
=> string.IsNullOrEmpty(_parentSessionId) ? this : Connection.GetSession(_parentSessionId) ?? this;
6667

6768
/// <inheritdoc/>
68-
public async Task<T> SendAsync<T>(string method, object args = null)
69+
public async Task<T> SendAsync<T>(string method, object args = null, CommandOptions options = null)
6970
{
70-
var content = await SendAsync(method, args).ConfigureAwait(false);
71+
var content = await SendAsync(method, args, true, options).ConfigureAwait(false);
7172
return content.ToObject<T>(true);
7273
}
7374

7475
/// <inheritdoc/>
75-
public async Task<JObject> SendAsync(string method, object args = null, bool waitForCallback = true)
76+
public async Task<JObject> SendAsync(string method, object args = null, bool waitForCallback = true, CommandOptions options = null)
7677
{
7778
if (Connection == null)
7879
{
@@ -100,7 +101,7 @@ public async Task<JObject> SendAsync(string method, object args = null, bool wai
100101

101102
try
102103
{
103-
await Connection.RawSendAsync(message).ConfigureAwait(false);
104+
await Connection.RawSendAsync(message, options).ConfigureAwait(false);
104105
}
105106
catch (Exception ex)
106107
{
@@ -110,7 +111,7 @@ public async Task<JObject> SendAsync(string method, object args = null, bool wai
110111
}
111112
}
112113

113-
return waitForCallback ? await callback.TaskWrapper.Task.ConfigureAwait(false) : null;
114+
return waitForCallback ? await callback.TaskWrapper.Task.WithTimeout(options?.Timeout ?? Connection.ProtocolTimeout).ConfigureAwait(false) : null;
114115
}
115116

116117
/// <inheritdoc/>
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
namespace PuppeteerSharp;
2+
3+
/// <summary>
4+
/// Command options. See <see cref="ICDPConnection.SendAsync{T}(string, object, CommandOptions)"/>.
5+
/// </summary>
6+
public class CommandOptions
7+
{
8+
/// <summary>
9+
/// Gets or sets the timeout.
10+
/// </summary>
11+
/// <value>The timeout.</value>
12+
public int Timeout { get; set; }
13+
}

lib/PuppeteerSharp/ConnectOptions.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,9 @@ public class ConnectOptions : IBrowserOptions, IConnectionOptions
9191
/// </summary>
9292
public Func<Target, bool> TargetFilter { get; set; }
9393

94+
/// <inheritdoc />
95+
public int ProtocolTimeout { get; set; } = Connection.DefaultCommandTimeout;
96+
9497
/// <summary>
9598
/// Optional callback to initialize properties as soon as the <see cref="IBrowser"/> instance is created, i.e., set up event handlers.
9699
/// </summary>

lib/PuppeteerSharp/Connection.cs

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,25 +18,26 @@ namespace PuppeteerSharp
1818
/// <summary>
1919
/// A connection handles the communication with a Chromium browser.
2020
/// </summary>
21-
public class Connection : IDisposable, ICDPConnection
21+
public sealed class Connection : IDisposable, ICDPConnection
2222
{
23+
internal const int DefaultCommandTimeout = 180_000;
2324
private readonly ILogger _logger;
2425
private readonly TaskQueue _callbackQueue = new();
2526

2627
private readonly ConcurrentDictionary<int, MessageTask> _callbacks = new();
2728
private readonly AsyncDictionaryHelper<string, CDPSession> _sessions = new("Session {0} not found");
28-
private readonly List<string> _manuallyAttached = new();
29+
private readonly List<string> _manuallyAttached = [];
2930
private int _lastId;
3031

31-
internal Connection(string url, int delay, bool enqueueAsyncMessages, IConnectionTransport transport, ILoggerFactory loggerFactory = null)
32+
private Connection(string url, int delay, bool enqueueAsyncMessages, IConnectionTransport transport, ILoggerFactory loggerFactory = null, int protocolTimeout = DefaultCommandTimeout)
3233
{
3334
LoggerFactory = loggerFactory ?? new LoggerFactory();
3435
Url = url;
3536
Delay = delay;
3637
Transport = transport;
3738

3839
_logger = LoggerFactory.CreateLogger<Connection>();
39-
40+
ProtocolTimeout = protocolTimeout;
4041
MessageQueue = new AsyncMessageQueue(enqueueAsyncMessages, _logger);
4142

4243
Transport.MessageReceived += Transport_MessageReceived;
@@ -101,6 +102,8 @@ internal Connection(string url, int delay, bool enqueueAsyncMessages, IConnectio
101102

102103
internal ScriptInjector ScriptInjector { get; } = new();
103104

105+
internal int ProtocolTimeout { get; }
106+
104107
/// <inheritdoc />
105108
public void Dispose()
106109
{
@@ -109,7 +112,7 @@ public void Dispose()
109112
}
110113

111114
/// <inheritdoc/>
112-
public async Task<JObject> SendAsync(string method, object args = null, bool waitForCallback = true)
115+
public async Task<JObject> SendAsync(string method, object args = null, bool waitForCallback = true, CommandOptions options = null)
113116
{
114117
if (IsClosed)
115118
{
@@ -131,36 +134,30 @@ public async Task<JObject> SendAsync(string method, object args = null, bool wai
131134
_callbacks[id] = callback;
132135
}
133136

134-
await RawSendAsync(message).ConfigureAwait(false);
135-
return waitForCallback ? await callback.TaskWrapper.Task.ConfigureAwait(false) : null;
137+
await RawSendAsync(message, options).ConfigureAwait(false);
138+
return waitForCallback ? await callback.TaskWrapper.Task.WithTimeout(ProtocolTimeout).ConfigureAwait(false) : null;
136139
}
137140

138141
/// <inheritdoc/>
139-
public async Task<T> SendAsync<T>(string method, object args = null)
142+
public async Task<T> SendAsync<T>(string method, object args = null, CommandOptions options = null)
140143
{
141-
var response = await SendAsync(method, args).ConfigureAwait(false);
144+
var response = await SendAsync(method, args, true, options).ConfigureAwait(false);
142145
return response.ToObject<T>(true);
143146
}
144147

145148
internal static async Task<Connection> Create(string url, IConnectionOptions connectionOptions, ILoggerFactory loggerFactory = null, CancellationToken cancellationToken = default)
146149
{
147-
#pragma warning disable 618
148-
var transport = connectionOptions.Transport;
149-
#pragma warning restore 618
150-
if (transport == null)
151-
{
152-
var transportFactory = connectionOptions.TransportFactory ?? WebSocketTransport.DefaultTransportFactory;
153-
transport = await transportFactory(new Uri(url), connectionOptions, cancellationToken).ConfigureAwait(false);
154-
}
150+
var transportFactory = connectionOptions.TransportFactory ?? WebSocketTransport.DefaultTransportFactory;
151+
var transport = await transportFactory(new Uri(url), connectionOptions, cancellationToken).ConfigureAwait(false);
155152

156-
return new Connection(url, connectionOptions.SlowMo, connectionOptions.EnqueueAsyncMessages, transport, loggerFactory);
153+
return new Connection(url, connectionOptions.SlowMo, connectionOptions.EnqueueAsyncMessages, transport, loggerFactory, connectionOptions.ProtocolTimeout);
157154
}
158155

159156
internal static Connection FromSession(CDPSession session) => session.Connection;
160157

161158
internal int GetMessageID() => Interlocked.Increment(ref _lastId);
162159

163-
internal Task RawSendAsync(string message)
160+
internal Task RawSendAsync(string message, CommandOptions options = null)
164161
{
165162
_logger.LogTrace("Send ► {Message}", message);
166163
return Transport.SendAsync(message);
@@ -205,7 +202,7 @@ internal void Close(string closeReason)
205202
CloseReason = closeReason;
206203

207204
Transport.StopReading();
208-
Disconnected?.Invoke(this, new EventArgs());
205+
Disconnected?.Invoke(this, EventArgs.Empty);
209206

210207
foreach (var session in _sessions.Values.ToArray())
211208
{
@@ -237,7 +234,7 @@ internal void Close(string closeReason)
237234
/// <see cref="Connection"/> so the garbage collector can reclaim the memory that the
238235
/// <see cref="Connection"/> was occupying.</remarks>
239236
/// <param name="disposing">Indicates whether disposal was initiated by <see cref="Dispose()"/> operation.</param>
240-
protected virtual void Dispose(bool disposing)
237+
private void Dispose(bool disposing)
241238
{
242239
Close("Connection disposed");
243240
Transport.MessageReceived -= Transport_MessageReceived;

lib/PuppeteerSharp/ICDPConnection.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,19 @@ public interface ICDPConnection
2323
/// If <c>true</c> the method will return a task to be completed when the message is confirmed by Chromium.
2424
/// If <c>false</c> the task will be considered complete after sending the message to Chromium.
2525
/// </param>
26+
/// <param name="options">The options.</param>
2627
/// <returns>The task.</returns>
2728
/// <exception cref="PuppeteerSharp.PuppeteerException">If the <see cref="Connection"/> is closed.</exception>
28-
Task<JObject> SendAsync(string method, object args = null, bool waitForCallback = true);
29+
Task<JObject> SendAsync(string method, object args = null, bool waitForCallback = true, CommandOptions options = null);
2930

3031
/// <summary>
3132
/// Protocol methods can be called with this method.
3233
/// </summary>
3334
/// <param name="method">The method name.</param>
3435
/// <param name="args">The method args.</param>
36+
/// <param name="options">The options.</param>
3537
/// <typeparam name="T">Return type.</typeparam>
3638
/// <returns>The task.</returns>
37-
Task<T> SendAsync<T>(string method, object args = null);
39+
Task<T> SendAsync<T>(string method, object args = null, CommandOptions options = null);
3840
}
3941
}

lib/PuppeteerSharp/ICDPSession.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ namespace PuppeteerSharp
66
{
77
/// <summary>
88
/// The CDPSession instances are used to talk raw Chrome Devtools Protocol:
9-
/// * Protocol methods can be called with <see cref="ICDPConnection.SendAsync(string, object, bool)"/> method.
9+
/// * Protocol methods can be called with <see cref="ICDPConnection.SendAsync(string, object, bool, CommandOptions)"/> method.
1010
/// * Protocol events, using the <see cref="ICDPConnection.MessageReceived"/> event.
1111
///
1212
/// Documentation on DevTools Protocol can be found here: <see href="https://chromedevtools.github.io/devtools-protocol/"/>.

lib/PuppeteerSharp/IConnectionOptions.cs

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,24 +14,12 @@ public interface IConnectionOptions
1414
/// </summary>
1515
int SlowMo { get; set; }
1616

17-
/// <summary>
18-
/// Keep alive value (in milliseconds).
19-
/// </summary>
20-
[Obsolete("Chromium doesn't support pings yet (see: https://bugs.chromium.org/p/chromium/issues/detail?id=865002)")]
21-
int KeepAliveInterval { get; set; }
22-
2317
/// <summary>
2418
/// Optional factory for <see cref="WebSocket"/> implementations.
2519
/// If <see cref="Transport"/> is set this property will be ignored.
2620
/// </summary>
2721
WebSocketFactory WebSocketFactory { get; set; }
2822

29-
/// <summary>
30-
/// Optional connection transport factory.
31-
/// </summary>
32-
[Obsolete("Use " + nameof(TransportFactory) + " instead")]
33-
IConnectionTransport Transport { get; set; }
34-
3523
/// <summary>
3624
/// Optional factory for <see cref="IConnectionTransport"/> implementations.
3725
/// </summary>
@@ -51,5 +39,11 @@ public interface IConnectionOptions
5139
/// Callback to decide if Puppeteer should connect to a given target or not.
5240
/// </summary>
5341
public Func<Target, bool> TargetFilter { get; set; }
42+
43+
/// <summary>
44+
/// Timeout setting for individual protocol (CDP) calls.
45+
/// Defaults to 180_000.
46+
/// </summary>
47+
public int ProtocolTimeout { get; set; }
5448
}
5549
}

lib/PuppeteerSharp/LaunchOptions.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,9 @@ public string[] IgnoredDefaultArgs
198198
/// </summary>
199199
public Func<Target, bool> TargetFilter { get; set; }
200200

201+
/// <inheritdoc />
202+
public int ProtocolTimeout { get; set; } = Connection.DefaultCommandTimeout;
203+
201204
/// <summary>
202205
/// Callback to decide if Puppeteer should connect to a given target or not.
203206
/// </summary>

0 commit comments

Comments
 (0)