Skip to content

Commit

Permalink
Refactor usage of exceptions in converters/connectors (#286)
Browse files Browse the repository at this point in the history
* feat: Non-controversial rebased changes from alan/exception-handling

* fix: Fixed all other usages after rebase

* fix: Re-added exception throw on 0 objects converted

* fix: Missing using statement

* fix: Converter manager

* fix: Using statements

* fix: Exception usages coming in from merge
  • Loading branch information
AlanRynne authored Oct 16, 2024
1 parent da0a35a commit fa1fa35
Show file tree
Hide file tree
Showing 79 changed files with 232 additions and 440 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ mapMember is not ILayerContainer

if (results.All(x => x.Status == Status.ERROR))
{
throw new SpeckleConversionException("Failed to convert all objects."); // fail fast instead creating empty commit! It will appear as model card error with red color.
throw new SpeckleException("Failed to convert all objects."); // fail fast instead creating empty commit! It will appear as model card error with red color.
}

// POC: Add Color Proxies
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
using Autodesk.AutoCAD.DatabaseServices;
using Speckle.Connectors.Autocad.HostApp.Extensions;
using Speckle.Converters.Common;
using Speckle.Sdk;
using Speckle.Sdk.Models.Collections;

namespace Speckle.Connectors.Autocad.HostApp;
Expand All @@ -24,6 +24,7 @@ public Layer GetOrCreateSpeckleLayer(Entity entity, Transaction tr, out LayerTab
layer = autocadLayer;
return speckleLayer;
}
throw new SpeckleConversionException("Unexpected condition in GetOrCreateSpeckleLayer");

throw new SpeckleException("Unexpected condition in GetOrCreateSpeckleLayer");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ private RootObjectBuilderResult BuildSync(

if (results.All(x => x.Status == Status.ERROR))
{
throw new SpeckleConversionException("Failed to convert all objects."); // fail fast instead creating empty commit! It will appear as model card error with red color.
throw new SpeckleException("Failed to convert all objects."); // fail fast instead creating empty commit! It will appear as model card error with red color.
}

// 4 - Unpack the render material proxies
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using Autodesk.Revit.DB;
using Speckle.Converters.Common;
using Speckle.Sdk;

namespace Speckle.Connectors.RevitShared;

Expand All @@ -10,7 +10,7 @@ public static ElementId GetElementIdFromUniqueId(Document doc, string uniqueId)
Element element = doc.GetElement(uniqueId);
if (element == null)
{
throw new SpeckleConversionException($"Cannot find element with UniqueId: {uniqueId}");
throw new SpeckleException($"Cannot find element with UniqueId: {uniqueId}");
}

return element.Id;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
using Speckle.Objects;
using Speckle.Objects.Geometry;
using Speckle.Sdk;
using Speckle.Sdk.Common.Exceptions;
using Speckle.Sdk.Logging;
using Speckle.Sdk.Models;
using Transform = Speckle.Objects.Other.Transform;
Expand Down Expand Up @@ -221,9 +222,7 @@ localToGlobalMap.AtomicObject is ITransformable transformable and ICurve
}
else
{
throw new SpeckleConversionException(
$"Failed to cast {result.GetType()} to direct shape definition wrapper."
);
throw new ConversionException($"Failed to cast {result.GetType()} to direct shape definition wrapper.");
}
}
catch (Exception ex) when (!ex.IsFatal())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public async Task<RootObjectBuilderResult> Build(

if (results.All(x => x.Status == Status.ERROR))
{
throw new SpeckleConversionException("Failed to convert all objects.");
throw new SpeckleException("Failed to convert all objects.");
}

var idsAndSubElementIds = _elementUnpacker.GetElementsAndSubelementIdsFromAtomicObjects(atomicObjects);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
using Rhino;
using Speckle.Sdk;
using Speckle.Sdk.Common;
using Speckle.Sdk.Common.Exceptions;

namespace Speckle.Connectors.Rhino.Extensions;

Expand Down Expand Up @@ -31,7 +31,7 @@ public static string ToSpeckleString(this UnitSystem unitSystem)
case UnitSystem.Unset:
return Units.Meters;
default:
throw new SpeckleException($"The Unit System \"{unitSystem}\" is unsupported.");
throw new UnitNotSupportedException($"The Unit System \"{unitSystem}\" is unsupported.");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ CancellationToken cancellationToken

if (conversionIds.Count == 0)
{
throw new SpeckleConversionException($"Failed to convert object.");
throw new SpeckleException($"Failed to convert object.");
}

// 4: log
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public async Task<RootObjectBuilderResult> Build(

if (results.All(x => x.Status == Status.ERROR))
{
throw new SpeckleConversionException("Failed to convert all objects."); // fail fast instead creating empty commit! It will appear as model card error with red color.
throw new SpeckleException("Failed to convert all objects."); // fail fast instead creating empty commit! It will appear as model card error with red color.
}

using (var _ = _activityFactory.Start("UnpackRenderMaterials"))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
using ArcGIS.Core.Geometry;
using Speckle.Converters.Common;
using Speckle.Sdk;
using Speckle.Sdk.Common;
using Speckle.Sdk.Common.Exceptions;

namespace Speckle.Converters.ArcGIS3;

Expand Down Expand Up @@ -41,7 +41,6 @@ public string ConvertOrThrow(Unit hostUnit)
return value;
}

// POC: probably would prefer something more specific
throw new SpeckleException($"The Unit System \"{hostUnit}\" is unsupported.");
throw new UnitNotSupportedException($"The Unit System \"{hostUnit}\" is unsupported.");
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Speckle.Converters.Common.Objects;
using Speckle.Objects;
using Speckle.Sdk.Common.Exceptions;

namespace Speckle.Converters.ArcGIS3.ToHost.Raw;

Expand Down Expand Up @@ -34,7 +35,7 @@ public CurveToHostConverter(
/// </summary>
/// <param name="target">The ICurve object to convert.</param>
/// <returns>The converted RG.Curve object.</returns>
/// <exception cref="NotSupportedException">Thrown when the conversion is not supported for the given type of curve.</exception>
/// <exception cref="ValidationException">Thrown when the conversion is not supported for the given type of curve.</exception>
/// <remarks>⚠️ This conversion does NOT perform scaling.</remarks>
public ACG.Polyline Convert(ICurve target) =>
target switch
Expand All @@ -47,6 +48,6 @@ public ACG.Polyline Convert(ICurve target) =>
SOG.Polyline polyline => _polylineConverter.Convert(polyline),
SOG.Curve curve => _polylineConverter.Convert(curve.displayValue),
SOG.Polycurve polyCurve => _polyCurveConverter.Convert(polyCurve),
_ => throw new NotSupportedException($"Unable to convert curves of type {target.GetType().Name}")
_ => throw new ValidationException($"Converter does not support curves of type '{target.GetType().Name}'")
};
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using Speckle.Converters.Common;
using Speckle.Converters.Common.Objects;
using Speckle.Sdk.Common.Exceptions;
using Speckle.Sdk.Models;

namespace Speckle.Converters.ArcGIS3.ToHost.Raw;
Expand Down Expand Up @@ -29,32 +29,19 @@ public GeometryToHostConverter(

public ACG.Geometry Convert(IReadOnlyList<Base> target)
{
try
if (target.Count == 0)
{
if (target.Count > 0)
{
switch (target[0])
{
case SOG.Point point:
return _multipointConverter.Convert(target.Cast<SOG.Point>().ToList());
case SOG.Polyline polyline:
return _polylineConverter.Convert(target.Cast<SOG.Polyline>().ToList());
case SGIS.PolygonGeometry3d geometry3d:
return _polygon3dConverter.Convert(target.Cast<SGIS.PolygonGeometry3d>().ToList());
case SGIS.PolygonGeometry geometry:
return _polygonConverter.Convert(target.Cast<SGIS.PolygonGeometry>().ToList());
case SGIS.GisMultipatchGeometry mesh:
return _multipatchConverter.Convert(target.Cast<SGIS.GisMultipatchGeometry>().ToList());
default:
throw new NotSupportedException($"No conversion found for type {target[0]}");
}
}
throw new NotSupportedException($"Feature contains no geometry");
throw new ValidationException("Feature contains no geometry");
}
catch (SpeckleConversionException e)

return target[0] switch
{
Console.WriteLine(e);
throw; // log errors
}
SOG.Point => _multipointConverter.Convert(target.Cast<SOG.Point>().ToList()),
SOG.Polyline => _polylineConverter.Convert(target.Cast<SOG.Polyline>().ToList()),
SGIS.PolygonGeometry3d => _polygon3dConverter.Convert(target.Cast<SGIS.PolygonGeometry3d>().ToList()),
SGIS.PolygonGeometry => _polygonConverter.Convert(target.Cast<SGIS.PolygonGeometry>().ToList()),
SGIS.GisMultipatchGeometry => _multipatchConverter.Convert(target.Cast<SGIS.GisMultipatchGeometry>().ToList()),
_ => throw new ValidationException($"No conversion found for type {target[0]}")
};
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using Speckle.Converters.Common;
using Speckle.Converters.Common.Objects;
using Speckle.Objects.Utils;
using Speckle.Sdk.Common.Exceptions;

namespace Speckle.Converters.ArcGIS3.ToHost.Raw;

Expand All @@ -22,7 +23,7 @@ public ACG.Multipatch Convert(List<SOG.Mesh> target)
{
if (target.Count == 0)
{
throw new SpeckleConversionException("Feature contains no geometries");
throw new ValidationException("Feature contains no geometries");
}
ACG.MultipatchBuilderEx multipatchPart = new(_settingsStore.Current.ActiveCRSoffsetRotation.SpatialReference);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using Speckle.Converters.Common;
using Speckle.Converters.Common.Objects;
using Speckle.Sdk.Common.Exceptions;

namespace Speckle.Converters.ArcGIS3.ToHost.Raw;

Expand All @@ -16,7 +16,7 @@ public ACG.Multipatch Convert(List<SGIS.GisMultipatchGeometry> target)
{
if (target.Count == 0)
{
throw new SpeckleConversionException("Feature contains no geometries");
throw new ValidationException("Feature contains no geometries");
}
ACG.MultipatchBuilderEx multipatchPart = new();
foreach (SGIS.GisMultipatchGeometry part in target)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using Speckle.Converters.Common;
using Speckle.Converters.Common.Objects;
using Speckle.Sdk.Common.Exceptions;

namespace Speckle.Converters.ArcGIS3.ToHost.Raw;

Expand All @@ -16,7 +16,7 @@ public ACG.Multipoint Convert(List<SOG.Point> target)
{
if (target.Count == 0)
{
throw new SpeckleConversionException("Feature contains no geometries");
throw new ValidationException("Feature contains no geometries");
}
List<ACG.MapPoint> pointList = new();
foreach (SOG.Point pt in target)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using Speckle.Converters.Common;
using Speckle.Converters.Common.Objects;
using Speckle.Sdk.Common.Exceptions;

namespace Speckle.Converters.ArcGIS3.ToHost.Raw;

Expand All @@ -21,7 +21,7 @@ public ACG.Multipatch Convert(List<SGIS.PolygonGeometry3d> target)
{
if (target.Count == 0)
{
throw new SpeckleConversionException("Feature contains no geometries");
throw new ValidationException("Feature contains no geometries");
}

ACG.MultipatchBuilderEx multipatchPart = new();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using Speckle.Converters.Common;
using Speckle.Converters.Common.Objects;
using Speckle.Sdk.Common.Exceptions;

namespace Speckle.Converters.ArcGIS3.ToHost.Raw;

Expand All @@ -16,7 +16,7 @@ public ACG.Polygon Convert(List<SGIS.PolygonGeometry> target)
{
if (target.Count == 0)
{
throw new SpeckleConversionException("Feature contains no geometries");
throw new ValidationException("Feature contains no geometries");
}
List<ACG.Polygon> polyList = new();
foreach (SGIS.PolygonGeometry poly in target)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using Speckle.Converters.Common;
using Speckle.Converters.Common.Objects;
using Speckle.Sdk.Common.Exceptions;

namespace Speckle.Converters.ArcGIS3.ToHost.Raw;

Expand All @@ -16,7 +16,7 @@ public ACG.Polyline Convert(List<SOG.Polyline> target)
{
if (target.Count == 0)
{
throw new SpeckleConversionException("Feature contains no geometries");
throw new ValidationException("Feature contains no geometries");
}
List<ACG.Polyline> polyList = new();
foreach (SOG.Polyline poly in target)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Speckle.Converters.Common;
using Speckle.Converters.Common.Objects;
using Speckle.Sdk.Common.Exceptions;
using Speckle.Sdk.Models;

namespace Speckle.Converters.ArcGIS3.ToHost.TopLevel;
Expand Down Expand Up @@ -28,7 +29,7 @@ public ACG.Geometry Convert(DisplayableObject target)
{
if (!target.displayValue.Any())
{
throw new NotSupportedException($"Zero fallback values specified");
throw new ValidationException($"Zero fallback values specified");
}

var first = target.displayValue[0];
Expand All @@ -38,7 +39,7 @@ public ACG.Geometry Convert(DisplayableObject target)
SOG.Polyline => _polylineListConverter.Convert(target.displayValue.Cast<SOG.Polyline>().ToList()),
SOG.Mesh => _meshListConverter.Convert(target.displayValue.Cast<SOG.Mesh>().ToList()),
SOG.Point => _pointListConverter.Convert(target.displayValue.Cast<SOG.Point>().ToList()),
_ => throw new NotSupportedException($"Found unsupported fallback geometry: {first.GetType()}")
_ => throw new ValidationException($"Found unsupported fallback geometry: {first.GetType()}")
};
}
}
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
using Speckle.Converters.Common;
using Speckle.Converters.Common.Objects;
using Speckle.Objects;
using Speckle.Sdk.Common.Exceptions;
using Speckle.Sdk.Models;

namespace Speckle.Converters.ArcGIS3.ToHost.TopLevel;

/// <summary>
/// Converter for <see cref="IGisFeature"/> with geometry.
/// </summary>
/// <exception cref="ArgumentException"> Thrown when IGisFeature is <see cref="SGIS.GisNonGeometricFeature"/> because it has no geometry, or when Multipatch geometry contained invalid types.</exception>
/// <exception cref="NotSupportedException">Thrown for unsupported <see cref="IGisFeature"/> classes.</exception>
[NameAndRankValue(nameof(SGIS.GisMultipatchFeature), NameAndRankValueAttribute.SPECKLE_DEFAULT_RANK)]
public class GisMultipatchFeatureToHostConverter
: IToHostTopLevelConverter,
Expand Down Expand Up @@ -48,8 +47,9 @@ public ACG.Geometry Convert(SGIS.GisMultipatchFeature target)

if (multipatch is null)
{
throw new SpeckleConversionException("Multipatch conversion did not return valid geometry");
throw new ConversionException("Multipatch conversion did not return valid geometry");
}

return multipatch;
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Speckle.Converters.Common;
using Speckle.Converters.Common.Objects;
using Speckle.Sdk.Common.Exceptions;
using Speckle.Sdk.Models;

namespace Speckle.Converters.ArcGIS3.ToHost.TopLevel;
Expand Down Expand Up @@ -41,7 +42,7 @@ public ACG.Polyline Convert(SOG.Polycurve target)
)
)
{
throw new SpeckleConversionException("Polycurve segments are not in a correct sequence/orientation");
throw new ValidationException("Polycurve segments are not in a correct sequence/orientation");
}

lastConvertedPt = segmentPts[^1];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using Speckle.Converters.Common;
using Speckle.Converters.Common.Objects;
using Speckle.Objects;
using Speckle.Sdk.Common.Exceptions;
using Speckle.Sdk.Models;

namespace Speckle.Converters.ArcGIS3.ToSpeckle.Raw;
Expand Down Expand Up @@ -190,7 +191,7 @@ public IGisFeature Convert((Row, string) target)
};

default:
throw new NotSupportedException($"No geometry conversion found for {shape.GetType().Name}");
throw new ValidationException($"No geometry conversion found for {shape.GetType().Name}");
}
}
}
Loading

0 comments on commit fa1fa35

Please sign in to comment.