Merge pull request #350 from specklesystems/dev

(dev to main) If we're already cancelling, ignore extra exceptions (#349)

src/Speckle.Sdk.Dependencies/Serialization/ChannelLoader.cs

@@ -109,6 +109,9 @@ public abstract class ChannelLoader<T>(CancellationToken cancellationToken)
     Exception = ex;
     _channel.Writer.TryComplete(ex);
     //cancel everything!
-    _cts.Cancel();
+    if (!_cts.IsCancellationRequested)
+    {
+      _cts.Cancel();
+    }
   }
 }

src/Speckle.Sdk.Dependencies/SpeckleHttpClientHandler.cs

@@ -41,13 +41,7 @@ internal sealed class SpeckleHttpClientHandler : DelegatingHandler
       activity?.InjectHeaders((k, v) => request.Headers.TryAddWithoutValidation(k, v));
 
       var policyResult = await _resiliencePolicy
-        .ExecuteAndCaptureAsync(
-          ctx =>
-          {
-            return base.SendAsync(request, cancellationToken);
-          },
-          context
-        )
+        .ExecuteAndCaptureAsync(ctx => base.SendAsync(request, cancellationToken), context)
         .ConfigureAwait(false);
       context.TryGetValue("retryCount", out var retryCount);
       activity?.SetTag("retryCount", retryCount);

src/Speckle.Sdk/Api/GraphQL/Client.cs

@@ -127,10 +127,10 @@ public sealed class Client : ISpeckleGraphQLClient, IClient
       activity?.SetStatus(SdkActivityStatusCode.Ok);
       return ret;
     }
-    catch (Exception ex)
+    catch (Exception)
     {
       activity?.SetStatus(SdkActivityStatusCode.Error);
-      activity?.RecordException(ex);
+      // Don't record exception as it's rethrown.
       throw;
     }
   }

src/Speckle.Sdk/Api/GraphQL/GraphQLHttpClientExtensions.cs

@@ -40,7 +40,8 @@ public static class GraphQLHttpClientExtensions
     response.EnsureGraphQLSuccess();
 
     string versionString = response.Data.data.data;
-    if (versionString == "dev")
+    //Local server builds will have a non-numerical version string
+    if (versionString == "dev" || versionString == "custom")
     {
       return new Version(999, 999, 999);
     }

src/Speckle.Sdk/Serialisation/V2/Receive/ObjectLoader.cs

@@ -171,7 +171,7 @@ public sealed class ObjectLoader(
     cancellationToken.ThrowIfCancellationRequested();
     if (Exception is not null)
     {
-      throw new SpeckleException("Error while loading", Exception);
+      throw new SpeckleException($"Error while loading: {Exception.Message}", Exception);
     }
   }
 }

src/Speckle.Sdk/Serialisation/V2/Send/ObjectSaver.cs

