Avoid unhandled exceptions on unawaited async path (#2289)

jnyrup · web-flow · commit 0d03855d249c · 2023-08-23T13:55:06.000-03:00
* Avoid unhandled exceptions on unawaited async path

* Add "Async" suffix to Task returning methods
diff --git a/lib/PuppeteerSharp/ChromeTargetManager.cs b/lib/PuppeteerSharp/ChromeTargetManager.cs
@@ -129,7 +129,7 @@ private void StoreExistingTargetsForInit()
             }
         }
 
-        private async Task EnsureTargetsIdsForInit()
+        private async Task EnsureTargetsIdsForInitAsync()
         {
             if (_targetDiscoveryTimeout > 0)
             {
@@ -160,7 +160,7 @@ private void OnMessageReceived(object sender, MessageEventArgs e)
                         return;
 
                     case "Target.targetDestroyed":
-                        OnTargetDestroyed(e.MessageData.ToObject<TargetDestroyedResponse>(true));
+                        _ = OnTargetDestroyedAsync(e.MessageID, e.MessageData.ToObject<TargetDestroyedResponse>(true));
                         return;
 
                     case "Target.targetInfoChanged":
@@ -198,15 +198,22 @@ private void OnTargetCreated(TargetCreatedResponse e)
             }
         }
 
-        private async void OnTargetDestroyed(TargetDestroyedResponse e)
+        private async Task OnTargetDestroyedAsync(string messageId, TargetDestroyedResponse e)
         {
-            _discoveredTargetsByTargetId.TryRemove(e.TargetId, out var targetInfo);
-            await EnsureTargetsIdsForInit().ConfigureAwait(false);
-            FinishInitializationIfReady(e.TargetId);
+            try
+            {
+                _discoveredTargetsByTargetId.TryRemove(e.TargetId, out var targetInfo);
+                await EnsureTargetsIdsForInitAsync().ConfigureAwait(false);
+                FinishInitializationIfReady(e.TargetId);
 
-            if (targetInfo?.Type == TargetType.ServiceWorker && _availableTargetsByTargetIdDictionary.TryRemove(e.TargetId, out var target))
+                if (targetInfo?.Type == TargetType.ServiceWorker && _availableTargetsByTargetIdDictionary.TryRemove(e.TargetId, out var target))
+                {
+                    TargetGone?.Invoke(this, new TargetChangedArgs { Target = target, TargetInfo = targetInfo });
+                }
+            }
+            catch (Exception ex)
             {
-                TargetGone?.Invoke(this, new TargetChangedArgs { Target = target, TargetInfo = targetInfo });
+                HandleExceptionOnMessageReceived(messageId, ex);
             }
         }
 
@@ -224,7 +231,7 @@ private void OnTargetInfoChanged(TargetCreatedResponse e)
             TargetChanged?.Invoke(this, new TargetChangedArgs { Target = target, TargetInfo = e.TargetInfo });
         }
 
