dotnet / SqlClient

Microsoft.Data.SqlClient provides database connectivity to SQL Server for .NET applications.
MIT License
851 stars 285 forks source link

5.2 localized `Operation cancelled by user` and does not have a specific error code #2424

Closed ionmincu closed 7 months ago

ionmincu commented 7 months ago

Is your feature request related to a problem? Please describe.

In MDS 5.2 when Operation canceled by user occurs there is no way to tell from the exception that the operation was canceled. We can't just look at the token, the application is huge, and we are using ApplicationInsights to track exceptions using ITelemetryProcessor to add custom dimentions on which exceptions to ignore.

Now because the exception message is Localized we can't reliably tell if the operation was from a canceled token. For example when the Current culture to ja.

image

image

image

Describe the solution you'd like

Describe alternatives you've considered

A clear and concise description of any alternative solutions or features you've considered.

How to repro

See te attached zip file for repro project. Or inline here:

Expand code example ### EfExample.csproj ```xml Exe net6.0 enable enable ``` ### Program.cs ```csharp using System.ComponentModel.DataAnnotations; using System.Data.Entity; using System.Globalization; namespace EfExample { internal class Program { private const string MyConnectionString = "Server=.\\sqlexpress;Database=SchoolDb;User ID=sa;Password=mypassword;TrustServerCertificate=True"; static async Task Main(string[] args) { var context = new SchoolContext(MyConnectionString); var allCourses = await context.Courses.ToListAsync(); var simulateUseRequestLocalisation = true; if (simulateUseRequestLocalisation) { var culture = new CultureInfo("ja"); CultureInfo.CurrentCulture = culture; CultureInfo.CurrentUICulture = culture; } try { var cts = new CancellationTokenSource(200); await context.Database.ExecuteSqlCommandAsync("WAITFOR DELAY '00:00:10';", cts.Token); //var resuult = await context.Courses.Where(x => x.Students.Any()).ToListAsync(cancel); } catch(Exception e) { Console.WriteLine(e); } } } [DbConfigurationType(typeof(System.Data.Entity.SqlServer.MicrosoftSqlDbConfiguration))] public class SchoolContext : DbContext { public SchoolContext(string connectionString) : base(connectionString) { Database.SetInitializer(new SchoolDBInitializer()); } public DbSet Students { get; set; } public DbSet Courses { get; set; } protected override void OnModelCreating(DbModelBuilder modelBuilder) { } } public class SchoolDBInitializer : DropCreateDatabaseAlways { protected override void Seed(SchoolContext context) { var students = new List { new Student { LastName = "S", FirstMidName = "Alex", }, new Student { LastName = "M", FirstMidName = "Bob", } }; var courses = new List { new Course { Title = "Math", Credits = 100 }, new Course { Title = "Astro", Credits = 200 }, }; context.Students.AddRange(students); context.Courses.AddRange(courses); context.SaveChanges(); base.Seed(context); } } public class Student { [Key] public int ID { get; set; } public string LastName { get; set; } public string FirstMidName { get; set; } } public class Course { [Key] public int CourseID { get; set; } public string Title { get; set; } public int Credits { get; set; } public virtual ICollection Students { get; set; } } } ```

Additional context

Microsoft.Data.SqlClient.SqlException: 'A severe error occurred on the current command.  The results, if any, should be discarded.
Operation cancelled by user.'

   at Microsoft.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, SqlCommand command, Boolean callerHasConnectionLock, Boolean asyncClose)
   at Microsoft.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)
   at Microsoft.Data.SqlClient.SqlCommand.InternalEndExecuteNonQuery(IAsyncResult asyncResult, Boolean isInternal, String endMethod)
   at Microsoft.Data.SqlClient.SqlCommand.EndExecuteNonQueryInternal(IAsyncResult asyncResult)
   at Microsoft.Data.SqlClient.SqlCommand.EndExecuteNonQueryAsync(IAsyncResult asyncResult)
   at System.Threading.Tasks.TaskFactory`1.FromAsyncCoreLogic(IAsyncResult iar, Func`2 endFunction, Action`1 endAction, Task`1 promise, Boolean requiresSynchronization)
--- End of stack trace from previous location ---
   at System.Data.Entity.Core.Objects.ObjectContext.<ExecuteStoreCommandInternalAsync>d__181.MoveNext()
   at System.Data.Entity.Core.Objects.ObjectContext.<ExecuteInTransactionAsync>d__156`1.MoveNext()
   at System.Data.Entity.SqlServer.DefaultSqlExecutionStrategy.<ExecuteAsyncImplementation>d__6`1.MoveNext()
   at System.Data.Entity.Core.Objects.ObjectContext.<ExecuteStoreCommandInternalAsync>d__180.MoveNext()
DavoudEshtehari commented 7 months ago

@ionmincu, could you add more info like the underlaying operating system, SqlClient version and if it's an issue starting from a specific SqlClient version, besides providing a simple console app to repro the issue?

ionmincu commented 7 months ago

See te attached zip file for repro project. Or inline here:

