Skip to content

Commit 3affa0b

Browse files
amaitlandamaitland
committed
DevTools Client - Add Event error handling
- New event that is raised when an exception occurs during converting a response to JSON - Test cleanup - New unit test to validate IEventProxy has been removed from ConcurrentDictionary Resolves #3787
1 parent 880c466 commit 3affa0b

File tree

7 files changed

+179
-50
lines changed

7 files changed

+179
-50
lines changed

‎CefSharp.Test/DevTools/DevToolsClientFacts.cs‎

Lines changed: 52 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,6 @@ public async Task CanSetExtraHTTPHeaders()
263263
{
264264
await browser.CreateBrowserAsync();
265265

266-
RequestWillBeSentEventArgs requestWillBeSentEventArgs = null;
267266
using (var devToolsClient = browser.GetDevToolsClient())
268267
{
269268
var extraHeaders = new Headers();
@@ -273,32 +272,35 @@ public async Task CanSetExtraHTTPHeaders()
273272

274273
await devToolsClient.Network.SetExtraHTTPHeadersAsync(extraHeaders);
275274

276-
devToolsClient.Network.RequestWillBeSent += (sender, args) =>
277-
{
278-
if (requestWillBeSentEventArgs == null)
275+
var evtTask = Assert.RaisesAsync<RequestWillBeSentEventArgs>(
276+
x => devToolsClient.Network.RequestWillBeSent += x,
277+
x => devToolsClient.Network.RequestWillBeSent -= x,
278+
async () =>
279279
{
280-
requestWillBeSentEventArgs = args;
281-
}
282-
};
283-
284-
// enable events
285-
await devToolsClient.Network.EnableAsync();
286-
287-
await browser.LoadUrlAsync("www.google.com");
288-
}
289-
290-
Assert.NotNull(requestWillBeSentEventArgs);
291-
Assert.NotEmpty(requestWillBeSentEventArgs.RequestId);
292-
Assert.NotEqual(0, requestWillBeSentEventArgs.Timestamp);
293-
Assert.NotEqual(0, requestWillBeSentEventArgs.WallTime);
294-
Assert.NotNull(requestWillBeSentEventArgs.Request);
295-
Assert.True(requestWillBeSentEventArgs.Request.Headers.TryGetValues("TeSt", out var values));
296-
Assert.Collection(values,
297-
v => Assert.Equal("0", v),
298-
v => Assert.Equal("1", v),
299-
v => Assert.Equal(" 2 ", v),
300-
v => Assert.Equal(" 2,5 ", v)
301-
);
280+
// enable events
281+
await devToolsClient.Network.EnableAsync();
282+
283+
await browser.LoadUrlAsync("www.google.com");
284+
});
285+
286+
var xUnitEvent = await evtTask;
287+
Assert.NotNull(xUnitEvent);
288+
289+
var args = xUnitEvent.Arguments;
290+
291+
Assert.NotNull(args);
292+
Assert.NotEmpty(args.RequestId);
293+
Assert.NotEqual(0, args.Timestamp);
294+
Assert.NotEqual(0, args.WallTime);
295+
Assert.NotNull(args.Request);
296+
Assert.True(args.Request.Headers.TryGetValues("TeSt", out var values));
297+
Assert.Collection(values,
298+
v => Assert.Equal("0", v),
299+
v => Assert.Equal("1", v),
300+
v => Assert.Equal(" 2 ", v),
301+
v => Assert.Equal(" 2,5 ", v)
302+
);
303+
}
302304
}
303305
}
304306

@@ -388,16 +390,33 @@ public async Task CanRegisterMultipleEventHandlers()
388390
}
389391

390392
[Fact]
391-
public async Task CanRemoveEventListenerBeforeAddingOne()
393+
public void CanRemoveEventListenerBeforeAddingOne()
392394
{
393-
using (var browser = new ChromiumWebBrowser("about:blank", automaticallyCreateBrowser: false))
395+
using (var devToolsClient = new DevToolsClient(null))
394396
{
395-
await browser.CreateBrowserAsync();
397+
devToolsClient.Network.RequestWillBeSent -= (sender, args) => { };
398+
}
399+
}
396400

397-
using (var devToolsClient = browser.GetDevToolsClient())
398-
{
399-
devToolsClient.Network.RequestWillBeSent -= (sender, args) => { };
400-
}
401+
[Fact]
402+
public void IsIEventProxyRemovedFromConcurrentDictionary()
403+
{
404+
const string eventName = "Browser.downloadProgress";
405+
using (var devToolsClient = new DevToolsClient(null))
406+
{
407+
EventHandler<DownloadProgressEventArgs> eventHandler1 = (object sender, DownloadProgressEventArgs args) => { };
408+
EventHandler<DownloadProgressEventArgs> eventHandler2 = (object sender, DownloadProgressEventArgs args) => { };
409+
410+
devToolsClient.AddEventHandler(eventName, eventHandler1);
411+
devToolsClient.AddEventHandler(eventName, eventHandler2);
412+
413+
var hasHandlers = devToolsClient.RemoveEventHandler(eventName, eventHandler1);
414+
415+
Assert.True(hasHandlers);
416+
417+
hasHandlers = devToolsClient.RemoveEventHandler(eventName, eventHandler2);
418+
419+
Assert.False(hasHandlers);
401420
}
402421
}
403422
}

