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

Bug: Async methods use synchronous calls #589

Closed
BieleckiLtd opened this issue Sep 21, 2020 · 3 comments
Closed

Bug: Async methods use synchronous calls #589

BieleckiLtd opened this issue Sep 21, 2020 · 3 comments
Assignees
Labels
bug Something isn't working deployed Feature or bug is deployed at the current release fixed The bug, issue, incident has been fixed.

Comments

@BieleckiLtd
Copy link
Contributor

public static Task<IEnumerable<TResult>> ExecuteQueryAsync<TResult>() is a wrapper for ExecuteQueryAsyncInternal(). This one encapsulates private static DbCommand CreateDbCommandForExecution() which uses synchronous EnsureOpen() call. This renders the entire outer method synchronous if user does not opt in to EnsureOpenAsync() themself. Maybe new async method private static DbCommand CreateDbCommandForExecutionAsync() is needed?

Examples:

// this method will return asap no matter if the connection is closed or open
public async Task<int> GetSomethingAsync()
{
    using var connection = await new SqlConnection(connectionString).EnsureOpenAsync().ConfigureAwait(false);
    var result = await connection
        .ExecuteQueryAsync<int>(@"SELECT 1;")
        .ConfigureAwait(false);
    return result.FirstOrDefault();
}
// if the connection is closed ExecuteQueryAsync is trying to open it, but synchronously:
public async Task<int> GetSomethingAsync()
{
    using var connection = new SqlConnection(connectionString);
    var result = await connection
        .ExecuteQueryAsync<int>(@"SELECT 1;")
        .ConfigureAwait(false);
    return result.FirstOrDefault();
}
@mikependon mikependon self-assigned this Sep 21, 2020
@mikependon mikependon added the bug Something isn't working label Sep 21, 2020
@mikependon mikependon pinned this issue Sep 21, 2020
@mikependon
Copy link
Owner

Great findings! Yes, an overloaded method is needed for this. I will definitely handle this!

As you mentioned, the fix is simple, we can create separate methods and named them CreateDbCommandForExecution and CreateDbCommandForExecutionAsync respectively. Make the current method as internal named CreateDbCommandForExecutionInternal. Then, transfer only the calls to the EnsureOpen and EnsureOpenAync methods to the separated methods, and call this Internal method internally from the 2 separated methods, making sure we do not have repetitive code written.

@mikependon mikependon changed the title Bug? Async methods use synchronous calls Bug: Async methods use synchronous calls Sep 22, 2020
mikependon added a commit that referenced this issue Sep 22, 2020
@mikependon mikependon added the fixed The bug, issue, incident has been fixed. label Sep 22, 2020
@mikependon mikependon unpinned this issue Sep 22, 2020
mikependon added a commit that referenced this issue Sep 22, 2020
@mikependon mikependon added the deployed Feature or bug is deployed at the current release label Sep 24, 2020
@mikependon
Copy link
Owner

This is now available at RepoDb v1.12.0.

@mikependon
Copy link
Owner

This has been fixed and is now available at RepoDb v1.12.3. Closing this incident ticket now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working deployed Feature or bug is deployed at the current release fixed The bug, issue, incident has been fixed.
Projects
None yet
Development

No branches or pull requests

2 participants