Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Async support for other operations #7

Merged
merged 7 commits into from
Apr 1, 2018

Conversation

benmccallum
Copy link
Contributor

@benmccallum benmccallum commented Mar 28, 2018

Adds on to work in issue #1 and previous PR #5.

@benmccallum
Copy link
Contributor Author

Having an exception in the Insert person async and Update person async tests I created. Stack trace below. Not exactly sure what would be causing a "System.InvalidCastException : Unable to cast object of type 'Dapper.GraphQL.Test.DbConnectionWrapper' to type 'System.Data.Common.DbConnection'.". I mean that would make sense since DbConnectionWrapper is only implementing IDbConnection and isn't a DbConnection per se. Is that Dapper accepting the interface in method sigs but casting to the concrete implementation incorrectly?

=====
Test Name: INSERT person asynchronously succeeds
Test FullName: Dapper.GraphQL.Test.InsertTests.InsertPersonAsync
Test Source: C:\src\Dapper.GraphQL\Dapper.GraphQL.Test\InsertTests.cs : line 108
Test Outcome: Failed
Test Duration: 0:00:05.465

Result StackTrace:
at Dapper.SqlMapper.d__311.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Runtime.CompilerServices.TaskAwaiter1.GetResult()
at Dapper.GraphQL.SqlInsertContextExtensions.d__11.MoveNext() in C:\src\Dapper.GraphQL\Dapper.GraphQL\Extensions\SqlInsertContextExtensions.cs:line 26 --- End of stack trace from previous location where exception was thrown --- at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Runtime.CompilerServices.TaskAwaiter1.GetResult()
at Dapper.GraphQL.Test.InsertTests.d__3.MoveNext() in C:\src\Dapper.GraphQL\Dapper.GraphQL.Test\InsertTests.cs:line 104
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
Result Message: System.InvalidCastException : Unable to cast object of type 'Dapper.GraphQL.Test.DbConnectionWrapper' to type 'System.Data.Common.DbConnection'.

@dougrday
Copy link
Member

Try calling connection.Open(); before doing Async operations. Looking at the Dapper docs, they do that there.

Still, that error message is very misleading. We should see if we can get a very direct exception message back to the consumer so they aren't scratching their heads at this one.

@benmccallum
Copy link
Contributor Author

benmccallum commented Mar 28, 2018

Weird, as you said doing connection.Open() works. Commenting it out fails. With the stack trace I posted.

Is this unique to the Async variants of Dapper methods?

We could add a check on connection.State in the Dapper.Contexts.XyzContext::ExecuteAsync methods. It's an enum with a bunch of options, but if it's definitely closed we could do a connection.Open().

It bothers me though that this is Dapper potentially handling Sync vs Async executions differently. What do you think?

@dougrday
Copy link
Member

dougrday commented Mar 29, 2018

Ah. DbConnection also has an OpenAsync() method, but the strange thing is Dapper has code to open the connection for you (which doesn't appear to be called):

https://github.com/StackExchange/Dapper/blob/071a3fd5a0e9a3ebb9b598737b94b49c56446722/Dapper/SqlMapper.Async.cs#L388

I'm not sure why that line isn't being executed - I've seen a lot of examples where dbConnection.OpenAsync() is explicitly called before working with Dapper. I think for the time being let's call it for unit tests, since it's a Dapper concern.

@benmccallum
Copy link
Contributor Author

benmccallum commented Mar 29, 2018

Hey @dougrday , looks like the issue is that for the async versions of the methods Dapper try to cast the connection to a concrete DbConnection to then open it, whereas for the standard versions they don't. It's surfacing for us because they only try open it if it's closed.

I'll download their repo tomorrow and see why they need to be casting. It could be a bug in Dapper if the cast is unnecessary or can't be made to a more generic IDbConnection.

Thanks for looking into it more. Two heads are better than one!

@benmccallum
Copy link
Contributor Author

Yea looks like IDbConnection doesn't have an OpenAsync member...

@benmccallum
Copy link
Contributor Author

Just found a relevant issue in Dapper here: DapperLib/Dapper#757

@dougrday
Copy link
Member

dougrday commented Apr 1, 2018

Looks good, thanks again :)

@dougrday dougrday merged commit 0c12d02 into landmarkhw:master Apr 1, 2018
@dougrday dougrday added this to the 0.2.2-beta milestone Apr 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants