fix(rhino): CNX-8388 handle ca1031 warnings in rhino connector (#3110)

* fixes exception handling in rhino connector files * refactors getlayer method to be more robust * refactors to use new GetGuidFromString method * removes unnecessary else blocks * fixes one more ca1031 warning * pr fixes * Update MappingBindingsRhino.cs * Update Utils.cs * adds conversionnotsupported exceptions and better exception handling in converter * Update ConverterRhinoGh.cs * removes unnecessary disposal * fix(rhino): Added use of `IsFatal` in converter * fix(rhino): Added use of `IsFatal` in connector * fix(rhino): Missing newline * fix(rhino): Wrapped all generic exceptions with isFatal handling * fix(rhino): Typo in isFatal handling --------- Co-authored-by: Alan Rynne <[email protected]>
specklesystems · Jan 9, 2024 · 5bf6a8d · 5bf6a8d
1 parent 030ef1d
commit 5bf6a8d
Show file tree

Hide file tree

Showing 14 changed files with 292 additions and 248 deletions.
diff --git a/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/Entry/Plugin.cs b/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/Entry/Plugin.cs
@@ -144,7 +144,8 @@ private void RhinoDoc_EndOpenDocument(object sender, DocumentOpenEventArgs e)
         try
         {
           SpeckleCommandMac.CreateOrFocusSpeckle();
-        } catch (Exception ex)
+        } 
+        catch (Exception ex) when (!ex.IsFatal())
         {
           SpeckleLog.Logger.Fatal(ex, "Failed to create or focus Speckle window");
           RhinoApp.CommandLineOut.WriteLine($"Speckle error - {ex.ToFormattedString()}");
@@ -189,7 +190,7 @@ protected override LoadReturnCode OnLoad(ref string errorMessage)
         hostAppVersion
       );
     }
-    catch (Exception e)
+    catch (Exception e) when (!e.IsFatal())
     {
       RhinoApp.CommandLineOut.WriteLine("Failed to init speckle logger: " + e.ToFormattedString());
       return LoadReturnCode.ErrorShowDialog;
@@ -214,7 +215,8 @@ protected override LoadReturnCode OnLoad(ref string errorMessage)
     {
       Init();
     }
-    catch (Exception ex)
+    // need investigation in seq of specific excpetions thrown (FileNotFound, TypeInitialization)
+    catch (Exception ex) when (!ex.IsFatal())
     {
       SpeckleLog.Logger.Fatal(ex, "Failed to load Speckle Plugin with {exceptionMessage}", ex.Message);
       errorMessage = $"Failed to load Speckle Plugin with {ex.ToFormattedString()}";
@@ -267,17 +269,30 @@ private void EnsureVersionSettings()
 
     using (LogContext.PushProperty("path", path))
     {
-      SpeckleLog.Logger.Debug("Deleting and Updating RUI settings file");
-
       if (File.Exists(path))
       {
+        SpeckleLog.Logger.Information("Deleting and Updating RUI settings file");
         try
         {
           File.Delete(path);
         }
-        catch (Exception ex)
+        catch (IOException ioEx)
+        {
+          SpeckleLog.Logger.Error(
+            ioEx,
+            "Failed to delete Speckle toolbar .rui file with {exceptionMessage}",
+            ioEx.Message
+          );
+          RhinoApp.CommandLineOut.WriteLine($"Failed to delete Speckle toolbar {path} with {ioEx.ToFormattedString()}");
+        }
+        catch (UnauthorizedAccessException uaEx)
         {
-          SpeckleLog.Logger.Warning(ex, "Failed to delete rui file {exceptionMessage}", ex.Message);
+          SpeckleLog.Logger.Error(
+            uaEx,
+            "Failed to delete Speckle toolbar .rui file with {exceptionMessage}",
+            uaEx.Message
+          );
+          RhinoApp.CommandLineOut.WriteLine($"Failed to delete Speckle toolbar {path} with {uaEx.ToFormattedString()}");
         }
       }
     }
@@ -320,7 +335,7 @@ private void RhinoApp_Idle(object sender, EventArgs e)
         MappingBindings.UpdateExistingSchemaElements(MappingBindings.GetExistingSchemaElements());
       }
     }
-    catch (Exception ex) { }
+    catch (Exception ex) when (!ex.IsFatal()) { }
   }
 
   private void RhinoDoc_DeselectObjects(object sender, RhinoObjectSelectionEventArgs e)

diff --git a/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/Entry/SpeckleCommandMac.cs b/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/Entry/SpeckleCommandMac.cs
@@ -32,7 +32,7 @@ protected override Result RunCommand(RhinoDoc doc, RunMode mode)
         CreateOrFocusSpeckle();
         return Result.Success;
       }
