.Net: Global exceptions vs storage specific exceptions

[markwallace-microsoft]

Important exceptions like “the collection doesn’t exists” should be agnostic of the storage type, to avoid the need to catch several exception types.

For instance, this code would be preferable and wouldn’t need changes when swapping storage type:

Try { … } 

catch (Microsoft.Extensions.VectorData.CollectionNotFoundException e) 

{ … }

as opposed to:

Try { … } 

catch (Exception e) when (e is Qdrant.CollectionNotFoundException or Postgres.CollectionNotFoundException or AzureAiSearch.CollectionNotFoundException or Milvus.CollectionNotFoundException or ...)

{ … }

[westey-m]

All connectors are expected to return standard exceptions. Any deviation is a bug.
See the following link for the specification on this:
https://learn.microsoft.com/en-us/semantic-kernel/concepts/vector-store-connectors/how-to/build-your-own-connector?pivots=programming-language-csharp#11-standard-exceptions

@adamsitnik, one that would be good to cover in our specification tests as well to confirm.

[markwallace-microsoft]

This is the current design so the remaining work to be done here is to verify this via integration tests for all connectors.

[adamsitnik]

We should definitely always throw VectorStoreOperationException, but throwing more specific exception may be tricky. Example: when the collection does not exist and we try to remove it, Redis client throws RedisServerException, but it does not set an error code of any kind (at least to my knowledge), just a specific exception message (Unknown Index name). I am afraid that it can change over time (or simply get translated at some point), so relying on it to map it to CollectionNotFoundException could be very fragile.

[dluc]

I think we need to define a few important exceptions such as Not Found, Invalid Request, Conflict and Access Denied.

Otherwise, wrapping every storage exception in a generic OperationException would still require each app to inspect the error with custom logic (e.g. custom string matching) for each storage type. Everyone would have to do this, making the code less portable since string matching would be specific to individual engines. Copy&paste expectations would fail, etc.

If this abstraction wants to be useful, it should align the behavior for important scenarios. This is the same concern I see with different key types (Guid, string, composite keys, long, ulong, etc) which lead to code duplication and lengthy switch statements to handle each storage engine.

As for breaking changes, the risk is always present, but it can be mitigated with integration tests for each storage engine. The core team can focus on strong test coverage for Azure storage options, while the broader OSS community can support with integration tests for third party connectors.

[adamsitnik]

#10958 has made some good progress, but we need to have a complete conformance test suite (#10194) to be 100% sure we don't throw any storage specific exceptions