Make Exception<T> base class non-generic and clean up derived types (#…

…701) ### Motivation and Context Fixes #573 ### Description - Make the base class non-generic and rename it to SKException - Remove unnecessary private ctors from derived types - Clean up how messages are generated
microsoft · Apr 29, 2023 · 923e6d9 · 923e6d9
1 parent 5130c53
commit 923e6d9
Show file tree

Hide file tree

Showing 13 changed files with 489 additions and 431 deletions.
diff --git a/dotnet/src/Connectors/Connectors.AI.OpenAI/CustomClient/OpenAIClientBase.cs b/dotnet/src/Connectors/Connectors.AI.OpenAI/CustomClient/OpenAIClientBase.cs
@@ -200,7 +200,7 @@ private async Task<T> ExecutePostRequestAsync<T>(string url, string requestBody,
 
             if (response == null)
             {
-                throw new AIException(AIException.ErrorCodes.NoResponse, "Empty response");
+                throw new AIException(AIException.ErrorCodes.NoResponse);
             }
 
             this.Log.LogTrace("HTTP response: {0} {1}", (int)response.StatusCode, response.StatusCode.ToString("G"));

diff --git a/dotnet/src/Connectors/Connectors.Memory.Qdrant/Diagnostics/QdrantMemoryException.cs b/dotnet/src/Connectors/Connectors.Memory.Qdrant/Diagnostics/QdrantMemoryException.cs
@@ -5,11 +5,81 @@
 
 namespace Microsoft.SemanticKernel.Connectors.Memory.Qdrant.Diagnostics;
 
+#pragma warning disable CA1032 // Implement standard exception constructors
+
 /// <summary>
-/// Custom exceptions for the Qdrant connector.
+/// Exception thrown for errors related to the Qdrant connector.
 /// </summary>
-public class QdrantMemoryException : Exception<QdrantMemoryException.ErrorCodes>
+public class QdrantMemoryException : SKException
 {
+    /// <summary>
+    /// Initializes a new instance of the <see cref="QdrantMemoryException"/> class with a provided error code.
+    /// </summary>
+    /// <param name="errorCode">The error code.</param>
+    public QdrantMemoryException(ErrorCodes errorCode)
+        : this(errorCode, message: null, innerException: null)
+    {
+    }
+
+    /// <summary>
+    /// Initializes a new instance of the <see cref="QdrantMemoryException"/> class with a provided error code and message.
+    /// </summary>
+    /// <param name="errorCode">The error code.</param>
+    /// <param name="message">The exception message.</param>
+    public QdrantMemoryException(ErrorCodes errorCode, string? message)
+        : this(errorCode, message, innerException: null)
+    {
+    }
+
+    /// <summary>
+    /// Initializes a new instance of the <see cref="QdrantMemoryException"/> class with a provided error code and inner exception.
+    /// </summary>
+    /// <param name="errorCode">The error code.</param>
+    /// <param name="innerException">The exception that is the cause of the current exception.</param>
+    public QdrantMemoryException(ErrorCodes errorCode, Exception? innerException)
+        : this(errorCode, message: null, innerException)
+    {
+    }
+
+    /// <summary>
+    /// Initializes a new instance of the <see cref="QdrantMemoryException"/> class with a provided error code, message, and inner exception.
+    /// </summary>
+    /// <param name="errorCode">The error code.</param>
+    /// <param name="message">A string that describes the error.</param>
+    /// <param name="innerException">The exception that is the cause of the current exception.</param>
+    public QdrantMemoryException(ErrorCodes errorCode, string? message, Exception? innerException)
+        : base(GetDefaultMessage(errorCode, message, innerException), innerException)
+    {
+        this.ErrorCode = errorCode;
+    }
+
+    /// <summary>
+    /// Gets the error code for this exception.
+    /// </summary>
+    public ErrorCodes ErrorCode { get; }
+
+    /// <summary>Translate the error code into a default message.</summary>
+    private static string GetDefaultMessage(ErrorCodes errorCode, string? message, Exception? innerException)
+    {
+        if (message is not null)
+        {
+            return message;
+        }
+
+        string description = errorCode switch
+        {
+            ErrorCodes.UnableToDeserializeRecordPayload => "Unable to deserialize record payload",
+            ErrorCodes.FailedToUpsertVectors => "Failed to upsert vectors",
+            ErrorCodes.FailedToGetVectorData => "Failed to get vector data",
+            ErrorCodes.FailedToRemoveVectorData => "Failed to remove vector data",
+            ErrorCodes.FailedToConvertMemoryRecordToQdrantVectorRecord => "Failed to convert memory record to Qdrant vector record",
+            ErrorCodes.FailedToConvertQdrantVectorRecordToMemoryRecord => "Failed to convert Qdrant vector record to memory record",
+            _ => $"Unknown error ({errorCode:G})",
+        };
+
+        return innerException is not null ? $"{description}: {innerException.Message}" : description;
+    }
+
     /// <summary>
     /// Error codes for the Qdrant connector exceptions.
     /// </summary>
@@ -18,7 +88,7 @@ public enum ErrorCodes
         /// <summary>
         /// Unknown error.
         /// </summary>
-        UnknownError,
+        UnknownError = -1,
 
         /// <summary>
         /// Failed to deserialize the record payload.
@@ -50,47 +120,4 @@ public enum ErrorCodes
         /// </summary>
         FailedToConvertQdrantVectorRecordToMemoryRecord
     }
-
-    /// <summary>
-    /// Initializes a new instance of the <see cref="QdrantMemoryException"/> class with error code unknown.
-    /// </summary>
-    /// <param name="message">The exception message.</param>
-    public QdrantMemoryException(string message)
-        : this(ErrorCodes.UnknownError, message)
-    {
-    }
-
-    /// <summary>
-    /// Initializes a new instance of the <see cref="QdrantMemoryException"/> class with a provided error code.
-    /// </summary>
-    /// <param name="error">The error code.</param>
-    /// <param name="message">The exception message.</param>
-    public QdrantMemoryException(ErrorCodes error, string message)
-        : base(error, message)
-    {
-    }
-
-    /// <summary>
-    /// Initializes a new instance of the <see cref="QdrantMemoryException"/> class with an inner exception.
-    /// </summary>
-    /// <param name="error">The error code.</param>
-    /// <param name="message">The exception message.</param>
-    /// <param name="innerException">The inner exception</param>
-    public QdrantMemoryException(ErrorCodes error, string message, Exception innerException)
-        : base(BuildMessage(error, message), innerException)
-    {
-    }
-
-    private QdrantMemoryException()
-    {
-    }
-
-    private static string BuildMessage(ErrorCodes error, string? message)
-    {
-        return message != null ? $"{error.ToString("G")}: {message}" : error.ToString("G");
-    }
-
-    private QdrantMemoryException(string message, System.Exception innerException) : base(message, innerException)
-    {
-    }
 }