-      catch (Exception e)
+      catch (Exception e) when (!e.IsFatal())
       {
         SpeckleLog.Logger.Fatal(e, "Failed to create or focus Speckle window");
         RhinoApp.CommandLineOut.WriteLine($"Speckle Error - { e.ToFormattedString() }");

diff --git a/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/Entry/SpeckleCommandWin.cs b/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/Entry/SpeckleCommandWin.cs
@@ -3,6 +3,7 @@
 using Rhino;
 using Rhino.Commands;
 using Rhino.UI;
+using Speckle.Core.Logging;
 using Speckle.Core.Models.Extensions;
 
 namespace SpeckleRhino;
@@ -25,8 +26,10 @@ protected override Result RunCommand(RhinoDoc doc, RunMode mode)
       Panels.OpenPanel(typeof(DuiPanel).GUID);
       return Result.Success;
     }
-    catch (Exception e)
+    catch (Exception e) when (!e.IsFatal())
     {
+      // needs more investigation. logging to seq for now.
+      SpeckleLog.Logger.Error(e, "Failed to open Speckle Rhino Connector DuiPanel with {exceptionMessage}", e.Message);
       RhinoApp.CommandLineOut.WriteLine($"Speckle Error - {e.ToFormattedString()}");
       return Result.Failure;
     }

diff --git a/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/Entry/SpeckleMappingsCommandMac.cs b/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/Entry/SpeckleMappingsCommandMac.cs
@@ -41,7 +41,7 @@ protected override Result RunCommand(RhinoDoc doc, RunMode mode)
         MainWindow.Activate();
         return Result.Success;
       }
-      catch (Exception e)
+      catch (Exception e) when (!e.IsFatal())
       {
         SpeckleLog.Logger.Fatal(e, "Failed to create or focus Speckle mappings window");
         RhinoApp.CommandLineOut.WriteLine($"Speckle Error - {e.ToFormattedString()}");

diff --git a/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/Entry/SpeckleMappingsCommandWin.cs b/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/Entry/SpeckleMappingsCommandWin.cs
@@ -4,6 +4,7 @@
 using Rhino;
 using Rhino.Commands;
 using Rhino.UI;
+using Speckle.Core.Logging;
 using Speckle.Core.Models.Extensions;
 
 namespace SpeckleRhino;
@@ -26,8 +27,10 @@ protected override Result RunCommand(RhinoDoc doc, RunMode mode)
       Panels.OpenPanel(typeof(MappingsPanel).GUID);
       return Result.Success;
     }
-    catch (Exception e)
+    catch (Exception e) when (!e.IsFatal())
     {
+      // needs more investigation. logging to seq for now.
+      SpeckleLog.Logger.Error(e, "Failed to open Speckle Rhino Mapper DuiPanel with {exceptionMessage}", e.Message);
       RhinoApp.CommandLineOut.WriteLine($"Speckle Error - {e.ToFormattedString()}");
       return Result.Failure;
     }

diff --git a/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/UI/ConnectorBindingsRhino.Previews.cs b/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/UI/ConnectorBindingsRhino.Previews.cs
@@ -8,6 +8,7 @@
 using Rhino.DocObjects;
 using Speckle.Core.Api;
 using Speckle.Core.Kits;
+using Speckle.Core.Logging;
 using Speckle.Core.Models;
 
 namespace SpeckleRhino;
@@ -195,16 +196,15 @@ public override async Task<StreamState> PreviewReceive(StreamState state, Progre
     }
 
     // create display conduit
-    try
+    PreviewConduit = new PreviewConduit(Preview);
+    if (PreviewConduit.Preview.Count == 0)
     {
-      PreviewConduit = new PreviewConduit(Preview);
-    }
-    catch (Exception e)
-    {
-      progress.Report.OperationErrors.Add(new Exception($"Could not create preview: {e.Message}"));
+      SpeckleLog.Logger.Information("No previewable geometry was found.");
+      progress.Report.OperationErrors.Add(new Exception($"No previewable objects found."));
       ResetDocument();
       return null;
     }
+
     PreviewConduit.Enabled = true;
     Doc.Views.ActiveView.ActiveViewport.ZoomBoundingBox(PreviewConduit.bbox);
     Doc.Views.Redraw();