@@ -38,10 +38,11 @@ public sealed class ObjectSaver(
   private long _cached;
 
   private long _objectsSerialized;
+  private bool _disposed;
 
   protected override async Task SendToServerInternal(Batch<BaseItem> batch)
   {
-    if (_cancellationTokenSource.IsCancellationRequested)
+    if (IsCancelled())
     {
       return;
     }
@@ -66,7 +67,7 @@ public sealed class ObjectSaver(
     }
     catch (OperationCanceledException)
     {
-      _cancellationTokenSource.Cancel();
+      CancelSaving();
     }
 #pragma warning disable CA1031
     catch (Exception e)
@@ -91,7 +92,7 @@ public sealed class ObjectSaver(
 
   public override void SaveToCache(List<BaseItem> batch)
   {
-    if (_cancellationTokenSource.IsCancellationRequested)
+    if (IsCancelled())
     {
       return;
     }
@@ -106,7 +107,7 @@ public sealed class ObjectSaver(
     }
     catch (OperationCanceledException)
     {
-      _cancellationTokenSource.Cancel();
+      CancelSaving();
     }
 #pragma warning disable CA1031
     catch (Exception e)
@@ -123,8 +124,23 @@ public sealed class ObjectSaver(
     }
   }
 
+  private bool IsCancelled() => _disposed || _cancellationTokenSource.IsCancellationRequested;
+
+  private void CancelSaving()
+  {
+    if (IsCancelled())
+    {
+      return;
+    }
+    _cancellationTokenSource.Cancel();
+  }
+
   private void RecordException(Exception e)
   {
+    if (IsCancelled())
+    {
+      return;
+    }
     //order here matters
     logger.LogError(e, "Error in SDK: {message}", e.Message);
     Exception = e;
@@ -133,6 +149,7 @@ public sealed class ObjectSaver(
 
   public void Dispose()
   {
+    _disposed = true;
     _cancellationTokenSource.Dispose();
     sqLiteJsonCacheManager.Dispose();
   }

src/Speckle.Sdk/Serialisation/V2/Send/SerializeProcess.cs

@@ -47,6 +47,7 @@ public sealed class SerializeProcess(
     cancellationToken
   );
   private readonly ILogger<SerializeProcess> _logger = loggerFactory.CreateLogger<SerializeProcess>();
+  private bool _disposed;
 
   //async dispose
   [SuppressMessage("Usage", "CA2213:Disposable fields should be disposed")]
@@ -83,6 +84,7 @@ public sealed class SerializeProcess(
   [AutoInterfaceIgnore]
   public async ValueTask DisposeAsync()
   {
+    _disposed = true;
     await WaitForSchedulerCompletion().ConfigureAwait(false);
     await _highest.DisposeAsync().ConfigureAwait(false);
     await _belowNormal.DisposeAsync().ConfigureAwait(false);
@@ -95,7 +97,7 @@ public sealed class SerializeProcess(
     //order here matters...null with cancellation means a user did it, otherwise it's a real Exception
     if (objectSaver.Exception is not null)
     {
-      throw new SpeckleException("Error while sending", objectSaver.Exception);
+      throw new SpeckleException($"Error while sending: {objectSaver.Exception.Message}", objectSaver.Exception);
     }
     _processSource.Token.ThrowIfCancellationRequested();
   }
@@ -148,7 +150,7 @@ public sealed class SerializeProcess(
 
   private void TraverseTotal(Base obj)
   {
-    if (_processSource.Token.IsCancellationRequested)
+    if (IsCancelled())
     {
       return;
     }
@@ -162,7 +164,7 @@ public sealed class SerializeProcess(
 
   private async Task<Dictionary<Id, NodeInfo>> Traverse(Base obj)
   {
-    if (_processSource.Token.IsCancellationRequested)
+    if (IsCancelled())
     {
       return EMPTY_CLOSURES;
     }
@@ -174,7 +176,7 @@ public sealed class SerializeProcess(
       {
         // tmp is necessary because of the way closures close over loop variables
         var tmp = child;
-        if (_processSource.Token.IsCancellationRequested)
+        if (IsCancelled())
         {
           return EMPTY_CLOSURES;
         }
@@ -191,7 +193,7 @@ public sealed class SerializeProcess(
         tasks.Add(t);
       }
 
-      if (_processSource.Token.IsCancellationRequested)
+      if (IsCancelled())
       {
         return EMPTY_CLOSURES;
       }
@@ -218,7 +220,7 @@ public sealed class SerializeProcess(
       }
       _taskResultPool.Return(tasks);
 
-      if (_processSource.Token.IsCancellationRequested)
+      if (IsCancelled())
       {
         return EMPTY_CLOSURES;
       }
@@ -226,14 +228,14 @@ public sealed class SerializeProcess(
       var childClosures = _childClosurePool.Get();
       foreach (var childClosure in taskClosures)
       {
-        if (_processSource.Token.IsCancellationRequested)
+        if (IsCancelled())
         {
           return EMPTY_CLOSURES;
         }
         foreach (var kvp in childClosure)
         {
           childClosures[kvp.Key] = kvp.Value;
-          if (_processSource.Token.IsCancellationRequested)
+          if (IsCancelled())
           {
             return EMPTY_CLOSURES;
           }
@@ -242,14 +244,14 @@ public sealed class SerializeProcess(
         _currentClosurePool.Return(childClosure);
       }
 
-      if (_processSource.Token.IsCancellationRequested)
+      if (IsCancelled())
       {
         return EMPTY_CLOSURES;
       }
 
       var items = baseSerializer.Serialise(obj, childClosures, _options.SkipCacheRead, _processSource.Token);
 
-      if (_processSource.Token.IsCancellationRequested)
+      if (IsCancelled())
       {
         return EMPTY_CLOSURES;
       }
@@ -261,7 +263,7 @@ public sealed class SerializeProcess(
         progress?.Report(new(ProgressEvent.FromCacheOrSerialized, _objectCount, Math.Max(_objectCount, _objectsFound)));
         foreach (var item in items)
         {
-          if (_processSource.Token.IsCancellationRequested)
+          if (IsCancelled())
           {
             return EMPTY_CLOSURES;
           }
@@ -298,6 +300,8 @@ public sealed class SerializeProcess(
     }
   }
 
+  public bool IsCancelled() => _disposed || _processSource.IsCancellationRequested;
+
   public void RecordException(Exception e)
   {
     if (e is OperationCanceledException)
@@ -312,6 +316,11 @@ public sealed class SerializeProcess(
     {
       return;
     }
+    if (IsCancelled())
+    {
+      //if we are already cancelled, don't log or save the exceptions
+      return;
+    }
     //order here matters
     _logger.LogError(e, "Error in SDK: {message}", e.Message);
     objectSaver.Exception = e;

tests/Speckle.Sdk.Serialization.Tests/ExceptionTests.Test_Exceptions_Cache.verified.json

@@ -6,6 +6,6 @@
     "Message": "The method or operation is not implemented.",
     "Type": "NotImplementedException"
   },
-  "Message": "Error while sending",
+  "Message": "Error while sending: The method or operation is not implemented.",
   "Type": "SpeckleException"
 }

tests/Speckle.Sdk.Serialization.Tests/ExceptionTests.Test_Exceptions_Cache_ExceptionsAfter_10.verified.json

@@ -6,6 +6,6 @@
     "Message": "Count exceeded",
     "Type": "Exception"
   },
-  "Message": "Error while sending",
+  "Message": "Error while sending: Count exceeded",
   "Type": "SpeckleException"
 }

tests/Speckle.Sdk.Serialization.Tests/ExceptionTests.Test_Exceptions_Receive_Cache_fileName=False.verified.json

@@ -6,6 +6,6 @@
     "Message": "The method or operation is not implemented.",
     "Type": "NotImplementedException"
   },
-  "Message": "Error while loading",
+  "Message": "Error while loading: The method or operation is not implemented.",
   "Type": "SpeckleException"
 }

tests/Speckle.Sdk.Serialization.Tests/ExceptionTests.Test_Exceptions_Upload.verified.json

@@ -6,6 +6,6 @@
     "Message": "The method or operation is not implemented.",
     "Type": "NotImplementedException"
   },
-  "Message": "Error while sending",
+  "Message": "Error while sending: The method or operation is not implemented.",
   "Type": "SpeckleException"
 }