diff --git a/dotnet/src/Connectors/Connectors.Memory.Qdrant/QdrantMemoryStore.cs b/dotnet/src/Connectors/Connectors.Memory.Qdrant/QdrantMemoryStore.cs
@@ -85,8 +85,7 @@ public async Task<string> UpsertAsync(string collectionName, MemoryRecord record
 
         if (vectorData == null)
         {
-            throw new QdrantMemoryException(QdrantMemoryException.ErrorCodes.FailedToConvertMemoryRecordToQdrantVectorRecord,
-                $"Failed to convert MemoryRecord to QdrantVectorRecord");
+            throw new QdrantMemoryException(QdrantMemoryException.ErrorCodes.FailedToConvertMemoryRecordToQdrantVectorRecord);
         }
 
         try
@@ -100,7 +99,6 @@ public async Task<string> UpsertAsync(string collectionName, MemoryRecord record
         {
             throw new QdrantMemoryException(
                 QdrantMemoryException.ErrorCodes.FailedToUpsertVectors,
-                $"Failed to upsert due to HttpRequestException: {ex.Message}",
                 ex);
         }
 
@@ -125,7 +123,6 @@ public async Task<string> UpsertAsync(string collectionName, MemoryRecord record
         {
             throw new QdrantMemoryException(
                 QdrantMemoryException.ErrorCodes.FailedToUpsertVectors,
-                $"Failed to upsert due to HttpRequestException: {ex.Message}",
                 ex);
         }
 
@@ -157,14 +154,12 @@ public async Task<string> UpsertAsync(string collectionName, MemoryRecord record
         {
             throw new QdrantMemoryException(
                 QdrantMemoryException.ErrorCodes.FailedToGetVectorData,
-                $"Failed to get vector data from Qdrant: {ex.Message}",
                 ex);
         }
         catch (MemoryException ex)
         {
             throw new QdrantMemoryException(
                 QdrantMemoryException.ErrorCodes.FailedToConvertQdrantVectorRecordToMemoryRecord,
-                $"Failed deserialize Qdrant response to Memory Record: {ex.Message}",
                 ex);
         }
     }
@@ -216,14 +211,12 @@ public async Task<string> UpsertAsync(string collectionName, MemoryRecord record
         {
             throw new QdrantMemoryException(
                 QdrantMemoryException.ErrorCodes.FailedToGetVectorData,
-                $"Failed to get vector data from Qdrant: {ex.Message}",
                 ex);
         }
         catch (MemoryException ex)
         {
             throw new QdrantMemoryException(
                 QdrantMemoryException.ErrorCodes.FailedToConvertQdrantVectorRecordToMemoryRecord,
-                $"Failed deserialize Qdrant response to Memory Record: {ex.Message}",
                 ex);
         }
     }
@@ -262,7 +255,6 @@ public async Task RemoveAsync(string collectionName, string key, CancellationTok
         {
             throw new QdrantMemoryException(
                 QdrantMemoryException.ErrorCodes.FailedToRemoveVectorData,
-                $"Failed to remove vector data from Qdrant {ex.Message}",
                 ex);
         }
     }
@@ -291,7 +283,6 @@ public async Task RemoveWithPointIdAsync(string collectionName, string pointId,
         {
             throw new QdrantMemoryException(
                 QdrantMemoryException.ErrorCodes.FailedToRemoveVectorData,
-                $"Failed to remove vector data from Qdrant {ex.Message}",
                 ex);
         }
     }
@@ -314,7 +305,6 @@ public async Task RemoveWithPointIdBatchAsync(string collectionName, IEnumerable
         {
             throw new QdrantMemoryException(
                 QdrantMemoryException.ErrorCodes.FailedToRemoveVectorData,
-                $"Error in batch removing data from Qdrant {ex.Message}",
                 ex);
         }
     }
@@ -433,8 +423,7 @@ private async Task<QdrantVectorRecord> ConvertFromMemoryRecordAsync(string colle
 
         if (vectorData == null)
         {
-            throw new QdrantMemoryException(QdrantMemoryException.ErrorCodes.FailedToConvertMemoryRecordToQdrantVectorRecord,
-                $"Failed to convert MemoryRecord to QdrantVectorRecord");
+            throw new QdrantMemoryException(QdrantMemoryException.ErrorCodes.FailedToConvertMemoryRecordToQdrantVectorRecord);
         }
 
         return vectorData;

diff --git a/dotnet/src/Connectors/Connectors.Memory.Qdrant/QdrantVectorRecord.cs b/dotnet/src/Connectors/Connectors.Memory.Qdrant/QdrantVectorRecord.cs
@@ -80,7 +80,7 @@ public static QdrantVectorRecord FromJsonMetadata(string pointId, IEnumerable<fl
         }
         else
         {
-            throw new QdrantMemoryException(QdrantMemoryException.ErrorCodes.UnableToDeserializeRecordPayload, "Failed to deserialize payload");
+            throw new QdrantMemoryException(QdrantMemoryException.ErrorCodes.UnableToDeserializeRecordPayload);
         }
     }
 }