Skip to content

Commit

Permalink
.Net: [Error handling] [Part3] Replacing a few connector exceptions b…
Browse files Browse the repository at this point in the history
…y SKException (#2299)

### Motivation and Context
This PR is the first in a chain of following PRs aimed at providing a
simple, consistent, and extensible exception model. You can find more
details in the
[ADR](https://github.com/microsoft/semantic-kernel/blob/main/docs/decisions/0004-error-handling.md)
discussing error handling.

### Description
This PR changes PineconeMemoryStore, QdrantMemoryStore,
WeaviateMemoryStore, etc.. connectors to throw SKException instead of
their dedicated exceptions and removes those exceptions.

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄

---------

Co-authored-by: Shawn Callegari <[email protected]>
  • Loading branch information
SergeyMenshykh and shawncal authored Aug 5, 2023
1 parent ba93373 commit bd8d7d4
Show file tree
Hide file tree
Showing 24 changed files with 91 additions and 783 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ public async Task<IReadOnlyList<ITextResult>> GetCompletionsAsync(

if (completionResponse is null)
{
throw new OobaboogaInvalidResponseException<string>(body, "Unexpected response from Oobabooga API");
throw new SKException($"Unexpected response from Oobabooga API: {body}");
}

return completionResponse.Results.Select(completionText => new TextCompletionResult(completionText)).ToList();
Expand Down Expand Up @@ -312,7 +312,7 @@ private async Task ProcessWebSocketMessagesAsync(ClientWebSocket clientWebSocket

if (responseObject is null)
{
throw new OobaboogaInvalidResponseException<string>(messageText, "Unexpected response from Oobabooga API");
throw new SKException($"Unexpected response from Oobabooga API: {messageText}");
}

switch (responseObject.Event)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.SemanticKernel.Connectors.Memory.Chroma.Http.ApiSchema;
using Microsoft.SemanticKernel.Connectors.Memory.Chroma.Http.ApiSchema.Internal;
using Microsoft.SemanticKernel.Diagnostics;

namespace Microsoft.SemanticKernel.Connectors.Memory.Chroma;

Expand Down Expand Up @@ -41,12 +42,12 @@ public ChromaClient(string endpoint, ILogger? logger = null)
/// <param name="httpClient">The <see cref="HttpClient"/> instance used for making HTTP requests.</param>
/// <param name="endpoint">Chroma server endpoint URL.</param>
/// <param name="logger">Optional logger instance.</param>
/// <exception cref="ChromaClientException">Occurs when <see cref="HttpClient"/> doesn't have base address and endpoint parameter is not provided.</exception>
/// <exception cref="SKException">Occurs when <see cref="HttpClient"/> doesn't have base address and endpoint parameter is not provided.</exception>
public ChromaClient(HttpClient httpClient, string? endpoint = null, ILogger? logger = null)
{
if (string.IsNullOrEmpty(httpClient.BaseAddress?.AbsoluteUri) && string.IsNullOrEmpty(endpoint))
{
throw new ChromaClientException("The HttpClient BaseAddress and endpoint are both null or empty. Please ensure at least one is provided.");
throw new SKException("The HttpClient BaseAddress and endpoint are both null or empty. Please ensure at least one is provided.");
}

this._httpClient = httpClient;
Expand Down Expand Up @@ -183,7 +184,7 @@ public async Task<ChromaQueryResultModel> QueryEmbeddingsAsync(string collection
catch (HttpRequestException e)
{
this._logger.LogError(e, "{0} {1} operation failed: {2}, {3}", request.Method.Method, operationName, e.Message, responseContent);
throw new ChromaClientException($"{request.Method.Method} {operationName} operation failed: {e.Message}, {responseContent}", e);
throw new SKException($"{request.Method.Method} {operationName} operation failed: {e.Message}, {responseContent}", e);
}

return (response, responseContent);
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Copyright (c) Microsoft. All rights reserved.

using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using System.Net.Http;
using System.Runtime.CompilerServices;
Expand Down Expand Up @@ -70,10 +72,10 @@ public async Task DeleteCollectionAsync(string collectionName, CancellationToken
{
await this._chromaClient.DeleteCollectionAsync(collectionName, cancellationToken).ConfigureAwait(false);
}
catch (ChromaClientException e) when (e.CollectionDoesNotExistException(collectionName))
catch (SKException e) when (CollectionDoesNotExistException(e, collectionName))
{
this._logger.LogError("Cannot delete non-existent collection {0}", collectionName);
throw new ChromaMemoryStoreException($"Cannot delete non-existent collection {collectionName}", e);
throw new SKException($"Cannot delete non-existent collection {collectionName}", e);
}
}

Expand Down Expand Up @@ -239,7 +241,7 @@ private async Task<ChromaCollectionModel> GetCollectionOrThrowAsync(string colle
{
return
await this.GetCollectionAsync(collectionName, cancellationToken).ConfigureAwait(false) ??
throw new ChromaMemoryStoreException($"Collection {collectionName} does not exist");
throw new SKException($"Collection {collectionName} does not exist");
}

private async Task<ChromaCollectionModel?> GetCollectionAsync(string collectionName, CancellationToken cancellationToken)
Expand All @@ -248,7 +250,7 @@ await this.GetCollectionAsync(collectionName, cancellationToken).ConfigureAwait(
{
return await this._chromaClient.GetCollectionAsync(collectionName, cancellationToken).ConfigureAwait(false);
}
catch (ChromaClientException e) when (e.CollectionDoesNotExistException(collectionName))
catch (SKException e) when (CollectionDoesNotExistException(e, collectionName))
{
this._logger.LogError("Collection {0} does not exist", collectionName);

Expand Down Expand Up @@ -309,7 +311,7 @@ private MemoryRecordMetadata GetMetadataForMemoryRecord(List<Dictionary<string,

return
JsonSerializer.Deserialize<MemoryRecordMetadata>(serializedMetadata, this._jsonSerializerOptions) ??
throw new ChromaMemoryStoreException("Unable to deserialize memory record metadata.");
throw new SKException("Unable to deserialize memory record metadata.");
}

private Embedding<float> GetEmbeddingForMemoryRecord(List<float[]>? embeddings, int recordIndex)
Expand All @@ -329,5 +331,15 @@ private double GetSimilarityScore(List<double>? distances, int recordIndex)
return similarityScore;
}

/// <summary>
/// Checks if Chroma API error means that collection does not exist.
/// </summary>
/// <param name="exception">Chroma exception.</param>
/// <param name="collectionName">Collection name.</param>
private static bool CollectionDoesNotExistException(Exception exception, string collectionName)
{
return exception?.Message?.Contains(string.Format(CultureInfo.InvariantCulture, "Collection {0} does not exist", collectionName)) ?? false;
}

#endregion
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.SemanticKernel.Connectors.Memory.Pinecone.Http.ApiSchema;
using Microsoft.SemanticKernel.Connectors.Memory.Pinecone.Model;
using Microsoft.SemanticKernel.Diagnostics;

namespace Microsoft.SemanticKernel.Connectors.Memory.Pinecone;

Expand Down Expand Up @@ -258,9 +259,7 @@ public async Task DeleteAsync(
{
if (ids == null && string.IsNullOrEmpty(indexNamespace) && filter == null && !deleteAll)
{
throw new PineconeMemoryException(
PineconeMemoryException.ErrorCodes.FailedToRemoveVectorData,
"Must provide at least one of ids, filter, or deleteAll");
throw new SKException("Must provide at least one of ids, filter, or deleteAll");
}

ids = ids?.ToList();
Expand Down Expand Up @@ -583,16 +582,12 @@ private async Task<string> GetIndexHostAsync(string indexName, CancellationToken

if (pineconeIndex == null)
{
throw new PineconeMemoryException(
PineconeMemoryException.ErrorCodes.IndexNotFound,
"Index not found in Pinecone. Create index to perform operations with vectors.");
throw new SKException("Index not found in Pinecone. Create index to perform operations with vectors.");
}

if (string.IsNullOrWhiteSpace(pineconeIndex.Status.Host))
{
throw new PineconeMemoryException(
PineconeMemoryException.ErrorCodes.UnknownIndexHost,
$"Host of index {indexName} is unknown.");
throw new SKException($"Host of index {indexName} is unknown.");
}

this._logger.LogDebug("Found host {0} for index {1}", pineconeIndex.Status.Host, indexName);
Expand Down

This file was deleted.

Loading

0 comments on commit bd8d7d4

Please sign in to comment.