‎CefSharp/CefSharp.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@
8888
<Compile Include="DevTools\DevToolsClient.cs" />
8989
<Compile Include="DevTools\DevToolsClient.Generated.cs" />
9090
<Compile Include="DevTools\DevToolsClientException.cs" />
91+
<Compile Include="DevTools\DevToolsErrorEventArgs.cs" />
9192
<Compile Include="DevTools\DevToolsMethodResponseContext.cs" />
9293
<Compile Include="DevTools\DevToolsDomainBase.cs" />
9394
<Compile Include="DevTools\DevToolsDomainEntityBase.cs" />

‎CefSharp/DevTools/DevToolsClient.cs‎

Lines changed: 45 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ public partial class DevToolsClient : IDevToolsMessageObserver, IDevToolsClient
3030
/// <inheritdoc/>
3131
public event EventHandler<DevToolsEventArgs> DevToolsEvent;
3232

33+
/// <inheritdoc/>
34+
public event EventHandler<DevToolsErrorEventArgs> DevToolsEventError;
35+
3336
/// <summary>
3437
/// Capture the current <see cref="SynchronizationContext"/> so
3538
/// continuation executes on the original calling thread. If
@@ -83,21 +86,24 @@ public void AddEventHandler<T>(string eventName, EventHandler<T> eventHandler) w
8386
var eventProxy = eventHandlers.GetOrAdd(eventName, _ => new EventProxy<T>(DeserializeJsonEvent<T>));
8487

8588
var p = (EventProxy<T>)eventProxy;
89+
8690
p.AddHandler(eventHandler);
8791
}
8892

8993
/// <inheritdoc/>
90-
public void RemoveEventHandler<T>(string eventName, EventHandler<T> eventHandler) where T : EventArgs
94+
public bool RemoveEventHandler<T>(string eventName, EventHandler<T> eventHandler) where T : EventArgs
9195
{
9296
if (eventHandlers.TryGetValue(eventName, out IEventProxy eventProxy))
9397
{
9498
var p = ((EventProxy<T>)eventProxy);
99+
95100
if(p.RemoveHandler(eventHandler))
96101
{
97-
//TODO: Replace with out _ once we upgrade to VS2019
98-
eventHandlers.TryRemove(eventName, out IEventProxy e);
102+
return !eventHandlers.TryRemove(eventName, out _);
99103
}
100104
}
105+
106+
return true;
101107
}
102108

103109
/// <summary>
@@ -235,19 +241,46 @@ void IDevToolsMessageObserver.OnDevToolsAgentDetached(IBrowser browser)
235241
/// <inheritdoc/>
236242
void IDevToolsMessageObserver.OnDevToolsEvent(IBrowser browser, string method, Stream parameters)
237243
{
238-
var evt = DevToolsEvent;
239-
240-
//Only parse the data if we have an event handler
241-
if (evt != null)
244+
try
242245
{
243-
var paramsAsJsonString = StreamToString(parameters, leaveOpen: true);
246+
var evt = DevToolsEvent;
244247

245-
evt(this, new DevToolsEventArgs(method, paramsAsJsonString));
246-
}
248+
//Only parse the data if we have an event handler
249+
if (evt != null)
250+
{
251+
var paramsAsJsonString = StreamToString(parameters, leaveOpen: true);
252+
253+
evt(this, new DevToolsEventArgs(method, paramsAsJsonString));
254+
}
247255

248-
if (eventHandlers.TryGetValue(method, out IEventProxy eventProxy))
256+
if (eventHandlers.TryGetValue(method, out IEventProxy eventProxy))
257+
{
258+
eventProxy.Raise(this, method, parameters, SyncContext);
259+
}
260+
}
261+
catch(Exception ex)
249262
{
250-
eventProxy.Raise(this, method, parameters);
263+
var errorEvent = DevToolsEventError;
264+
265+
var json = "";
266+
267+
if(parameters.Length > 0)
268+
{
269+
parameters.Position = 0;
270+
271+
try
272+
{
273+
json = StreamToString(parameters, leaveOpen: false);
274+
}
275+
catch(Exception)
276+
{
277+
//TODO: do we somehow pass this exception to the user?
278+
}
279+
}
280+
281+
var args = new DevToolsErrorEventArgs(method, json, ex);
282+
283+
errorEvent?.Invoke(this, args);
251284
}
252285
}
253286