Expand code example ### EfExample.csproj ```xml Exe net6.0 enable enable ``` ### Program.cs ```csharp using System.ComponentModel.DataAnnotations; using System.Data.Entity; using System.Globalization; namespace EfExample { internal class Program { private const string MyConnectionString = "Server=.\\sqlexpress;Database=SchoolDb;User ID=sa;Password=mypassword;TrustServerCertificate=True"; static async Task Main(string[] args) { var context = new SchoolContext(MyConnectionString); var allCourses = await context.Courses.ToListAsync(); var simulateUseRequestLocalisation = true; if (simulateUseRequestLocalisation) { var culture = new CultureInfo("ja"); CultureInfo.CurrentCulture = culture; CultureInfo.CurrentUICulture = culture; } try { var cts = new CancellationTokenSource(200); await context.Database.ExecuteSqlCommandAsync("WAITFOR DELAY '00:00:10';", cts.Token); //var resuult = await context.Courses.Where(x => x.Students.Any()).ToListAsync(cancel); } catch(Exception e) { Console.WriteLine(e); } } } [DbConfigurationType(typeof(System.Data.Entity.SqlServer.MicrosoftSqlDbConfiguration))] public class SchoolContext : DbContext { public SchoolContext(string connectionString) : base(connectionString) { Database.SetInitializer(new SchoolDBInitializer()); } public DbSet Students { get; set; } public DbSet Courses { get; set; } protected override void OnModelCreating(DbModelBuilder modelBuilder) { } } public class SchoolDBInitializer : DropCreateDatabaseAlways { protected override void Seed(SchoolContext context) { var students = new List { new Student { LastName = "S", FirstMidName = "Alex", }, new Student { LastName = "M", FirstMidName = "Bob", } }; var courses = new List { new Course { Title = "Math", Credits = 100 }, new Course { Title = "Astro", Credits = 200 }, }; context.Students.AddRange(students); context.Courses.AddRange(courses); context.SaveChanges(); base.Seed(context); } } public class Student { [Key] public int ID { get; set; } public string LastName { get; set; } public string FirstMidName { get; set; } } public class Course { [Key] public int CourseID { get; set; } public string Title { get; set; } public int Credits { get; set; } public virtual ICollection Students { get; set; } } } ```

Basically in 5.1.5 the message was not localized, and in 5.2.0 it is localized to the request locale. We would like it if this message is not localized, or it should have some specific error code to be able to ignore it in alerts.

OS:

EfExample.zip

roji commented 7 months ago

Note the relationship with #26 - at least for async operations, SqlClient should be throwing TaskCanceledException which is the standard way to indicate cancellation.

ionmincu commented 7 months ago

TaskCanceledException would be fine, but I understand that this might be seen as a major breaking change.

I mostly see 5.2 making a breaking change from 5.1.5 (by translating exception messages). I would be satisfied with one of the 2 solutions (that could be implemented quickly in a minor release):

sebastienros commented 7 months ago

We are seeing the same issue while using Aspire (not Aspire specific) when running on Linux (at least WSL) and MacOS (customer reported). https://github.com/dotnet/aspire/issues/1023#issuecomment-2027844575

ErikEJ commented 7 months ago

@sebastienros and reverting to 5.1.5 "fixes" this?

sebastienros commented 7 months ago

@ErikEJ I didn't realize this was specific to a new version and only focused on the "no specific error code", I will try to confirm that, if not I will file another issue.

sebastienros commented 7 months ago

Can still repro with 5.1.5, filing a new issue

ErikEJ commented 7 months ago

@sebastienros Looking forward to your repro details

ionmincu commented 7 months ago

@ErikEJ did you check my repo details it should be the same thing for 5.1.5, just replace the version.

DavoudEshtehari commented 7 months ago

The thrown exception can be improved by including a more specific inner exception, such as TaskCanceledException as @roji mentioned, and/or avoiding the translation of the message "Operation canceled by user". The first part is a duplicate of issue #26, and the second part can be included in it.

cc @David-Engel

roji commented 7 months ago

@DavoudEshtehari note that the standard behavior wouldn't be to include TaskCanceledException as an inner exception, but rather to throw a TaskCanceledException (as the top-level exception). This is important as various code reacts differently when a TaskCanceledException is thrown (as opposed to other types). For example, if a Task is terminated because of a TaskCanceledException, its state is Canceled rather than Faulted.

So just throwing TaskCanceledException would be the standard behavior here.

DavoudEshtehari commented 7 months ago

@roji Thanks for the clarification. @ionmincu I'll close this issue, but feel free to continue the discussion here or on the related issue #26.

ionmincu commented 7 months ago

@DavoudEshtehari that issue seems to be very old. I'm talking about an issue that was introduce recently in 5.2. Can we get some kind of disable translations feature for 5.3 ?

DavoudEshtehari commented 7 months ago

@ionmincu This behavior is identical to .NET Framework, and you've encountered it because of adding support for localization in .NET as well. Unfortunately, there is no easy way to switch it off.