InvokeAsync on a stored procedure does not catch exceptions for procs without return data

[replicaJunction]

Description

Some stored procedures return result sets of data, but others just return int status codes to indicate success or failure. SQLProvider appears to ignore those status codes, and calling Invoke or InvokeAsync on one of those procedures returns Unit or Async<Unit> respectively.

When one of these procedures throws an error, Invoke raises a SqlException with the text of the error thrown from the procedure. This can be caught using a normal try...with block. However, the InvokeAsync method does not appear to catch this error, and the code continues as if the procedure was successful.

Also mentioned in the last comment in #535.

Repro steps

Use this SQL to create a table, procedure, and some example rows:

-- Create example table
DROP TABLE IF EXISTS [dbo].[TempTesting]
GO

CREATE TABLE [dbo].[TempTesting](
	[Id] [int] IDENTITY(1,1) NOT NULL,
	[Name] [varchar](50) NOT NULL,
    PRIMARY KEY CLUSTERED ([Id] ASC)
);
GO

-- Create example stored procedure
CREATE OR ALTER PROCEDURE [dbo].[spDoTempTesting]
    @Name VARCHAR (50)
AS
BEGIN
    DECLARE
         @existingId INT
        ,@errorMsg VARCHAR(50)
        ;

    SELECT @existingId = [Id] FROM [dbo].[TempTesting] WHERE Name = @Name;

    IF @existingId IS NULL
    BEGIN
        SET @errorMsg = CONCAT(
            'Could not find name [',
            @Name,
            ']'
        );
        THROW 50001, @errorMsg, 1
    END

    --SELECT * FROM [dbo].[TempTesting] WHERE Id = @existingId;
    RETURN 0
END
GO

-- Create example data
INSERT INTO [dbo].[TempTesting] ([Name]) VALUES ('John'), ('Sarah'), ('George')
GO

Now, run this F# code:

// #r statements excluded for brevity
// Also, assume the variable connectionString here represents a valid connection string.

open FSharp.Data.Sql

type sql = SqlDataProvider<
                ConnectionString = connectionString,
                DatabaseVendor = Common.DatabaseProviderTypes.MSSQLSERVER
                >


let ctx = sql.GetDataContext(SelectOperations.DatabaseSide)

let f name =
    try
        let normalResult = ctx.Procedures.SpDoTempTesting.Invoke(``@Name``=name)
        printfn "Normal result succeeded:\n%A\n\n" normalResult
    with e ->
        eprintfn "Normal result failed:\n%A\n\n" e

    let asyncResult =
        ctx.Procedures.SpDoTempTesting.InvokeAsync(``@Name``=name)
        |> Async.Catch
        |> Async.RunSynchronously

    match asyncResult with
        | Choice1Of2 c -> printfn "Async result succeeded:\n%A\n\n" c
        | Choice2Of2 c -> eprintfn "Async result failed:\n%A\n\n" c


// Should not error, since we created a John record
f "John"

// Should error, since we did not create this one
f "NotARealName"

These are the results of running the above code in F# interactive:

Normal result succeeded:
<null>


Async result succeeded:
<null>


Async result succeeded:
<null>


Normal result failed:
System.Data.SqlClient.SqlException (0x80131904): Could not find name [NotARealName]
   ...stack trace...

Finally, for comparison, modify and run the SQL by uncommenting the last SELECT line and commenting the RETURN line at the end. Repeat the F# code and compare the results.

Normal result succeeded:
FSharp.Data.Sql.Common.SqlEntity


Async result succeeded:
FSharp.Data.Sql.Common.SqlEntity


Normal result failed:
System.Data.SqlClient.SqlException (0x80131904): Could not find name [NotARealName]
    ...stack trace...

