You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by GitBox <gi...@apache.org> on 2020/09/02 13:09:19 UTC

[GitHub] [pulsar-dotpulsar] lee-chrisbell opened a new pull request #52: Eat errors in SetupChannel

lee-chrisbell opened a new pull request #52:
URL: https://github.com/apache/pulsar-dotpulsar/pull/52


   Errors thrown in an async void method cannot be caught, resulting in application crashes in some cases.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-dotpulsar] lee-chrisbell commented on a change in pull request #52: Eat errors in SetupChannel

Posted by GitBox <gi...@apache.org>.
lee-chrisbell commented on a change in pull request #52:
URL: https://github.com/apache/pulsar-dotpulsar/pull/52#discussion_r482066998



##########
File path: src/DotPulsar/Internal/ConsumerProcess.cs
##########
@@ -84,8 +84,15 @@ protected override void CalculateState()
 
         private async void SetupChannel()
         {
-            var channel = await _factory.Create(CancellationTokenSource.Token).ConfigureAwait(false);
-            await _consumer.SetChannel(channel).ConfigureAwait(false);
+            try
+            {
+                var channel = await _factory.Create(CancellationTokenSource.Token).ConfigureAwait(false);
+                await _consumer.SetChannel(channel).ConfigureAwait(false);
+            }
+            catch (Exception ex)
+            {
+                Console.WriteLine($"Setup Channel failed for Consumer \n {ex.ToString()}");

Review comment:
       Yeah, good point. Changed it.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-dotpulsar] blankensteiner commented on a change in pull request #52: Eat errors in SetupChannel

Posted by GitBox <gi...@apache.org>.
blankensteiner commented on a change in pull request #52:
URL: https://github.com/apache/pulsar-dotpulsar/pull/52#discussion_r482060380



##########
File path: src/DotPulsar/Internal/ConsumerProcess.cs
##########
@@ -84,8 +84,15 @@ protected override void CalculateState()
 
         private async void SetupChannel()
         {
-            var channel = await _factory.Create(CancellationTokenSource.Token).ConfigureAwait(false);
-            await _consumer.SetChannel(channel).ConfigureAwait(false);
+            try
+            {
+                var channel = await _factory.Create(CancellationTokenSource.Token).ConfigureAwait(false);
+                await _consumer.SetChannel(channel).ConfigureAwait(false);
+            }
+            catch (Exception ex)
+            {
+                Console.WriteLine($"Setup Channel failed for Consumer \n {ex.ToString()}");

Review comment:
       Let's remove the output to the console here. We don't know if there even is a console and the exception thrown has already been through the exception handling pipeline (and outputted if that is what the user wanted).
   So, we can have an empty catch.
   ```csharp
   catch
   {
       // ignored
   }
   ```
   The same goes for the ProducerProcess and we also need this for the ReaderProcess.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-dotpulsar] blankensteiner merged pull request #52: Eat errors in SetupChannel

Posted by GitBox <gi...@apache.org>.
blankensteiner merged pull request #52:
URL: https://github.com/apache/pulsar-dotpulsar/pull/52


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org