-        private async Task OnAttachedToTarget(object sender, TargetAttachedToTargetResponse e)
+        private async Task OnAttachedToTargetAsync(object sender, TargetAttachedToTargetResponse e)
         {
             var parent = sender as ICDPConnection;
             var targetInfo = e.TargetInfo;
@@ -256,7 +263,7 @@ await parent.SendAsync(
             if (targetInfo.Type == TargetType.ServiceWorker &&
                 _connection.IsAutoAttached(targetInfo.TargetId))
             {
-                await EnsureTargetsIdsForInit().ConfigureAwait(false);
+                await EnsureTargetsIdsForInitAsync().ConfigureAwait(false);
                 FinishInitializationIfReady(targetInfo.TargetId);
                 await SilentDetach().ConfigureAwait(false);
                 if (_availableTargetsByTargetIdDictionary.ContainsKey(targetInfo.TargetId))
@@ -273,7 +280,7 @@ await parent.SendAsync(
             if (_targetFilterFunc?.Invoke(targetInfo) == false)
             {
                 _ignoredTargets.Add(targetInfo.TargetId);
-                await EnsureTargetsIdsForInit().ConfigureAwait(false);
+                await EnsureTargetsIdsForInitAsync().ConfigureAwait(false);
                 FinishInitializationIfReady(targetInfo.TargetId);
                 await SilentDetach().ConfigureAwait(false);
                 return;
@@ -311,7 +318,7 @@ await parent.SendAsync(
                 }
             }
 
-            await EnsureTargetsIdsForInit().ConfigureAwait(false);
+            await EnsureTargetsIdsForInitAsync().ConfigureAwait(false);
             _targetsIdsForInit.Remove(target.TargetId);
 
             if (!existingTarget)
@@ -342,7 +349,7 @@ private async Task OnAttachedToTargetHandlingExceptionsAsync(object sender, stri
         {
             try
             {
-                await OnAttachedToTarget(sender, e).ConfigureAwait(false);
+                await OnAttachedToTargetAsync(sender, e).ConfigureAwait(false);
             }
             catch (Exception ex)
             {
diff --git a/lib/PuppeteerSharp/IsolatedWorld.cs b/lib/PuppeteerSharp/IsolatedWorld.cs
@@ -448,7 +448,7 @@ private async void Client_MessageReceived(object sender, MessageEventArgs e)
                 switch (e.MessageID)
                 {
                     case "Runtime.bindingCalled":
-                        await OnBindingCalled(e.MessageData.ToObject<BindingCalledResponse>(true)).ConfigureAwait(false);
+                        await OnBindingCalledAsync(e.MessageData.ToObject<BindingCalledResponse>(true)).ConfigureAwait(false);
                         break;
                 }
             }
@@ -460,7 +460,7 @@ private async void Client_MessageReceived(object sender, MessageEventArgs e)
             }
         }
 
-        private async Task OnBindingCalled(BindingCalledResponse e)
+        private async Task OnBindingCalledAsync(BindingCalledResponse e)
         {
             var payload = e.BindingPayload;
 
diff --git a/lib/PuppeteerSharp/Page.cs b/lib/PuppeteerSharp/Page.cs
@@ -329,9 +329,9 @@ public Task<IJSHandle> QuerySelectorAllHandleAsync(string selector)
             => MainFrame.QuerySelectorAllHandleAsync(selector);
 
         /// <inheritdoc/>
-        #pragma warning disable CS0618 // Using obsolets
+#pragma warning disable CS0618 // Using obsolets
         public Task<IElementHandle[]> XPathAsync(string expression) => MainFrame.XPathAsync(expression);
-        #pragma warning restore CS0618
+#pragma warning restore CS0618
 
         /// <inheritdoc/>
         public async Task<IJSHandle> EvaluateExpressionHandleAsync(string script)
@@ -784,10 +784,10 @@ public Task<IElementHandle> WaitForSelectorAsync(string selector, WaitForSelecto
             => MainFrame.WaitForSelectorAsync(selector, options ?? new WaitForSelectorOptions());
 
         /// <inheritdoc/>
-        #pragma warning disable CS0618 // WaitForXPathAsync is obsolete
+#pragma warning disable CS0618 // WaitForXPathAsync is obsolete
         public Task<IElementHandle> WaitForXPathAsync(string xpath, WaitForSelectorOptions options = null)
             => MainFrame.WaitForXPathAsync(xpath, options ?? new WaitForSelectorOptions());
-        #pragma warning restore CS0618
+#pragma warning restore CS0618
 
         /// <inheritdoc/>
         public Task<IResponse> WaitForNavigationAsync(NavigationOptions options = null)
@@ -1470,7 +1470,7 @@ private async void Client_MessageReceived(object sender, MessageEventArgs e)
                         await OnLogEntryAddedAsync(e.MessageData.ToObject<LogEntryAddedResponse>(true)).ConfigureAwait(false);
                         break;
                     case "Runtime.bindingCalled":
-                        await OnBindingCalled(e.MessageData.ToObject<BindingCalledResponse>(true)).ConfigureAwait(false);
+                        await OnBindingCalledAsync(e.MessageData.ToObject<BindingCalledResponse>(true)).ConfigureAwait(false);
                         break;
                     case "Page.fileChooserOpened":
                         await OnFileChooserAsync(e.MessageData.ToObject<PageFileChooserOpenedResponse>(true)).ConfigureAwait(false);
@@ -1519,7 +1519,7 @@ private async Task OnFileChooserAsync(PageFileChooserOpenedResponse e)
             }
         }
 
-        private async Task OnBindingCalled(BindingCalledResponse e)
+        private async Task OnBindingCalledAsync(BindingCalledResponse e)
         {
             if (e.BindingPayload.Type != "exposedFun" || !_bindings.ContainsKey(e.BindingPayload.Name))
             {
diff --git a/lib/PuppeteerSharp/Worker.cs b/lib/PuppeteerSharp/Worker.cs
@@ -138,7 +138,7 @@ internal async void OnMessageReceived(object sender, MessageEventArgs e)
                         OnExecutionContextCreated(e.MessageData.ToObject<RuntimeExecutionContextCreatedResponse>(true));
                         break;
                     case "Runtime.consoleAPICalled":
-                        await OnConsoleAPICalled(e).ConfigureAwait(false);
+                        await OnConsoleAPICalledAsync(e).ConfigureAwait(false);
                         break;
                     case "Runtime.exceptionThrown":
                         OnExceptionThrown(e.MessageData.ToObject<RuntimeExceptionThrownResponse>(true));
@@ -155,7 +155,7 @@ internal async void OnMessageReceived(object sender, MessageEventArgs e)
 
         private void OnExceptionThrown(RuntimeExceptionThrownResponse e) => _exceptionThrown(e.ExceptionDetails);
 
-        private async Task OnConsoleAPICalled(MessageEventArgs e)
+        private async Task OnConsoleAPICalledAsync(MessageEventArgs e)
         {
             var consoleData = e.MessageData.ToObject<PageConsoleResponse>(true);
             await _consoleAPICalled(

Original file line number	Diff line number	Diff line change
`@@ -129,7 +129,7 @@ private void StoreExistingTargetsForInit()`
`129`	`129`	`}`
`130`	`130`	`}`
`131`	`131`
`132`		`- private async Task EnsureTargetsIdsForInit()`
	`132`	`+ private async Task EnsureTargetsIdsForInitAsync()`
`133`	`133`	`{`
`134`	`134`	`if (_targetDiscoveryTimeout > 0)`
`135`	`135`	`{`
`@@ -160,7 +160,7 @@ private void OnMessageReceived(object sender, MessageEventArgs e)`
`160`	`160`	`return;`
`161`	`161`
`162`	`162`	`case "Target.targetDestroyed":`
`163`		`- OnTargetDestroyed(e.MessageData.ToObject<TargetDestroyedResponse>(true));`
	`163`	`+ _ = OnTargetDestroyedAsync(e.MessageID, e.MessageData.ToObject<TargetDestroyedResponse>(true));`
`164`	`164`	`return;`
`165`	`165`
`166`	`166`	`case "Target.targetInfoChanged":`
`@@ -198,15 +198,22 @@ private void OnTargetCreated(TargetCreatedResponse e)`
`198`	`198`	`}`
`199`	`199`	`}`
`200`	`200`
`201`		`- private async void OnTargetDestroyed(TargetDestroyedResponse e)`
	`201`	`+ private async Task OnTargetDestroyedAsync(string messageId, TargetDestroyedResponse e)`
`202`	`202`	`{`
`203`		`- _discoveredTargetsByTargetId.TryRemove(e.TargetId, out var targetInfo);`
`204`		`- await EnsureTargetsIdsForInit().ConfigureAwait(false);`
`205`		`- FinishInitializationIfReady(e.TargetId);`
	`203`	`+ try`
	`204`	`+ {`
	`205`	`+ _discoveredTargetsByTargetId.TryRemove(e.TargetId, out var targetInfo);`
	`206`	`+ await EnsureTargetsIdsForInitAsync().ConfigureAwait(false);`
	`207`	`+ FinishInitializationIfReady(e.TargetId);`
`206`	`208`
`207`		`- if (targetInfo?.Type == TargetType.ServiceWorker && _availableTargetsByTargetIdDictionary.TryRemove(e.TargetId, out var target))`
	`209`	`+ if (targetInfo?.Type == TargetType.ServiceWorker && _availableTargetsByTargetIdDictionary.TryRemove(e.TargetId, out var target))`
	`210`	`+ {`
	`211`	`+ TargetGone?.Invoke(this, new TargetChangedArgs { Target = target, TargetInfo = targetInfo });`
	`212`	`+ }`
	`213`	`+ }`
	`214`	`+ catch (Exception ex)`
`208`	`215`	`{`
`209`		`- TargetGone?.Invoke(this, new TargetChangedArgs { Target = target, TargetInfo = targetInfo });`
	`216`	`+ HandleExceptionOnMessageReceived(messageId, ex);`
`210`	`217`	`}`
`211`	`218`	`}`
`212`	`219`
`@@ -224,7 +231,7 @@ private void OnTargetInfoChanged(TargetCreatedResponse e)`
`224`	`231`	`TargetChanged?.Invoke(this, new TargetChangedArgs { Target = target, TargetInfo = e.TargetInfo });`
`225`	`232`	`}`
`226`	`233`
`227`		`- private async Task OnAttachedToTarget(object sender, TargetAttachedToTargetResponse e)`
	`234`	`+ private async Task OnAttachedToTargetAsync(object sender, TargetAttachedToTargetResponse e)`
`228`	`235`	`{`
`229`	`236`	`var parent = sender as ICDPConnection;`
`230`	`237`	`var targetInfo = e.TargetInfo;`
`@@ -256,7 +263,7 @@ await parent.SendAsync(`
`256`	`263`	`if (targetInfo.Type == TargetType.ServiceWorker &&`
`257`	`264`	`_connection.IsAutoAttached(targetInfo.TargetId))`
`258`	`265`	`{`
`259`		`- await EnsureTargetsIdsForInit().ConfigureAwait(false);`
	`266`	`+ await EnsureTargetsIdsForInitAsync().ConfigureAwait(false);`
`260`	`267`	`FinishInitializationIfReady(targetInfo.TargetId);`
`261`	`268`	`await SilentDetach().ConfigureAwait(false);`
`262`	`269`	`if (_availableTargetsByTargetIdDictionary.ContainsKey(targetInfo.TargetId))`
`@@ -273,7 +280,7 @@ await parent.SendAsync(`
`273`	`280`	`if (_targetFilterFunc?.Invoke(targetInfo) == false)`
`274`	`281`	`{`
`275`	`282`	`_ignoredTargets.Add(targetInfo.TargetId);`
`276`		`- await EnsureTargetsIdsForInit().ConfigureAwait(false);`
	`283`	`+ await EnsureTargetsIdsForInitAsync().ConfigureAwait(false);`
`277`	`284`	`FinishInitializationIfReady(targetInfo.TargetId);`
`278`	`285`	`await SilentDetach().ConfigureAwait(false);`
`279`	`286`	`return;`
`@@ -311,7 +318,7 @@ await parent.SendAsync(`
`311`	`318`	`}`
`312`	`319`	`}`
`313`	`320`
`314`		`- await EnsureTargetsIdsForInit().ConfigureAwait(false);`
	`321`	`+ await EnsureTargetsIdsForInitAsync().ConfigureAwait(false);`
`315`	`322`	`_targetsIdsForInit.Remove(target.TargetId);`
`316`	`323`
`317`	`324`	`if (!existingTarget)`
`@@ -342,7 +349,7 @@ private async Task OnAttachedToTargetHandlingExceptionsAsync(object sender, stri`
`342`	`349`	`{`
`343`	`350`	`try`
`344`	`351`	`{`
`345`		`- await OnAttachedToTarget(sender, e).ConfigureAwait(false);`
	`352`	`+ await OnAttachedToTargetAsync(sender, e).ConfigureAwait(false);`
`346`	`353`	`}`
`347`	`354`	`catch (Exception ex)`
`348`	`355`	`{`
Original file line number	Diff line number	Diff line change
`@@ -448,7 +448,7 @@ private async void Client_MessageReceived(object sender, MessageEventArgs e)`
`448`	`448`	`switch (e.MessageID)`
`449`	`449`	`{`
`450`	`450`	`case "Runtime.bindingCalled":`
`451`		`- await OnBindingCalled(e.MessageData.ToObject<BindingCalledResponse>(true)).ConfigureAwait(false);`
	`451`	`+ await OnBindingCalledAsync(e.MessageData.ToObject<BindingCalledResponse>(true)).ConfigureAwait(false);`
`452`	`452`	`break;`
`453`	`453`	`}`
`454`	`454`	`}`
`@@ -460,7 +460,7 @@ private async void Client_MessageReceived(object sender, MessageEventArgs e)`
`460`	`460`	`}`
`461`	`461`	`}`
`462`	`462`
`463`		`- private async Task OnBindingCalled(BindingCalledResponse e)`
	`463`	`+ private async Task OnBindingCalledAsync(BindingCalledResponse e)`
`464`	`464`	`{`
`465`	`465`	`var payload = e.BindingPayload;`
`466`	`466`