Async result failed:
System.AggregateException: One or more errors occurred. ---> System.Data.SqlClient.SqlException: Could not find name [NotARealName]
   at System.Data.SqlClient.SqlCommand.<>c.<ExecuteDbDataReaderAsync>b__180_0(Task`1 result)
   at System.Threading.Tasks.ContinuationResultTaskFromResultTask`2.InnerInvoke
   at System.Threading.Tasks.Task.Execute()
   --- End of inner exception stack trace ---
---> (Inner Exception #0) System.Data.SqlClient.SqlException (0x80131904): Could not find name [NotARealName]
   at System.Data.SqlClient.SqlCommand.<>c.<ExecuteDbDataReaderAsync>b__180_0(Task`1 result)
   at System.Threading.Tasks.ContinuationResultTaskFromResultTask`2.InnerInvoke
   at System.Threading.Tasks.Task.Execute()
ClientConnectionId:ab1983fc-944c-4afa-af60-d28a758a9af8
Error Number:50001,State:1,Class:16<---

Expected behavior

InvokeAsync should return Choice2Of2 with an exception in both test cases.

Actual behavior

In the first case, InvokeAsync appears to succeed, while Invoke returns a clear failure message. In the second test case, both Invoke and InvokeAsync failed as expected.

Known workarounds

I can think of two workarounds:

1. Modify all stored procedures to return data, not just result codes
2. Modify all F# code calling stored procedures to be synchronous

Neither of these are great options.

Related information

• Used database: Microsoft SQL Server 14.0.2027
• Operating system: Windows 10
• Branch: NuGet release, version 1.1.76
• .NET Runtime, CoreCLR or Mono Version: .NET Core 3.1
• Performance information, links to performance testing scripts: N/A

[Thorium]

I think the problem is around here:

SQLProvider/src/SQLProvider/SqlDesignTime.fs

Line 408 in 6a089a0

return ()

Instead of return () it could do Aync.Catch and then in case of error, return the error, not empty.

[Thorium]

Do you get this compile, do you want to send a PR?

[replicaJunction]

I'm afraid I'm having some trouble getting a working build environment set up on my machine - I can't compile the code as-is before I make any changes.

[Thorium]

This should be now fixed in the most recent version of SQLProvider (Nuget package).

[TheJayMann]

After this fix, it appears that I am getting a compile error for every InvokeAsync in my project. I'm building with Visual Studio, but compiling to netstandard2.0, while having also tried netcoreapp3.1. I'm not able to try out net472 on this project, as there are other netcoreapp3.1 depending on this project. The compile error is error FS1109: A reference to the type 'Microsoft.FSharp.Core.FSharpChoice``2.Choice2Of2' in assembly 'FSharp.Core' was found, but the type could not be found in that assembly along with a corresponding error FS0193: The module/namespace 'Microsoft.FSharp.Core.FSharpChoice``2' from compilation unit 'FSharp.Core' did not contain the namespace, module or type 'Choice2Of2'. Downgrading back to 1.1.76 removes the errors from the build, and, upon restarting Visual Studio, removes the errors from the IDE.

If I use Choice<_,_> and Choice2Of2 directly in my code, it doesn't appear to have any issues.

[replicaJunction]

I thought I was going crazy when the same thing happened to me. Thanks for pointing it out.

I can also confirm this behavior using a library project targeting .NET Standard 2.1. Version 1.1.76 (from NuGet) works without error, and versions 1.1.78 and 1.1.79 both cause the error message detailed above.

[Thorium]

Sounds strange, I'm using dotnet.exe 3.1.101 targetting netcoreapp2.0 and calling async stored procedure works fine.

Now, one thing I did for over 1.1.76 was that I changed the FSharp.Core from 4.2.3 to 4.5.4
So somehow something is now referencing an incorrect core-version. What version does your project use? Should I revert back that change?

[replicaJunction]

My project uses FSharp.Core 4.7.0. I wonder if they made changes to the Choice type?

[Thorium]

Maybe the real reason is actually the Choice-record on the quotation code. Let me try to fix it another way then. Try the 1.1.80, please, just pushed that to the NuGet.

[Thorium]

I assume you know using exceptions as a flow control is a bad practice in .NET:

• The performance hit, exception handling code is not optimised as well in the compiler
• The possible leak of resources, are the disposables cleaned in all the intermediate libraries correctly.
• Makes code more harder to reason about, thus increases cost of maintenance.

But I totally agree this as an issue, exceptions shouldn't be swallowed on any conditions.

[TheJayMann]

I can try this out shortly. However, I wanted to emphasize that I was not using dotnet.exe at all, but, rather, using Visual Studio while targetting netcoreapp3.1, which, I assume, uses F# compiler running on .NET Framework. My previous attempts using dotnet.exe wouldn't work. I think something to do with not finding dependencies or something.

[replicaJunction]

From a philosophical point of view, I take the stance that "exceptions should be for exceptional cases." If I can anticipate something going wrong, I should explicitly address that case before it could happen.

That said, the reason this issue came to my attention was that I was writing code to use someone else's stored procedures that would sometimes raise errors. In this scenario, I don't have the ability to change the stored procedure. I can do my best to code around the cases I know will fail, but I need to be able to handle failures as well (log the exception and the faulty data, stop executing that branch, etc.).

Somewhere along the pipeline, those SQL errors get translated to SQLExceptions. (I think that's in SqlClient itself, so that's no fault of this library.) I'd be much happier if that was translated to some kind of "SqlResult" class instead, but I've got to work with the tools I'm given. :)

[replicaJunction]

Unfortunately, version 1.1.80 fixes the previous issue and allows the code to compile, but it doesn't seem to fix the original problem. I still don't see the exception when using InvokeAsync.

Running normal test with name John
Normal result succeeded:
<null>


Running async test with name John
Async result succeeded:
<null>


Running normal test with name NotARealName
Normal result failed:
System.Data.SqlClient.SqlException (0x80131904): Could not find name [NotARealName]
   ...stack trace...
   
ClientConnectionId:21bd88e4-0341-43e4-8679-42f31969843a
Error Number:50001,State:1,Class:16


Running async test with name NotARealName
Async result succeeded:
<null>

[replicaJunction]

@TheJayMann I haven't been able to compile F# projects using this library using dotnet build at all. I believe that's the issue described in #580, #626, and #645. Using MSBuild (either from VS or from the command line) seems to avoid the issue.

[Thorium]

Ok, found it. It wasn't Async.Catch related, instead the Async.AwaitIAsyncResult in the source code swallowed the exception generated by ExecuteNonQueryAsync.
Nuget 1.1.81 is now pushed and I'm quite sure it will fix this problem. :-)

P.S. I can build via both dotnet and VS, but I don't know what I've done differently than others.