Azure / azure-kusto-dotnet

Azure Data Explorer (Kusto) SDK for .NET
MIT License
8 stars 5 forks source link

KustoClientRequestCanceledByUserException should inherit from OperationCancelledException #34

Closed Emeka-MSFT closed 2 months ago

Emeka-MSFT commented 2 months ago

This is related to the following issue: #30.

By convention, KustoClientRequestCanceledByUserException should inherit from OperationCancelledException.

Side Note:

ohadbitt commented 2 months ago

Hi Kusto decided to (almost) only throw KustoException type, so as for users (and ourselves) to be able to handle it together. As for the side note - it is a new exception - we do need to document it.

Emeka-MSFT commented 2 months ago

Kusto decided to (almost) only throw KustoException type, so as for users (and ourselves) to be able to handle it together.

By design, operation-cancelled-exception is a special case; i.e. it is the/that exception to the rule (of single base exception). Therefore, please reconsider; this is a wrong decision.

More Data/Justification: If every component/library decides to create their own user-cancelled-exception (which doesn't derive from OperationCancelledException), then async pattern will be broken. All code will have to:

  1. understand the dependency chain of all sdks/libraries that they reference/consume.
  2. ensure that they handle all possible (one-off) user-cancelled-exception types (in there catch block)
  3. expand test matrix to ensure that all user-cancelled-exception corner-cases are capture.
  4. devote time to always carefully review any update to these nuget packages, in case there are changes.

Summary: This decision (a) breaks c# convention, and (b) doesn't consider consumer/consumption pattern, and (c) prioritizes sdk-team's convenience over customer products complexity and cost of maintenance.