diff --git a/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/UI/ConnectorBindingsRhino.Receive.cs b/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/UI/ConnectorBindingsRhino.Receive.cs
@@ -314,26 +314,29 @@ public override async Task<StreamState> ReceiveStream(StreamState state, Progres
   // gets objects by id directly or by application id user string
   private List<RhinoObject> GetObjectsByApplicationId(string applicationId)
   {
+    List<RhinoObject> match = new();
+
     if (string.IsNullOrEmpty(applicationId))
     {
-      return new List<RhinoObject>();
+      return match;
     }
 
     // first try to find the object by app id user string
-    var match = Doc.Objects.FindByUserString(ApplicationIdKey, applicationId, true).ToList();
+    if (Doc.Objects.FindByUserString(ApplicationIdKey, applicationId, true) is RhinoObject[] foundObjects)
+    {
+      match = foundObjects.ToList();
+    }
 
     // if nothing is found, look for the geom obj by its guid directly
-    if (!match.Any())
+    if (match.Count == 0)
     {
-      try
+      if (Utils.GetGuidFromString(applicationId, out Guid id))
       {
-        RhinoObject obj = Doc.Objects.FindId(new Guid(applicationId));
-        if (obj != null)
+        if (Doc.Objects.FindId(id) is RhinoObject obj)
         {
           match.Add(obj);
         }
       }
-      catch { }
     }
 
     return match;
@@ -542,9 +545,10 @@ private void BakeObject(
           Base render = obj["renderMaterial"] as Base ?? obj["@renderMaterial"] as Base;
           if (display != null)
           {
-            if (converter.ConvertToNative(display) is ObjectAttributes displayAttribute)
+            var convertedDisplay = converter.ConvertToNative(display) as ObjectAttributes;
+            if (convertedDisplay is not null)
             {
-              attributes = displayAttribute;
+              attributes = convertedDisplay;
             }
           }
           else if (render != null)
@@ -607,8 +611,8 @@ private void BakeObject(
           // handle render material
           if (render != null)
           {
-            var convertedMaterial = converter.ConvertToNative(render) as RenderMaterial; //Maybe wrap in try catch in case no conversion exists?
-            if (convertedMaterial != null)
+            var convertedMaterial = converter.ConvertToNative(render) as RenderMaterial;
+            if (convertedMaterial is not null)
             {
               RhinoObject rhinoObject = Doc.Objects.FindId(id);
               rhinoObject.RenderMaterial = convertedMaterial;
@@ -672,11 +676,7 @@ private void SetUserInfo(Base obj, ObjectAttributes attributes, ApplicationObjec
 
     // set application id
     var appId = parent != null ? parent.applicationId : obj.applicationId;
-    try
-    {
-      attributes.SetUserString(ApplicationIdKey, appId);
-    }
-    catch { }
+    attributes.SetUserString(ApplicationIdKey, appId);
 
     // set user dictionaries
     if (obj[UserDictionary] is Base userDictionary)

diff --git a/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/UI/ConnectorBindingsRhino.Selection.cs b/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/UI/ConnectorBindingsRhino.Selection.cs
@@ -76,37 +76,23 @@ public override List<ISelectionFilter> GetSelectionFilters()
 
   public override void SelectClientObjects(List<string> objs, bool deselect = false)
   {
-    var isPreview = PreviewConduit != null && PreviewConduit.Enabled ? true : false;
+    var isPreview = PreviewConduit != null && PreviewConduit.Enabled;
 
     foreach (var id in objs)
     {
-      RhinoObject obj = null;
-      try
+      if (Utils.GetGuidFromString(id, out Guid guid))
       {
-        obj = Doc.Objects.FindId(new Guid(id)); // this is a rhinoobj
-      }
-      catch
-      {
-        continue; // this was a named view!
-      }
-
-      if (obj != null)
-      {
-        if (deselect)
+        if (Doc.Objects.FindId(guid) is RhinoObject obj)
         {
-          obj.Select(false, true, false, true, true, true);
+          obj.Select(!deselect, true, true, true, true, true);
         }
-        else
+        else if (isPreview)
         {
-          obj.Select(true, true, true, true, true, true);
+          PreviewConduit.Enabled = false;
+          PreviewConduit.SelectPreviewObject(id, deselect);
+          PreviewConduit.Enabled = true;
         }
       }
-      else if (isPreview)
-      {
-        PreviewConduit.Enabled = false;
-        PreviewConduit.SelectPreviewObject(id, deselect);
-        PreviewConduit.Enabled = true;
-      }
     }
 
     Doc.Views.ActiveView.ActiveViewport.ZoomExtentsSelected();

diff --git a/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/UI/DuiPanel.xaml.cs b/ConnectorRhino/ConnectorRhino/ConnectorRhinoShared/UI/DuiPanel.xaml.cs
@@ -3,6 +3,7 @@
 using System.Windows.Controls;
 using DesktopUI2.ViewModels;
 using DesktopUI2.Views;
+using Speckle.Core.Logging;
 
 namespace SpeckleRhino;
 
@@ -24,6 +25,6 @@ public DuiPanel()
       DataContext = viewModel;
       AvaloniaHost.Content = new MainUserControl();
     }
-    catch (Exception ex) { }
+    catch (Exception ex) when (!ex.IsFatal()) { }
   }
 }