Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

request error handling #23

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions source/ChromeDevTools/ChromeSession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ public class ChromeSession : IChromeSession
private readonly ConcurrentDictionary<string, ConcurrentBag<Action<object>>> _handlers = new ConcurrentDictionary<string, ConcurrentBag<Action<object>>>();
private ICommandFactory _commandFactory;
private IEventFactory _eventFactory;
private readonly Action<Exception> _onError;
private ManualResetEvent _openEvent = new ManualResetEvent(false);
private ManualResetEvent _publishEvent = new ManualResetEvent(false);
private ConcurrentDictionary<long, ManualResetEventSlim> _requestWaitHandles = new ConcurrentDictionary<long, ManualResetEventSlim>();
Expand All @@ -24,12 +25,13 @@ public class ChromeSession : IChromeSession
private WebSocket _webSocket;
private static object _Lock = new object();

public ChromeSession(string endpoint, ICommandFactory commandFactory, ICommandResponseFactory responseFactory, IEventFactory eventFactory)
public ChromeSession(string endpoint, ICommandFactory commandFactory, ICommandResponseFactory responseFactory, IEventFactory eventFactory, Action<Exception> onError)
Copy link
Member

@brewdente brewdente Dec 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onError should be optional. If it's not provided, it should throw by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is a breaking change, but if we keep the throwing behaviour, there is no place to catch it - since it can occur on another thread while recieving event from chrome.
Are you sure you really want to keep this behaviour by default instead of forcing user to handle it?

{
_endpoint = endpoint;
_commandFactory = commandFactory;
_responseFactory = responseFactory;
_eventFactory = eventFactory;
_onError = onError;
}

public void Dispose()
Expand Down Expand Up @@ -235,12 +237,12 @@ private void WebSocket_DataReceived(object sender, DataReceivedEventArgs e)
HandleEvent(evnt);
return;
}
throw new Exception("Don't know what to do with response: " + e.Data);
_onError(new Exception("Don't know what to do with response: " + e.Data));
}

private void WebSocket_Error(object sender, SuperSocket.ClientEngine.ErrorEventArgs e)
{
throw e.Exception;
_onError(e.Exception);
}

private void WebSocket_MessageReceived(object sender, MessageReceivedEventArgs e)
Expand All @@ -257,7 +259,7 @@ private void WebSocket_MessageReceived(object sender, MessageReceivedEventArgs e
HandleEvent(evnt);
return;
}
throw new Exception("Don't know what to do with response: " + e.Message);
_onError(new Exception("Don't know what to do with response: " + e.Message));
}

private void WebSocket_Opened(object sender, EventArgs e)
Expand Down
12 changes: 7 additions & 5 deletions source/ChromeDevTools/ChromeSessionFactory.cs
Original file line number Diff line number Diff line change
@@ -1,22 +1,24 @@
#if !NETSTANDARD1_5
using System;

#if !NETSTANDARD1_5
namespace MasterDevs.ChromeDevTools
{
public class ChromeSessionFactory : IChromeSessionFactory
{
public IChromeSession Create(ChromeSessionInfo sessionInfo)
public IChromeSession Create(ChromeSessionInfo sessionInfo, Action<Exception> onError)
{
return Create(sessionInfo.WebSocketDebuggerUrl);
return Create(sessionInfo.WebSocketDebuggerUrl, onError);
}

public IChromeSession Create(string endpointUrl)
public IChromeSession Create(string endpointUrl, Action<Exception> onError)
{
// Sometimes binding to localhost might resolve wrong AddressFamily, force IPv4
endpointUrl = endpointUrl.Replace("ws://localhost", "ws://127.0.0.1");
var methodTypeMap = new MethodTypeMap();
var commandFactory = new CommandFactory();
var responseFactory = new CommandResponseFactory(methodTypeMap, commandFactory);
var eventFactory = new EventFactory(methodTypeMap);
var session = new ChromeSession(endpointUrl, commandFactory, responseFactory, eventFactory);
var session = new ChromeSession(endpointUrl, commandFactory, responseFactory, eventFactory, onError);
return session;
}
}
Expand Down
6 changes: 4 additions & 2 deletions source/ChromeDevTools/IChromeSessionFactory.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
namespace MasterDevs.ChromeDevTools
using System;

namespace MasterDevs.ChromeDevTools
{
public interface IChromeSessionFactory
{
IChromeSession Create(string endpointUrl);
IChromeSession Create(string endpointUrl, Action<Exception> onError);
}
}
8 changes: 7 additions & 1 deletion source/Sample/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ private static void Main(string[] args)
// STEP 3 - Create a debugging session
var sessionInfo = (await chromeProcess.GetSessionInfo()).LastOrDefault();
var chromeSessionFactory = new ChromeSessionFactory();
var chromeSession = chromeSessionFactory.Create(sessionInfo.WebSocketDebuggerUrl);
var chromeSession = chromeSessionFactory.Create(sessionInfo.WebSocketDebuggerUrl,OnError );

// STEP 4 - Send a command
//
Expand Down Expand Up @@ -100,5 +100,11 @@ await chromeSession.SendAsync(
}
}).Wait();
}

private static void OnError(Exception exception)
{
Console.WriteLine("Error during communication:");
Console.WriteLine(exception);
}
}
}