‎CefSharp/DevTools/DevToolsErrorEventArgs.cs‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// Copyright © 2021 The CefSharp Authors. All rights reserved.
2+
//
3+
// Use of this source code is governed by a BSD-style license that can be found in the LICENSE file.
4+
5+
using System;
6+
7+
namespace CefSharp.DevTools
8+
{
9+
/// <summary>
10+
/// DevToolsErrorEventArgs - Raised when an exception occurs when
11+
/// attempting to raise <see cref="IDevToolsClient.DevToolsEvent"/>
12+
/// </summary>
13+
public class DevToolsErrorEventArgs : EventArgs
14+
{
15+
/// <summary>
16+
/// Event Name
17+
/// </summary>
18+
public string EventName { get; private set; }
19+
20+
/// <summary>
21+
/// Json
22+
/// </summary>
23+
public string Json { get; private set; }
24+
25+
/// <summary>
26+
/// Exception
27+
/// </summary>
28+
public Exception Exception { get; private set; }
29+
30+
/// <summary>
31+
/// DevToolsErrorEventArgs
32+
/// </summary>
33+
/// <param name="eventName">Event Name</param>
34+
/// <param name="json">json</param>
35+
/// <param name="ex">Exception</param>
36+
public DevToolsErrorEventArgs(string eventName, string json, Exception ex)
37+
{
38+
EventName = eventName;
39+
Json = json;
40+
Exception = ex;
41+
}
42+
}
43+
}

‎CefSharp/DevTools/EventProxy.cs‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
using System;
88
using System.IO;
9+
using System.Threading;
910

1011
namespace CefSharp.DevTools
1112
{
@@ -18,11 +19,19 @@ internal class EventProxy<T> : IEventProxy
1819
private event EventHandler<T> handlers;
1920
private Func<string, Stream, T> convert;
2021

22+
/// <summary>
23+
/// Constructor
24+
/// </summary>
25+
/// <param name="convert">Delegate used to convert from the Stream to event args</param>
2126
public EventProxy(Func<string, Stream, T> convert)
2227
{
2328
this.convert = convert;
2429
}
2530

31+
/// <summary>
32+
/// Add the event handler
33+
/// </summary>
34+
/// <param name="handler">event handler to add</param>
2635
public void AddHandler(EventHandler<T> handler)
2736
{
2837
handlers += handler;
@@ -40,15 +49,28 @@ public bool RemoveHandler(EventHandler<T> handler)
4049
return handlers == null;
4150
}
4251

43-
public void Raise(object sender, string eventName, Stream stream)
52+
/// <inheritdoc/>
53+
public void Raise(object sender, string eventName, Stream stream, SynchronizationContext syncContext)
4454
{
4555
stream.Position = 0;
4656

4757
var args = convert(eventName, stream);
4858

49-
handlers?.Invoke(sender, args);
59+
if (syncContext == null)
60+
{
61+
handlers?.Invoke(sender, args);
62+
}
63+
else
64+
{
65+
syncContext.Post(new SendOrPostCallback(state =>
66+
{
67+
handlers?.Invoke(sender, args);
68+
69+
}), null);
70+
}
5071
}
5172

73+
/// <inheritdoc/>
5274
public void Dispose()
5375
{
5476
handlers = null;

‎CefSharp/DevTools/IDevToolsClient.cs‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ public interface IDevToolsClient
2020
/// </summary>
2121
event EventHandler<DevToolsEventArgs> DevToolsEvent;
2222

23+
/// <summary>
24+
/// Will be called when an error occurs when attempting to raise <see cref="DevToolsEvent"/>
25+
/// </summary>
26+
event EventHandler<DevToolsErrorEventArgs> DevToolsEventError;
27+
2328
/// <summary>
2429
/// Add event handler for a DevTools protocol event. Events by default are disabled and need to be
2530
/// enabled on a per domain basis, e.g. Sending Network.enable (or calling <see cref="Network.NetworkClient.EnableAsync(int?, int?, int?)"/>)
@@ -36,7 +41,11 @@ public interface IDevToolsClient
3641
/// <typeparam name="T">The event args type to which the event will be deserialized to.</typeparam>
3742
/// <param name="eventName">is the event name to listen to</param>
3843
/// <param name="eventHandler">event handler to call when the event occurs</param>
39-
void RemoveEventHandler<T>(string eventName, EventHandler<T> eventHandler) where T : EventArgs;
44+
/// <returns>
45+
/// Returns false if all handlers for the <paramref name="eventName"/> have been removed,
46+
/// otherwise returns true if there are still handlers registered.
47+
/// </returns>
48+
bool RemoveEventHandler<T>(string eventName, EventHandler<T> eventHandler) where T : EventArgs;
4049

4150
/// <summary>
4251
/// Execute a method call over the DevTools protocol. This method can be called on any thread.

‎CefSharp/DevTools/IEventProxy.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
using System;
88
using System.IO;
9+
using System.Threading;
910

1011
namespace CefSharp.DevTools
1112
{
@@ -19,7 +20,8 @@ internal interface IEventProxy : IDisposable
1920
/// </summary>
2021
/// <param name="sender">sender</param>
2122
/// <param name="eventName">event name</param>
22-
/// <param name="stream">json Stream</param>
23-
void Raise(object sender, string eventName, Stream stream);
23+
/// <param name="stream">Stream containing JSON</param>
24+
/// <param name="syncContext">SynchronizationContext</param>
25+
void Raise(object sender, string eventName, Stream stream, SynchronizationContext syncContext);
2426
}
2527
}

0 commit comments

Comments
 (0)