FSharp.Data.SqlClient icon indicating copy to clipboard operation
FSharp.Data.SqlClient copied to clipboard

NullReferenceException in SqlConnection.UseLocally

Open daniellittledev opened this issue 2 years ago • 1 comments

Issue Summary

I've run into a NullReferenceException internal to FSharp.Data.SqlClient.

To Reproduce

I'm not exactly sure of how to reproduce it but it's possibly caused by a timeout or database error.

Error

System.NullReferenceException: Object reference not set to an instance of an object. at
FSharp.Data.SqlClient.Internals.Extensions.SqlConnection.UseLocally(SqlConnection this, FSharpOption`1 privateConnection) at
<StartupCode$FSharp-Data-SqlClient>[email protected](Unit unitVar) at
Microsoft.FSharp.Control.AsyncPrimitives.CallThenInvoke[T,TResult](AsyncActivation`1 ctxt, TResult result1, FSharpFunc`2 part2) in D:\a\_work\1\s\src\FSharp.Core\async.fs:line 508

Expected behavior

I would expect to see a database error, not a null reference inside FSharp.Data.SqlClient.

What you can do

  • [] I am willing to contribute a PR with a unit test showcasing the issue
  • [x] I am willing to test the bug fix before next release

daniellittledev avatar Jun 23 '23 05:06 daniellittledev

@daniellittledev, thanks for the report, with the stacktrace, we know the issue surfaced from this:

https://github.com/fsprojects/FSharp.Data.SqlClient/blob/712b8cb1b00389d7ad6564b09da0638a0a39e0d3/src/SqlClient/ISqlCommand.fs#L374

Can you update us with your intuition if this is a case of SQL Server dropping a connection due to activity there?

I am not aware on the issues that came in terms of implementing the async support, but the implementation of the UseLocally doesn't strike as NRE triggering type of code, I'd assume with a full debug build, the stacktrace may highlight SqlConnection object.

I don't have the context on the UseLocally function, but it basically ensures in different contexts, that code wouldn't leave open connections if the code path is one where the library will manage the connection.

Are you creating the commands using a connection string or connection, transaction timeouts constructor?

You can see here:

https://github.com/fsprojects/FSharp.Data.SqlClient/blob/712b8cb1b00389d7ad6564b09da0638a0a39e0d3/src/SqlClient/ISqlCommand.fs#L57

and try to identify if in your context, how this would flow, down to https://github.com/fsprojects/FSharp.Data.SqlClient/blob/712b8cb1b00389d7ad6564b09da0638a0a39e0d3/src/SqlClient/ISqlCommand.fs#L105

I would suggest not using connection string constructor for production code, always have your top level code handling the overall workflow manage the connections.

Design wise, I'd consider removing the code that deals with connection string only constructors, I think this has been discussed a bit back & forth, but even in scripting context, it is bad practice (you surrender ability for code to be handled in a transaction, or just deal with basic command timeout setup).

I would on the other hand, provide a constructor parameter of type unit -> IDbCommand or unit -> SqlCommand that would also replace how currently, you have to pass 3 things to do it properly (which can be worked around with SRTP, thankfully).

Sorry for not having seen this earlier, and please feel free to come back with more insights about what may be occurring in your case.

smoothdeveloper avatar Jul 05 '23 12:07 smoothdeveloper