Wrong exception message for cancellation of request

cplankl commented 3 months ago

Hey there,

I`m quite happy with the cancellation feature for the Azure Kusto SDK introduced in Version 11.3.0.

I tried to implement the new cancellation feature using the latest Microsoft.Azure.Kusto.Data package (Version 12.1.2) in my backend API but somehow I find the exception type and also the exception message a bit misleading or maybe I do it wrong.

I get the following message after cancellation in e.g. Postman:

fail: KustoWebApiWithControllers.Controllers.DataExplorer[0]
      Failed to execute query
      [0]Kusto.Data.Exceptions.KustoClientTimeoutException: Kusto client timed-out when sending a request to the service.
      Error details:
      DataSource='<resource>.westeurope.kusto.windows.net/v1/rest/query',
      DatabaseName='<database>',
      ClientRequestId='KD2RunQuery;343c4f33-a989-41be-a345-f85a18cf8e57',
      Timestamp='2024-04-12T11:32:49.1559586Z',
      Timeout='00:06:00'.
      Timestamp=2024-04-12T11:32:49.1559586Z
      ClientRequestId=KD2RunQuery;343c4f33-a989-41be-a345-f85a18cf8e57
      ActivityId=9d5048d7-e689-4193-9a42-3bbe8079c317
      ActivityType=KD.RestClient.ExecuteQuery
      MachineName=DESKTOP-2FSPV7M
      ProcessName=KustoWebApiWithControllers
      ProcessId=7508
      ThreadId=31140
      ActivityStack=(Activity stack: CRID=KD2RunQuery;343c4f33-a989-41be-a345-f85a18cf8e57 ARID=9d5048d7-e689-4193-9a42-3bbe8079c317 > KD.RestClient.ExecuteQuery/9d5048d7-e689-4193-9a42-3bbe8079c317)
      MonitoredActivityContext=(ActivityType=KD.RestClient.ExecuteQuery, Timestamp=2024-04-12T11:32:48.6158062Z, ParentActivityId=9d5048d7-e689-4193-9a42-3bbe8079c317, TimeSinceStarted=540.1569 [ms])ErrorMessage=
      DataSource=<resource>.westeurope.kusto.windows.net/v1/rest/query
      DatabaseName=<database>
      ClientRequestId=KD2RunQuery;343c4f33-a989-41be-a345-f85a18cf8e57
      Timeout=00:06:00

         at Kusto.Data.Net.Client.RestClient2.MakeHttpRequestAsyncImpl(RestApi restApi, String address, String csl, String ns, String databaseName, Boolean streaming, ClientRequestProperties properties, ServiceModelTimeoutKind timeoutKind, String clientRequestId, Stream body, StreamProperties streamProperties, CancellationToken cancellationToken, KustoProtocolRequest request, String hostHeaderOverride)
         at Kusto.Cloud.Platform.Utils.MonitoredActivity.InvokeAsync[TActivityType,TResult](TActivityType activityType, Func`1 func, String clientRequestId)
         at Kusto.Cloud.Platform.Utils.MonitoredActivity.InvokeAsync[TActivityType,TResult](TActivityType activityType, Func`1 func, String clientRequestId)
         at Kusto.Data.Net.Client.RestClient2.MakeHttpRequestAsync(RestApi restApi, String baseAddress, String relativeAddress, String clientRequestIdPrefix, String ns, String databaseName, String csl, String addr, Boolean streaming, ClientRequestProperties properties, ServiceModelTimeoutKind timeoutKind, StreamProperties streamProperties, CancellationToken cancellationToken)
         at Kusto.Data.Net.Client.RestClient2.ExecuteQueryAsync(String databaseName, String query, ClientRequestProperties properties, CancellationToken cancellationToken)
         at KustoWebApiWithControllers.Controllers.DataExplorer.ExecuteQueryAsync(String query, ClientRequestProperties props, CancellationToken cancellationToken) in C:\dev\OwnProjects\KustoWebApiWithControllers\KustoWebApiWithControllers\Controllers\WeatherForecastController.cs:line 98

I removed default database name and resource name in the logs

The type is a bit misleading here: KustoClientTimeoutException. I would expect some kind of TaskCancelledException or an OperationCancelledException or any other property to give me a hint that this operation is cancelled. Maybe also in an inner exception

Also the message is a bit misleading: Kusto client timed-out when sending a request to the service.

Maybe I did something wrong here in implementing this feature?

I also added the code I used to generate this example as zip file. KustoWebApiWithControllers.zip

AsafMah commented 2 months ago

Will be fixed in the next version of the client, I'll update when it does.

AsafMah commented 1 month ago

We just released 12.2.2 - try it out

cplankl commented 1 month ago

Awesome! Now the KustoClientRequestCanceledByUserException is thrown when cancellation was requested! Thank you very much! :-)

Emeka-MSFT commented 1 week ago

By convention, KustoClientRequestCanceledByUserException should inherit from OperationCancelledException.

It appears to inherit from KustoClientException -> KustoException -> Exception.
By not inheriting from OperationCancelledException, calling code will not be able to reuse catch-blocks that gracefully handle the user cancellation scenarios.

Side Note:

Please review other instances and consider addressing them as well.
~~If you need this to be new issue, then please let me know, or create the issue.~~ #34

Azure / azure-kusto-dotnet

Wrong exception message for cancellation of request #30