request error handling #23
Conversation
|
|
||
| namespace MasterDevs.ChromeDevTools | ||
| { | ||
| public interface ICommandResponseWrapper<T> |
There was a problem hiding this comment.
@svatal any reason not to merge the ICommandResponse and ICommandResponseWrapper?
There was a problem hiding this comment.
I have been afraid how deserialization would cope with a class that have something more than a basic properties. Did not tested it though.
if the communication from chrome results in Exception, there is no way for user of the library to catch/handle it (except for AppDomain.CurrentDomain.UnhandledException)
|
I've added error handling for the response. Since it does not happen on main program flow, I haven't found a way how to catch these exeptions ... I've been using protocol 60 with chromium 62 and for a one particulat website (seznam.cz) it delivered Page.lifecycle events even thought they have own switch to enable them. Protocol update will probably follow soon in own PR. |
| @@ -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) | |||
There was a problem hiding this comment.
onError should be optional. If it's not provided, it should throw by default.
There was a problem hiding this comment.
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?
Conflicts: source/Sample/Program.cs
|
Ad AppVeyor fail: comparing with older builds, dotnet pack built only netstandard, not net45 but were looking for both anyway - could you look into it, please? |
No description provided.