From e55dc2e4966c94c9a1597e7927e1fce99dce7cb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dar=C3=ADo=20Kondratiuk?= Date: Wed, 21 Nov 2018 08:06:09 -0300 Subject: [PATCH] Improve internal exception handling (#753) --- .../BrowserTests/Events/DisconnectedTests.cs | 1 + .../PageTests/CloseTests.cs | 5 +- lib/PuppeteerSharp/Browser.cs | 29 ++-- lib/PuppeteerSharp/CDPSession.cs | 21 ++- lib/PuppeteerSharp/Connection.cs | 131 ++++++++++-------- lib/PuppeteerSharp/FrameManager.cs | 89 ++++++------ lib/PuppeteerSharp/IConnection.cs | 13 +- lib/PuppeteerSharp/NavigatorWatcher.cs | 5 +- lib/PuppeteerSharp/NetworkManager.cs | 47 ++++--- lib/PuppeteerSharp/Page.cs | 83 ++++++----- .../PageCoverage/CSSCoverage.cs | 29 ++-- lib/PuppeteerSharp/PageCoverage/JSCoverage.cs | 29 ++-- lib/PuppeteerSharp/TargetClosedException.cs | 11 +- lib/PuppeteerSharp/Tracing.cs | 22 ++- .../Transport/IConnectionTransport.cs | 2 +- .../Transport/TransportClosedEventArgs.cs | 19 +++ .../Transport/WebSocketTransport.cs | 14 +- lib/PuppeteerSharp/Worker.cs | 29 ++-- 18 files changed, 357 insertions(+), 222 deletions(-) create mode 100644 lib/PuppeteerSharp/Transport/TransportClosedEventArgs.cs diff --git a/lib/PuppeteerSharp.Tests/BrowserTests/Events/DisconnectedTests.cs b/lib/PuppeteerSharp.Tests/BrowserTests/Events/DisconnectedTests.cs index 139b704b7..15ccdddc9 100644 --- a/lib/PuppeteerSharp.Tests/BrowserTests/Events/DisconnectedTests.cs +++ b/lib/PuppeteerSharp.Tests/BrowserTests/Events/DisconnectedTests.cs @@ -90,6 +90,7 @@ public async Task ShouldRejectWaitForSelectorWhenBrowserCloses() //Whether from the Connection rejecting a message from the CDPSession //or from the CDPSession trying to send a message to a closed connection Assert.IsType(exception.InnerException); + Assert.Equal("Connection disposed", ((TargetClosedException)exception.InnerException).CloseReason); } } } diff --git a/lib/PuppeteerSharp.Tests/PageTests/CloseTests.cs b/lib/PuppeteerSharp.Tests/PageTests/CloseTests.cs index 1aca1b711..425a78a1a 100644 --- a/lib/PuppeteerSharp.Tests/PageTests/CloseTests.cs +++ b/lib/PuppeteerSharp.Tests/PageTests/CloseTests.cs @@ -13,13 +13,12 @@ public class CloseTests : PuppeteerPageBaseTest public async Task ShouldRejectAllPromisesWhenPageIsClosed() { var neverResolves = Page.EvaluateFunctionAsync("() => new Promise(r => {})"); - - // Put into a var to avoid warning - var t = Page.CloseAsync(); + _ = Page.CloseAsync(); var exception = await Assert.ThrowsAsync(async () => await neverResolves); Assert.IsType(exception.InnerException); Assert.Contains("Protocol error", exception.Message); + Assert.Equal("Target.detachedFromTarget", ((TargetClosedException)exception.InnerException).CloseReason); } [Fact] diff --git a/lib/PuppeteerSharp/Browser.cs b/lib/PuppeteerSharp/Browser.cs index 1ae05be9b..f4752a751 100644 --- a/lib/PuppeteerSharp/Browser.cs +++ b/lib/PuppeteerSharp/Browser.cs @@ -392,19 +392,28 @@ internal async Task DisposeContextAsync(string contextId) private async void Connect_MessageReceived(object sender, MessageEventArgs e) { - switch (e.MessageID) + try { - case "Target.targetCreated": - await CreateTargetAsync(e.MessageData.ToObject()).ConfigureAwait(false); - return; + switch (e.MessageID) + { + case "Target.targetCreated": + await CreateTargetAsync(e.MessageData.ToObject()).ConfigureAwait(false); + return; - case "Target.targetDestroyed": - await DestroyTargetAsync(e.MessageData.ToObject()).ConfigureAwait(false); - return; + case "Target.targetDestroyed": + await DestroyTargetAsync(e.MessageData.ToObject()).ConfigureAwait(false); + return; - case "Target.targetInfoChanged": - ChangeTargetInfo(e.MessageData.ToObject()); - return; + case "Target.targetInfoChanged": + ChangeTargetInfo(e.MessageData.ToObject()); + return; + } + } + catch (Exception ex) + { + var message = $"Browser failed to process {e.MessageID}. {ex.Message}. {ex.StackTrace}"; + _logger.LogError(ex, message); + Connection.Close(message); } } diff --git a/lib/PuppeteerSharp/CDPSession.cs b/lib/PuppeteerSharp/CDPSession.cs index 3b84b1a90..872be94b0 100755 --- a/lib/PuppeteerSharp/CDPSession.cs +++ b/lib/PuppeteerSharp/CDPSession.cs @@ -92,7 +92,10 @@ internal CDPSession(IConnection connection, TargetType targetType, string sessio /// /// true if is closed; otherwise, false. public bool IsClosed { get; internal set; } - + /// + /// Connection close reason. + /// + public string CloseReason { get; private set; } /// /// Gets the logger factory. /// @@ -133,7 +136,10 @@ public async Task SendAsync(string method, dynamic args = null, bool wa { if (Connection == null) { - throw new PuppeteerException($"Protocol error ({method}): Session closed. Most likely the {TargetType} has been closed."); + throw new PuppeteerException( + $"Protocol error ({method}): Session closed. " + + $"Most likely the {TargetType} has been closed." + + $"Close reason: {CloseReason}"); } var id = Interlocked.Increment(ref _lastId); var message = JsonConvert.SerializeObject(new Dictionary @@ -252,7 +258,7 @@ internal void OnMessage(string message) if (_sessions.TryRemove(sessionId, out var session)) { - session.OnClosed(); + session.Close("Target.detachedFromTarget"); } } @@ -264,24 +270,26 @@ internal void OnMessage(string message) } } - internal void OnClosed() + internal void Close(string closeReason) { if (IsClosed) { return; } + CloseReason = closeReason; IsClosed = true; foreach (var session in _sessions.Values.ToArray()) { - session.OnClosed(); + session.Close(closeReason); } _sessions.Clear(); foreach (var callback in _callbacks.Values.ToArray()) { callback.TaskWrapper.TrySetException(new TargetClosedException( - $"Protocol error({callback.Method}): Target closed." + $"Protocol error({callback.Method}): Target closed.", + closeReason )); } _callbacks.Clear(); @@ -303,6 +311,7 @@ internal CDPSession CreateSession(TargetType targetType, string sessionId) Task IConnection.SendAsync(string method, dynamic args, bool waitForCallback) => SendAsync(method, args, waitForCallback); IConnection IConnection.Connection => Connection; + void IConnection.Close(string closeReason) => Close(closeReason); #endregion } } \ No newline at end of file diff --git a/lib/PuppeteerSharp/Connection.cs b/lib/PuppeteerSharp/Connection.cs index baddbd24c..4c48fb15e 100644 --- a/lib/PuppeteerSharp/Connection.cs +++ b/lib/PuppeteerSharp/Connection.cs @@ -74,6 +74,11 @@ internal Connection(string url, int delay, IConnectionTransport transport, ILogg /// true if is closed; otherwise, false. public bool IsClosed { get; internal set; } + /// + /// Connection close reason. + /// + public string CloseReason { get; private set; } + /// /// Gets the logger factory. /// @@ -88,7 +93,7 @@ internal async Task SendAsync(string method, dynamic args = null, bool { if (IsClosed) { - throw new TargetClosedException($"Protocol error({method}): Target closed."); + throw new TargetClosedException($"Protocol error({method}): Target closed.", CloseReason); } var id = Interlocked.Increment(ref _lastId); @@ -137,27 +142,29 @@ internal async Task CreateSessionAsync(TargetInfo targetInfo) internal bool HasPendingCallbacks() => _callbacks.Count != 0; #endregion - private void OnClose() + internal void Close(string closeReason) { if (IsClosed) { return; } IsClosed = true; + CloseReason = closeReason; Transport.StopReading(); Closed?.Invoke(this, new EventArgs()); foreach (var session in _sessions.Values.ToArray()) { - session.OnClosed(); + session.Close(closeReason); } _sessions.Clear(); foreach (var response in _callbacks.Values.ToArray()) { response.TaskWrapper.TrySetException(new TargetClosedException( - $"Protocol error({response.Method}): Target closed." + $"Protocol error({response.Method}): Target closed.", + closeReason )); } _callbacks.Clear(); @@ -176,77 +183,86 @@ internal static IConnection FromSession(CDPSession session) private async void Transport_MessageReceived(object sender, MessageReceivedEventArgs e) { - var response = e.Message; - JObject obj = null; - - if (response.Length > 0 && Delay > 0) - { - await Task.Delay(Delay).ConfigureAwait(false); - } - try { - obj = JObject.Parse(response); - } - catch (JsonException exc) - { - _logger.LogError(exc, "Failed to deserialize response", response); - return; - } - - _logger.LogTrace("◀ Receive {Message}", response); - - var id = obj[MessageKeys.Id]?.Value(); + var response = e.Message; + JObject obj = null; - if (id.HasValue) - { - //If we get the object we are waiting for we return if - //if not we add this to the list, sooner or later some one will come for it - if (_callbacks.TryRemove(id.Value, out var callback)) + if (response.Length > 0 && Delay > 0) { - if (obj[MessageKeys.Error] != null) - { - callback.TaskWrapper.TrySetException(new MessageException(callback, obj)); - } - else - { - callback.TaskWrapper.TrySetResult(obj[MessageKeys.Result].Value()); - } + await Task.Delay(Delay).ConfigureAwait(false); } - } - else - { - var method = obj[MessageKeys.Method].AsString(); - var param = obj[MessageKeys.Params]; - if (method == "Target.receivedMessageFromTarget") + try { - var sessionId = param[MessageKeys.SessionId].AsString(); - if (_sessions.TryGetValue(sessionId, out var session)) - { - session.OnMessage(param[MessageKeys.Message].AsString()); - } + obj = JObject.Parse(response); } - else if (method == "Target.detachedFromTarget") + catch (JsonException exc) + { + _logger.LogError(exc, "Failed to deserialize response", response); + return; + } + + _logger.LogTrace("◀ Receive {Message}", response); + + var id = obj[MessageKeys.Id]?.Value(); + + if (id.HasValue) { - var sessionId = param[MessageKeys.SessionId].AsString(); - if (_sessions.TryRemove(sessionId, out var session) && !session.IsClosed) + //If we get the object we are waiting for we return if + //if not we add this to the list, sooner or later some one will come for it + if (_callbacks.TryRemove(id.Value, out var callback)) { - session.OnClosed(); + if (obj[MessageKeys.Error] != null) + { + callback.TaskWrapper.TrySetException(new MessageException(callback, obj)); + } + else + { + callback.TaskWrapper.TrySetResult(obj[MessageKeys.Result].Value()); + } } } else { - MessageReceived?.Invoke(this, new MessageEventArgs + var method = obj[MessageKeys.Method].AsString(); + var param = obj[MessageKeys.Params]; + + if (method == "Target.receivedMessageFromTarget") + { + var sessionId = param[MessageKeys.SessionId].AsString(); + if (_sessions.TryGetValue(sessionId, out var session)) + { + session.OnMessage(param[MessageKeys.Message].AsString()); + } + } + else if (method == "Target.detachedFromTarget") { - MessageID = method, - MessageData = param - }); + var sessionId = param[MessageKeys.SessionId].AsString(); + if (_sessions.TryRemove(sessionId, out var session) && !session.IsClosed) + { + session.Close("Target.detachedFromTarget"); + } + } + else + { + MessageReceived?.Invoke(this, new MessageEventArgs + { + MessageID = method, + MessageData = param + }); + } } } + catch (Exception ex) + { + var message = $"Connection failed to process {e.Message}. {ex.Message}. {ex.StackTrace}"; + _logger.LogError(ex, message); + Close(message); + } } - void Transport_Closed(object sender, EventArgs e) => OnClose(); + void Transport_Closed(object sender, TransportClosedEventArgs e) => Close(e.CloseReason); #endregion @@ -290,7 +306,7 @@ internal static async Task Create(string url, IConnectionOptions con /// was occupying. public void Dispose() { - OnClose(); + Close("Connection disposed"); Transport.Dispose(); } #endregion @@ -301,6 +317,7 @@ public void Dispose() Task IConnection.SendAsync(string method, dynamic args, bool waitForCallback) => SendAsync(method, args, waitForCallback); IConnection IConnection.Connection => null; + void IConnection.Close(string closeReason) => Close(closeReason); #endregion } } \ No newline at end of file diff --git a/lib/PuppeteerSharp/FrameManager.cs b/lib/PuppeteerSharp/FrameManager.cs index 5e198536a..60ea1dfd7 100644 --- a/lib/PuppeteerSharp/FrameManager.cs +++ b/lib/PuppeteerSharp/FrameManager.cs @@ -31,7 +31,7 @@ private FrameManager(CDPSession client, Page page, NetworkManager networkManager _networkManager = networkManager; _pendingFrameRequests = new MultiMap>(); - _client.MessageReceived += _client_MessageReceived; + _client.MessageReceived += Client_MessageReceived; } #region Properties @@ -154,47 +154,56 @@ public async Task WaitForFrameNavigationAsync(Frame frame, NavigationO #region Private Methods - private async void _client_MessageReceived(object sender, MessageEventArgs e) + private async void Client_MessageReceived(object sender, MessageEventArgs e) { - switch (e.MessageID) + try { - case "Page.frameAttached": - OnFrameAttached( - e.MessageData.SelectToken(MessageKeys.FrameId).ToObject(), - e.MessageData.SelectToken("parentFrameId").ToObject()); - break; - - case "Page.frameNavigated": - await OnFrameNavigatedAsync(e.MessageData.SelectToken(MessageKeys.Frame).ToObject()).ConfigureAwait(false); - break; - - case "Page.navigatedWithinDocument": - OnFrameNavigatedWithinDocument(e.MessageData.ToObject()); - break; - - case "Page.frameDetached": - OnFrameDetached(e.MessageData.ToObject()); - break; - - case "Page.frameStoppedLoading": - OnFrameStoppedLoading(e.MessageData.ToObject()); - break; - - case "Runtime.executionContextCreated": - await OnExecutionContextCreatedAsync(e.MessageData.SelectToken(MessageKeys.Context).ToObject()); - break; - - case "Runtime.executionContextDestroyed": - OnExecutionContextDestroyed(e.MessageData.SelectToken(MessageKeys.ExecutionContextId).ToObject()); - break; - case "Runtime.executionContextsCleared": - OnExecutionContextsCleared(); - break; - case "Page.lifecycleEvent": - OnLifeCycleEvent(e.MessageData.ToObject()); - break; - default: - break; + switch (e.MessageID) + { + case "Page.frameAttached": + OnFrameAttached( + e.MessageData.SelectToken(MessageKeys.FrameId).ToObject(), + e.MessageData.SelectToken("parentFrameId").ToObject()); + break; + + case "Page.frameNavigated": + await OnFrameNavigatedAsync(e.MessageData.SelectToken(MessageKeys.Frame).ToObject()).ConfigureAwait(false); + break; + + case "Page.navigatedWithinDocument": + OnFrameNavigatedWithinDocument(e.MessageData.ToObject()); + break; + + case "Page.frameDetached": + OnFrameDetached(e.MessageData.ToObject()); + break; + + case "Page.frameStoppedLoading": + OnFrameStoppedLoading(e.MessageData.ToObject()); + break; + + case "Runtime.executionContextCreated": + await OnExecutionContextCreatedAsync(e.MessageData.SelectToken(MessageKeys.Context).ToObject()); + break; + + case "Runtime.executionContextDestroyed": + OnExecutionContextDestroyed(e.MessageData.SelectToken(MessageKeys.ExecutionContextId).ToObject()); + break; + case "Runtime.executionContextsCleared": + OnExecutionContextsCleared(); + break; + case "Page.lifecycleEvent": + OnLifeCycleEvent(e.MessageData.ToObject()); + break; + default: + break; + } + } + catch (Exception ex) + { + var message = $"Connection failed to process {e.MessageID}. {ex.Message}. {ex.StackTrace}"; + _logger.LogError(ex, message); + _client.Close(message); } } diff --git a/lib/PuppeteerSharp/IConnection.cs b/lib/PuppeteerSharp/IConnection.cs index de97cc010..d290cae85 100644 --- a/lib/PuppeteerSharp/IConnection.cs +++ b/lib/PuppeteerSharp/IConnection.cs @@ -19,7 +19,11 @@ internal interface IConnection /// Gets a value indicating whether this is closed. /// /// true if is closed; otherwise, false. - bool IsClosed { get; } + bool IsClosed { get; } + /// + /// Connection close reason. + /// + string CloseReason { get; } /// /// Sends a message to chromium. /// @@ -38,6 +42,11 @@ internal interface IConnection /// /// Occurs when the connection is closed. /// - event EventHandler Closed; + event EventHandler Closed; + /// + /// Close the connection. + /// + /// Close reason. + void Close(string closeReason); } } \ No newline at end of file diff --git a/lib/PuppeteerSharp/NavigatorWatcher.cs b/lib/PuppeteerSharp/NavigatorWatcher.cs index 7c9555719..92b970b46 100755 --- a/lib/PuppeteerSharp/NavigatorWatcher.cs +++ b/lib/PuppeteerSharp/NavigatorWatcher.cs @@ -63,8 +63,9 @@ internal class NavigatorWatcher frameManager.FrameNavigatedWithinDocument += NavigatedWithinDocument; frameManager.FrameDetached += OnFrameDetached; networkManager.Request += OnRequest; - Connection.FromSession(client).Closed += (sender, e) - => Terminate(new TargetClosedException("Navigation failed because browser has disconnected!")); + var connection = Connection.FromSession(client); + connection.Closed += (sender, e) + => Terminate(new TargetClosedException("Navigation failed because browser has disconnected!", connection.CloseReason)); _sameDocumentNavigationTaskWrapper = new TaskCompletionSource(); _newDocumentNavigationTaskWrapper = new TaskCompletionSource(); diff --git a/lib/PuppeteerSharp/NetworkManager.cs b/lib/PuppeteerSharp/NetworkManager.cs index d91c0bf9f..9dcae010a 100644 --- a/lib/PuppeteerSharp/NetworkManager.cs +++ b/lib/PuppeteerSharp/NetworkManager.cs @@ -101,26 +101,35 @@ internal Task SetRequestInterceptionAsync(bool value) private async void Client_MessageReceived(object sender, MessageEventArgs e) { - switch (e.MessageID) + try { - case "Network.requestWillBeSent": - await OnRequestWillBeSentAsync(e.MessageData.ToObject()); - break; - case "Network.requestIntercepted": - await OnRequestInterceptedAsync(e.MessageData.ToObject()).ConfigureAwait(false); - break; - case "Network.requestServedFromCache": - OnRequestServedFromCache(e.MessageData.ToObject()); - break; - case "Network.responseReceived": - OnResponseReceived(e.MessageData.ToObject()); - break; - case "Network.loadingFinished": - OnLoadingFinished(e.MessageData.ToObject()); - break; - case "Network.loadingFailed": - OnLoadingFailed(e.MessageData.ToObject()); - break; + switch (e.MessageID) + { + case "Network.requestWillBeSent": + await OnRequestWillBeSentAsync(e.MessageData.ToObject()); + break; + case "Network.requestIntercepted": + await OnRequestInterceptedAsync(e.MessageData.ToObject()).ConfigureAwait(false); + break; + case "Network.requestServedFromCache": + OnRequestServedFromCache(e.MessageData.ToObject()); + break; + case "Network.responseReceived": + OnResponseReceived(e.MessageData.ToObject()); + break; + case "Network.loadingFinished": + OnLoadingFinished(e.MessageData.ToObject()); + break; + case "Network.loadingFailed": + OnLoadingFailed(e.MessageData.ToObject()); + break; + } + } + catch (Exception ex) + { + var message = $"NetworkManager failed to process {e.MessageID}. {ex.Message}. {ex.StackTrace}"; + _logger.LogError(ex, message); + _client.Close(message); } } diff --git a/lib/PuppeteerSharp/Page.cs b/lib/PuppeteerSharp/Page.cs index c8b3698d1..151bc537f 100644 --- a/lib/PuppeteerSharp/Page.cs +++ b/lib/PuppeteerSharp/Page.cs @@ -1767,44 +1767,53 @@ private decimal ConvertPrintParameterToInches(object parameter) private async void Client_MessageReceived(object sender, MessageEventArgs e) { - switch (e.MessageID) + try { - case "Page.domContentEventFired": - DOMContentLoaded?.Invoke(this, EventArgs.Empty); - break; - case "Page.loadEventFired": - Load?.Invoke(this, EventArgs.Empty); - break; - case "Runtime.consoleAPICalled": - await OnConsoleAPI(e.MessageData.ToObject()).ConfigureAwait(false); - break; - case "Page.javascriptDialogOpening": - OnDialog(e.MessageData.ToObject()); - break; - case "Runtime.exceptionThrown": - HandleException(e.MessageData.SelectToken(MessageKeys.ExceptionDetails).ToObject()); - break; - case "Security.certificateError": - await OnCertificateError(e.MessageData.ToObject()).ConfigureAwait(false); - break; - case "Inspector.targetCrashed": - OnTargetCrashed(); - break; - case "Performance.metrics": - EmitMetrics(e.MessageData.ToObject()); - break; - case "Target.attachedToTarget": - await OnAttachedToTarget(e).ConfigureAwait(false); - break; - case "Target.detachedFromTarget": - OnDetachedFromTarget(e); - break; - case "Log.entryAdded": - OnLogEntryAdded(e.MessageData.ToObject()); - break; - case "Runtime.bindingCalled": - await OnBindingCalled(e.MessageData.ToObject()).ConfigureAwait(false); - break; + switch (e.MessageID) + { + case "Page.domContentEventFired": + DOMContentLoaded?.Invoke(this, EventArgs.Empty); + break; + case "Page.loadEventFired": + Load?.Invoke(this, EventArgs.Empty); + break; + case "Runtime.consoleAPICalled": + await OnConsoleAPI(e.MessageData.ToObject()).ConfigureAwait(false); + break; + case "Page.javascriptDialogOpening": + OnDialog(e.MessageData.ToObject()); + break; + case "Runtime.exceptionThrown": + HandleException(e.MessageData.SelectToken(MessageKeys.ExceptionDetails).ToObject()); + break; + case "Security.certificateError": + await OnCertificateError(e.MessageData.ToObject()).ConfigureAwait(false); + break; + case "Inspector.targetCrashed": + OnTargetCrashed(); + break; + case "Performance.metrics": + EmitMetrics(e.MessageData.ToObject()); + break; + case "Target.attachedToTarget": + await OnAttachedToTarget(e).ConfigureAwait(false); + break; + case "Target.detachedFromTarget": + OnDetachedFromTarget(e); + break; + case "Log.entryAdded": + OnLogEntryAdded(e.MessageData.ToObject()); + break; + case "Runtime.bindingCalled": + await OnBindingCalled(e.MessageData.ToObject()).ConfigureAwait(false); + break; + } + } + catch (Exception ex) + { + var message = $"Page failed to process {e.MessageID}. {ex.Message}. {ex.StackTrace}"; + _logger.LogError(ex, message); + Client.Close(message); } } diff --git a/lib/PuppeteerSharp/PageCoverage/CSSCoverage.cs b/lib/PuppeteerSharp/PageCoverage/CSSCoverage.cs index ec76f97e8..c0fc392f6 100755 --- a/lib/PuppeteerSharp/PageCoverage/CSSCoverage.cs +++ b/lib/PuppeteerSharp/PageCoverage/CSSCoverage.cs @@ -40,7 +40,7 @@ internal Task StartAsync(CoverageStartOptions options) _stylesheetURLs.Clear(); _stylesheetSources.Clear(); - _client.MessageReceived += client_MessageReceived; + _client.MessageReceived += Client_MessageReceived; return Task.WhenAll( _client.SendAsync("DOM.enable"), @@ -62,7 +62,7 @@ internal async Task StopAsync() _client.SendAsync("CSS.disable"), _client.SendAsync("DOM.disable") ).ConfigureAwait(false); - _client.MessageReceived -= client_MessageReceived; + _client.MessageReceived -= Client_MessageReceived; var styleSheetIdToCoverage = new Dictionary>(); foreach (var entry in trackingResponse.RuleUsage) @@ -98,16 +98,25 @@ internal async Task StopAsync() return coverage.ToArray(); } - private async void client_MessageReceived(object sender, MessageEventArgs e) + private async void Client_MessageReceived(object sender, MessageEventArgs e) { - switch (e.MessageID) + try + { + switch (e.MessageID) + { + case "CSS.styleSheetAdded": + await OnStyleSheetAdded(e.MessageData.ToObject()).ConfigureAwait(false); + break; + case "Runtime.executionContextsCleared": + OnExecutionContextsCleared(); + break; + } + } + catch (Exception ex) { - case "CSS.styleSheetAdded": - await OnStyleSheetAdded(e.MessageData.ToObject()).ConfigureAwait(false); - break; - case "Runtime.executionContextsCleared": - OnExecutionContextsCleared(); - break; + var message = $"CSSCoverage failed to process {e.MessageID}. {ex.Message}. {ex.StackTrace}"; + _logger.LogError(ex, message); + _client.Close(message); } } diff --git a/lib/PuppeteerSharp/PageCoverage/JSCoverage.cs b/lib/PuppeteerSharp/PageCoverage/JSCoverage.cs index bbbc8cd1a..79042da83 100755 --- a/lib/PuppeteerSharp/PageCoverage/JSCoverage.cs +++ b/lib/PuppeteerSharp/PageCoverage/JSCoverage.cs @@ -43,7 +43,7 @@ internal Task StartAsync(CoverageStartOptions options) _scriptURLs.Clear(); _scriptSources.Clear(); - _client.MessageReceived += client_MessageReceived; + _client.MessageReceived += Client_MessageReceived; return Task.WhenAll( _client.SendAsync("Profiler.enable"), @@ -68,7 +68,7 @@ internal async Task StopAsync() _client.SendAsync("Profiler.disable"), _client.SendAsync("Debugger.disable") ).ConfigureAwait(false); - _client.MessageReceived -= client_MessageReceived; + _client.MessageReceived -= Client_MessageReceived; var coverage = new List(); foreach (var entry in profileResponseTask.Result.Result) @@ -96,16 +96,25 @@ internal async Task StopAsync() return coverage.ToArray(); } - private async void client_MessageReceived(object sender, MessageEventArgs e) + private async void Client_MessageReceived(object sender, MessageEventArgs e) { - switch (e.MessageID) + try + { + switch (e.MessageID) + { + case "Debugger.scriptParsed": + await OnScriptParsed(e.MessageData.ToObject()).ConfigureAwait(false); + break; + case "Runtime.executionContextsCleared": + OnExecutionContextsCleared(); + break; + } + } + catch (Exception ex) { - case "Debugger.scriptParsed": - await OnScriptParsed(e.MessageData.ToObject()).ConfigureAwait(false); - break; - case "Runtime.executionContextsCleared": - OnExecutionContextsCleared(); - break; + var message = $"JSCoverage failed to process {e.MessageID}. {ex.Message}. {ex.StackTrace}"; + _logger.LogError(ex, message); + _client.Close(message); } } diff --git a/lib/PuppeteerSharp/TargetClosedException.cs b/lib/PuppeteerSharp/TargetClosedException.cs index 10811b0d8..5179ce29b 100644 --- a/lib/PuppeteerSharp/TargetClosedException.cs +++ b/lib/PuppeteerSharp/TargetClosedException.cs @@ -5,12 +5,17 @@ /// public class TargetClosedException : PuppeteerException { + /// + /// Close Reason. + /// + /// The close reason. + public string CloseReason { get; } + /// /// Initializes a new instance of the class. /// /// Message. - public TargetClosedException(string message) : base(message) - { - } + /// Close reason. + public TargetClosedException(string message, string closeReason) : base(message) => CloseReason = closeReason; } } \ No newline at end of file diff --git a/lib/PuppeteerSharp/Tracing.cs b/lib/PuppeteerSharp/Tracing.cs index 2a63712d9..6bed01a65 100755 --- a/lib/PuppeteerSharp/Tracing.cs +++ b/lib/PuppeteerSharp/Tracing.cs @@ -3,6 +3,7 @@ using System.IO; using System.Text; using System.Threading.Tasks; +using Microsoft.Extensions.Logging; using PuppeteerSharp.Messaging; namespace PuppeteerSharp @@ -39,11 +40,13 @@ public class Tracing "latencyInfo", "disabled-by-default-devtools.timeline.stack", "disabled-by-default-v8.cpu_profiler" - }; + }; + private readonly ILogger _logger; internal Tracing(CDPSession client) { - _client = client; + _client = client; + _logger = client.LoggerFactory.CreateLogger(); } /// @@ -85,9 +88,18 @@ public async Task StopAsync() async void EventHandler(object sender, TracingCompleteEventArgs e) { - var tracingData = await ReadStream(e.Stream, _path).ConfigureAwait(false); - _client.TracingComplete -= EventHandler; - taskWrapper.SetResult(tracingData); + try + { + var tracingData = await ReadStream(e.Stream, _path).ConfigureAwait(false); + _client.TracingComplete -= EventHandler; + taskWrapper.SetResult(tracingData); + } + catch (Exception ex) + { + var message = $"Tracing failed to process the tracing complete. {ex.Message}. {ex.StackTrace}"; + _logger.LogError(ex, message); + _client.Close(message); + } } _client.TracingComplete += EventHandler; diff --git a/lib/PuppeteerSharp/Transport/IConnectionTransport.cs b/lib/PuppeteerSharp/Transport/IConnectionTransport.cs index 826820769..5d2457545 100644 --- a/lib/PuppeteerSharp/Transport/IConnectionTransport.cs +++ b/lib/PuppeteerSharp/Transport/IConnectionTransport.cs @@ -26,7 +26,7 @@ public interface IConnectionTransport : IDisposable /// /// Occurs when the transport is closed. /// - event EventHandler Closed; + event EventHandler Closed; /// /// Occurs when a message is received. /// diff --git a/lib/PuppeteerSharp/Transport/TransportClosedEventArgs.cs b/lib/PuppeteerSharp/Transport/TransportClosedEventArgs.cs new file mode 100644 index 000000000..269633a5e --- /dev/null +++ b/lib/PuppeteerSharp/Transport/TransportClosedEventArgs.cs @@ -0,0 +1,19 @@ +using System; +namespace PuppeteerSharp.Transport +{ + /// + /// + /// + public class TransportClosedEventArgs : EventArgs + { + /// + /// Gets or sets the close reason. + /// + public string CloseReason { get; set; } + /// + /// Initializes a new instance of the class. + /// + /// Close reason. + public TransportClosedEventArgs(string closeReason) => CloseReason = closeReason; + } +} diff --git a/lib/PuppeteerSharp/Transport/WebSocketTransport.cs b/lib/PuppeteerSharp/Transport/WebSocketTransport.cs index 3b83a21bf..8eb847928 100644 --- a/lib/PuppeteerSharp/Transport/WebSocketTransport.cs +++ b/lib/PuppeteerSharp/Transport/WebSocketTransport.cs @@ -24,7 +24,7 @@ public class WebSocketTransport : IConnectionTransport /// /// Occurs when the transport is closed. /// - public event EventHandler Closed; + public event EventHandler Closed; /// /// Occurs when a message is received. /// @@ -77,7 +77,7 @@ private async Task GetResponseAsync() { if (IsClosed) { - OnClose(); + OnClose("WebSocket is closed"); return null; } @@ -97,9 +97,9 @@ private async Task GetResponseAsync() { return null; } - catch (Exception) + catch (Exception ex) { - OnClose(); + OnClose(ex.Message); return null; } @@ -111,7 +111,7 @@ private async Task GetResponseAsync() } else if (result.MessageType == WebSocketMessageType.Close) { - OnClose(); + OnClose("WebSocket closed"); return null; } } @@ -120,9 +120,9 @@ private async Task GetResponseAsync() } } - private void OnClose() + private void OnClose(string closeReason) { - Closed?.Invoke(this, EventArgs.Empty); + Closed?.Invoke(this, new TransportClosedEventArgs(closeReason)); IsClosed = true; } diff --git a/lib/PuppeteerSharp/Worker.cs b/lib/PuppeteerSharp/Worker.cs index 0c2135df5..6f87daad7 100644 --- a/lib/PuppeteerSharp/Worker.cs +++ b/lib/PuppeteerSharp/Worker.cs @@ -98,17 +98,26 @@ public async Task EvaluateExpressionHandleAsync(string script) internal async void OnMessageReceived(object sender, MessageEventArgs e) { - switch (e.MessageID) + try { - case "Runtime.executionContextCreated": - OnExecutionContextCreated(e); - break; - case "Runtime.consoleAPICalled": - await OnConsoleAPICalled(e).ConfigureAwait(false); - break; - case "Runtime.exceptionThrown": - OnExceptionThrown(e); - break; + switch (e.MessageID) + { + case "Runtime.executionContextCreated": + OnExecutionContextCreated(e); + break; + case "Runtime.consoleAPICalled": + await OnConsoleAPICalled(e).ConfigureAwait(false); + break; + case "Runtime.exceptionThrown": + OnExceptionThrown(e); + break; + } + } + catch (Exception ex) + { + var message = $"Worker failed to process {e.MessageID}. {ex.Message}. {ex.StackTrace}"; + _logger.LogError(ex, message); + _client.Close(message); } }