CNX-9303 define a top level error handling strategy for all connector…

…s most likely in the i bridge (#3439) * Catch TargetInvocationException on bridge * Catch AggregateException for async calls * Extract ReportUnhandledError method * Be more specific on binding methods and catch unhandled expections on the bridge * Remove unused namespaces * SpeckleSendFilterException inherit from SpeckleException * Simplify general error handling on bridge * Simplify error message
specklesystems · May 24, 2024 · 8fcf59a · 8fcf59a
1 parent 034f2e9
commit 8fcf59a
Show file tree

Hide file tree

Showing 12 changed files with 125 additions and 73 deletions.
diff --git a/DUI3-DX/Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Bindings/ArcGISReceiveBinding.cs b/DUI3-DX/Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Bindings/ArcGISReceiveBinding.cs
@@ -6,7 +6,6 @@
 using Speckle.Connectors.Utils;
 using Speckle.Connectors.Utils.Cancellation;
 using Speckle.Connectors.Utils.Operations;
-using Speckle.Core.Logging;
 using ICancelable = System.Reactive.Disposables.ICancelable;
 
 namespace Speckle.Connectors.ArcGIS.Bindings;
@@ -39,15 +38,16 @@ public async Task Receive(string modelCardId)
   {
     try
     {
-      // Init cancellation token source -> Manager also cancel it if exist before
-      CancellationTokenSource cts = _cancellationManager.InitCancellationTokenSource(modelCardId);
-
       // Get receiver card
       if (_store.GetModelById(modelCardId) is not ReceiverModelCard modelCard)
       {
+        // Handle as GLOBAL ERROR at BrowserBridge
         throw new InvalidOperationException("No download model card was found.");
       }
 
+      // Init cancellation token source -> Manager also cancel it if exist before
+      CancellationTokenSource cts = _cancellationManager.InitCancellationTokenSource(modelCardId);
+
       using IUnitOfWork<ReceiveOperation> unitOfWork = _unitOfWorkFactory.Resolve<ReceiveOperation>();
 
       // Receive host objects
@@ -65,9 +65,11 @@ public async Task Receive(string modelCardId)
 
       Commands.SetModelReceiveResult(modelCardId, receivedObjectIds.ToList());
     }
-    catch (Exception e) when (!e.IsFatal()) // All exceptions should be handled here if possible, otherwise we enter "crashing the host app" territory.
+    // Catch here specific exceptions if they related to model card.
+    catch (OperationCanceledException)
     {
-      Commands.SetModelError(modelCardId, e);
+      // SWALLOW -> UI handles it immediately, so we do not need to handle anything
+      return;
     }
   }
 

diff --git a/DUI3-DX/Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Bindings/ArcGISSendBinding.cs b/DUI3-DX/Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Bindings/ArcGISSendBinding.cs
@@ -3,19 +3,19 @@
 using Speckle.Connectors.DUI.Bindings;
 using Speckle.Connectors.DUI.Bridge;
 using Speckle.Connectors.Utils.Cancellation;
-using Speckle.Core.Logging;
 using ICancelable = System.Reactive.Disposables.ICancelable;
 using Speckle.Connectors.DUI.Models;
 using Speckle.Connectors.DUI.Models.Card;
 using Speckle.Connectors.DUI.Models.Card.SendFilter;
 using Speckle.Connectors.DUI.Settings;
-using Speckle.Connectors.Utils;
 using ArcGIS.Desktop.Mapping.Events;
 using ArcGIS.Desktop.Mapping;
 using Speckle.Connectors.ArcGIS.Filters;
 using ArcGIS.Desktop.Editing.Events;
 using ArcGIS.Desktop.Framework.Threading.Tasks;
 using ArcGIS.Core.Data;
+using Speckle.Connectors.DUI.Exceptions;
+using Speckle.Connectors.Utils;
 using Speckle.Connectors.Utils.Operations;
 using Speckle.Core.Models;
 
@@ -258,15 +258,15 @@ public async Task Send(string modelCardId)
     using IUnitOfWork<SendOperation<MapMember>> unitOfWork = _unitOfWorkFactory.Resolve<SendOperation<MapMember>>();
     try
     {
-      // 0 - Init cancellation token source -> Manager also cancel it if exist before
-      CancellationTokenSource cts = _cancellationManager.InitCancellationTokenSource(modelCardId);
-
-      // 1 - Get model
       if (_store.GetModelById(modelCardId) is not SenderModelCard modelCard)
       {
+        // Handle as GLOBAL ERROR at BrowserBridge
         throw new InvalidOperationException("No publish model card was found.");
       }
 
+      // Init cancellation token source -> Manager also cancel it if exist before
+      CancellationTokenSource cts = _cancellationManager.InitCancellationTokenSource(modelCardId);
+
       var sendInfo = new SendInfo(
         modelCard.AccountId.NotNull(),
         modelCard.ProjectId.NotNull(),
@@ -286,6 +286,14 @@ public async Task Send(string modelCardId)
             .Where(obj => obj != null)
             .ToList();
 
+          if (!mapMembers.Any())
+          {
+            // Handle as CARD ERROR in this function
+            throw new SpeckleSendFilterException(
+              "No objects were found to convert. Please update your publish filter!"
+            );
+          }
+
           var result = await unitOfWork.Service
             .Execute(
               mapMembers,
@@ -310,13 +318,15 @@ public async Task Send(string modelCardId)
 
       Commands.SetModelCreatedVersionId(modelCardId, sendResult.rootObjId);
     }
-    catch (OperationCanceledException)
+    // Catch here specific exceptions if they related to model card.
+    catch (SpeckleSendFilterException e)
     {
-      return;
+      Commands.SetModelError(modelCardId, e);
     }
-    catch (Exception e) when (!e.IsFatal()) // All exceptions should be handled here if possible, otherwise we enter "crashing the host app" territory.
+    catch (OperationCanceledException)
     {
-      Commands.SetModelError(modelCardId, e);
+      // SWALLOW -> UI handles it immediately, so we do not need to handle anything
+      return;
     }
   }
 

diff --git a/DUI3-DX/Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Operations/Send/RootObjectBuilder.cs b/DUI3-DX/Connectors/ArcGIS/Speckle.Connectors.ArcGIS3/Operations/Send/RootObjectBuilder.cs
@@ -26,12 +26,6 @@ public Base Build(
     CancellationToken ct = default
   )
   {
-    if (!objects.Any())
-    {
-      // POC: not sure if we would want to throw in here?
-      throw new InvalidOperationException("No objects were found. Please update your send filter!");
-    }
-
     // POC: does this feel like the right place? I am wondering if this should be called from within send/rcv?
     // begin the unit of work
     using var uow = _unitOfWorkFactory.Resolve<IRootToSpeckleConverter>();

diff --git a/...-DX/Connectors/Autocad/Speckle.Connectors.AutocadShared/Bindings/AutocadReceiveBinding.cs b/...-DX/Connectors/Autocad/Speckle.Connectors.AutocadShared/Bindings/AutocadReceiveBinding.cs
@@ -6,7 +6,6 @@
 using Speckle.Connectors.DUI.Models.Card;
 using Speckle.Connectors.Utils;
 using Speckle.Connectors.Utils.Operations;
-using Speckle.Core.Logging;
 using ICancelable = System.Reactive.Disposables.ICancelable;
 
 namespace Speckle.Connectors.Autocad.Bindings;
@@ -43,15 +42,16 @@ public async Task Receive(string modelCardId)
     using var unitOfWork = _unitOfWorkFactory.Resolve<ReceiveOperation>();
     try
     {
-      // Init cancellation token source -> Manager also cancel it if exist before
-      CancellationTokenSource cts = _cancellationManager.InitCancellationTokenSource(modelCardId);
-
       // Get receiver card
       if (_store.GetModelById(modelCardId) is not ReceiverModelCard modelCard)
       {
+        // Handle as GLOBAL ERROR at BrowserBridge
         throw new InvalidOperationException("No download model card was found.");
       }
 
+      // Init cancellation token source -> Manager also cancel it if exist before
+      CancellationTokenSource cts = _cancellationManager.InitCancellationTokenSource(modelCardId);
+
       // Receive host objects
       IEnumerable<string> receivedObjectIds = await unitOfWork.Service
         .Execute(
@@ -67,9 +67,11 @@ public async Task Receive(string modelCardId)
 
       Commands.SetModelReceiveResult(modelCardId, receivedObjectIds.ToList());
     }
-    catch (Exception e) when (!e.IsFatal()) // All exceptions should be handled here if possible, otherwise we enter "crashing the host app" territory.
+    // Catch here specific exceptions if they related to model card.
+    catch (OperationCanceledException)
     {
-      Commands.SetModelError(modelCardId, e);
+      // SWALLOW -> UI handles it immediately, so we do not need to handle anything
+      return;
     }
   }
 

diff --git a/DUI3-DX/Connectors/Autocad/Speckle.Connectors.AutocadShared/Bindings/AutocadSendBinding.cs b/DUI3-DX/Connectors/Autocad/Speckle.Connectors.AutocadShared/Bindings/AutocadSendBinding.cs
@@ -6,9 +6,9 @@
 using Speckle.Connectors.DUI.Models;
 using Speckle.Connectors.DUI.Models.Card;
 using Speckle.Connectors.Utils.Cancellation;
-using Speckle.Core.Logging;
 using Speckle.Autofac.DependencyInjection;
 using Speckle.Connectors.Autocad.Operations.Send;
+using Speckle.Connectors.DUI.Exceptions;
 using Speckle.Connectors.Utils.Operations;
 using Speckle.Core.Models;
 using ICancelable = System.Reactive.Disposables.ICancelable;
@@ -122,23 +122,26 @@ private async Task SendInternal(string modelCardId)
   {
     try
     {
-      using var uow = _unitOfWorkFactory.Resolve<SendOperation<AutocadRootObject>>();
-      // 0 - Init cancellation token source -> Manager also cancel it if exist before
-      CancellationTokenSource cts = _cancellationManager.InitCancellationTokenSource(modelCardId);
-
-      // 1 - Get model
       if (_store.GetModelById(modelCardId) is not SenderModelCard modelCard)
       {
+        // Handle as GLOBAL ERROR at BrowserBridge
         throw new InvalidOperationException("No publish model card was found.");
       }
 
+      using var uow = _unitOfWorkFactory.Resolve<SendOperation<AutocadRootObject>>();
+
+      // Init cancellation token source -> Manager also cancel it if exist before
+      CancellationTokenSource cts = _cancellationManager.InitCancellationTokenSource(modelCardId);
+
       // Get elements to convert
       List<AutocadRootObject> autocadObjects = Application.DocumentManager.CurrentDocument.GetObjects(
         modelCard.SendFilter.NotNull().GetObjectIds()
       );
+
       if (autocadObjects.Count == 0)
       {
-        throw new InvalidOperationException("No objects were found. Please update your send filter!");
+        // Handle as CARD ERROR in this function
+        throw new SpeckleSendFilterException("No objects were found to convert. Please update your publish filter!");
       }
 
       var sendInfo = new SendInfo(
@@ -170,11 +173,13 @@ private async Task SendInternal(string modelCardId)
 
       Commands.SetModelCreatedVersionId(modelCardId, sendResult.rootObjId);
     }
+    // Catch here specific exceptions if they related to model card.
     catch (OperationCanceledException)
     {
+      // SWALLOW -> UI handles it immediately, so we do not need to handle anything
       return;
     }
-    catch (Exception e) when (!e.IsFatal()) // All exceptions should be handled here if possible, otherwise we enter "crashing the host app" territory.
+    catch (SpeckleSendFilterException e)
     {
       Commands.SetModelError(modelCardId, e);
     }

diff --git a/DUI3-DX/Connectors/Revit/Speckle.Connectors.RevitShared/Bindings/SendBinding.cs b/DUI3-DX/Connectors/Revit/Speckle.Connectors.RevitShared/Bindings/SendBinding.cs
@@ -9,6 +9,7 @@
 using Speckle.Connectors.DUI.Models.Card;
 using Speckle.Connectors.DUI.Bindings;
 using Speckle.Autofac.DependencyInjection;
+using Speckle.Connectors.DUI.Exceptions;
 using Speckle.Connectors.DUI.Models;
 using Speckle.Connectors.Utils.Operations;
 using Speckle.Core.Models;
@@ -72,20 +73,21 @@ public void CancelSend(string modelCardId)
 
   private async Task HandleSend(string modelCardId)
   {
-    // POC: probably the CTS SHOULD be injected as InstancePerLifetimeScope and then
-    // it can be injected where needed instead of passing it around like a bomb :D
-    CancellationTokenSource cts = CancellationManager.InitCancellationTokenSource(modelCardId);
-
     // POC: this try catch pattern is coming from Rhino. I see there is a semi implemented "SpeckleTopLevelExceptionHandler"
     // above that wraps the call of this HandleSend, but it currently is not doing anything - resulting in all errors from here
     // bubbling up to the bridge.
     try
     {
       if (Store.GetModelById(modelCardId) is not SenderModelCard modelCard)
       {
+        // Handle as GLOBAL ERROR at BrowserBridge
         throw new InvalidOperationException("No publish model card was found.");
       }
 
+      // POC: probably the CTS SHOULD be injected as InstancePerLifetimeScope and then
+      // it can be injected where needed instead of passing it around like a bomb :D
+      CancellationTokenSource cts = CancellationManager.InitCancellationTokenSource(modelCardId);
+
       using IUnitOfWork<SendOperation<ElementId>> sendOperation = _unitOfWorkFactory.Resolve<
         SendOperation<ElementId>
       >();
@@ -96,6 +98,12 @@ private async Task HandleSend(string modelCardId)
         .Select(id => ElementId.Parse(id))
         .ToList();
 
+      if (!revitObjects.Any())
+      {
+        // Handle as CARD ERROR in this function
+        throw new SpeckleSendFilterException("No objects were found to convert. Please update your publish filter!");
+      }
+
       var sendInfo = new SendInfo(
         modelCard.AccountId.NotNull(),
         modelCard.ProjectId.NotNull(),
@@ -125,13 +133,14 @@ private async Task HandleSend(string modelCardId)
 
       Commands.SetModelCreatedVersionId(modelCardId, sendResult.rootObjId);
     }
-    catch (OperationCanceledException)
+    // Catch here specific exceptions if they related to model card.
+    catch (SpeckleSendFilterException e)
     {
-      return;
+      Commands.SetModelError(modelCardId, e);
     }
-    catch (Exception e) when (!e.IsFatal()) // All exceptions should be handled here if possible, otherwise we enter "crashing the host app" territory.
+    catch (OperationCanceledException)
     {
-      Commands.SetModelError(modelCardId, e);
+      return;
     }
   }
 

diff --git a/...Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs b/...Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs
@@ -1,6 +1,7 @@
 using Speckle.Converters.Common;
 using Speckle.Core.Models;
 using Autodesk.Revit.DB;
+using Speckle.Connectors.DUI.Exceptions;
 using Speckle.Converters.RevitShared.Helpers;
 using Speckle.Connectors.Utils.Builders;
 using Speckle.Connectors.Utils.Operations;
@@ -56,7 +57,7 @@ public Base Build(
 
     if (revitElements.Count == 0)
     {
-      throw new InvalidOperationException("No objects were found. Please update your send filter!");
+      throw new SpeckleSendFilterException("No objects were found. Please update your send filter!");
     }
 
     var countProgress = 0; // because for(int i = 0; ...) loops are so last year

diff --git a/DUI3-DX/Connectors/Rhino/Speckle.Connectors.Rhino7/Bindings/RhinoReceiveBinding.cs b/DUI3-DX/Connectors/Rhino/Speckle.Connectors.Rhino7/Bindings/RhinoReceiveBinding.cs
@@ -6,7 +6,6 @@
 using Speckle.Connectors.Utils;
 using Speckle.Connectors.Utils.Cancellation;
 using Speckle.Connectors.Utils.Operations;
-using Speckle.Core.Logging;
 
 namespace Speckle.Connectors.Rhino7.Bindings;
 
@@ -41,15 +40,16 @@ public async Task Receive(string modelCardId)
     using var unitOfWork = _unitOfWorkFactory.Resolve<ReceiveOperation>();
     try
     {
-      // Init cancellation token source -> Manager also cancel it if exist before
-      CancellationTokenSource cts = CancellationManager.InitCancellationTokenSource(modelCardId);
-
       // Get receiver card
       if (_store.GetModelById(modelCardId) is not ReceiverModelCard modelCard)
       {
+        // Handle as GLOBAL ERROR at BrowserBridge
         throw new InvalidOperationException("No download model card was found.");
       }
 
+      // Init cancellation token source -> Manager also cancel it if exist before
+      CancellationTokenSource cts = CancellationManager.InitCancellationTokenSource(modelCardId);
+
       // Receive host objects
       IEnumerable<string> receivedObjectIds = await unitOfWork.Service
         .Execute(
@@ -66,9 +66,11 @@ public async Task Receive(string modelCardId)
       // POC: Here we can't set receive result if ReceiveOperation throws an error.
       Commands.SetModelReceiveResult(modelCardId, receivedObjectIds.ToList());
     }
-    catch (Exception e) when (!e.IsFatal()) // All exceptions should be handled here if possible, otherwise we enter "crashing the host app" territory.
+    // Catch here specific exceptions if they related to model card.
+    catch (OperationCanceledException)
     {
-      Commands.SetModelError(modelCardId, e);
+      // SWALLOW -> UI handles it immediately, so we do not need to handle anything
+      return;
     }
   }