Is this SqlConnection / SqlCommand async wrapper both efficient and correct?

This is my first time writing async code, I intend to inject the interface IConnection into other classes in my project. Can you please tell me if the implementation class Connection, is the correct usage of await, Task, and async? The code does build, I'm more concerned with async got-ya's that I'm not aware of (I don't know what I don't know). Below is my code, I realise its a bit verbose, but the main part of the code is only about 65 lines.

```
using System;
using System.Collections.Generic;
using System.Data;
using System.Data.SqlClient;
using System.Linq;
using System.Threading.Tasks;

namespace My.Namespace
{
/// Provides both synchronous, and asynchronous methods calls to a database.
public interface IConnection
{
/// Maps the result from a stored procedure to a single custom object.
/// The type of the custom object that will be mapped from the stored procedure's result.
/// A function that takes an IDataReader, and maps the rows / columns to a custom object.
/// The connectionString to the database.
/// The name of the stored procedure.
/// The parameters that the stored procedure expects.
/// A custom object with the results from the stored procedure.
T ExecuteReaderSingle(Func reader, string connectionString, string usp, params SqlParameter[] parameters);

/// Maps the result from a stored procedure to zero or more custom objects.
/// The type of the custom object that will be mapped from the stored procedure's result.
/// A function that takes an IDataReader, and maps the rows / columns to zero or more custom objects.
/// The connectionString to the database.
/// The name of the stored procedure.
/// The parameters that the stored procedure expects.
/// Zero or more custom objects with the results from the stored procedure.
IEnumerable ExecuteReader(Func> reader, string connectionString, string usp, params SqlParam

private static T GetResultOf(Task task)
{
    return task.ContinueWith>(t =>
        {
            T tResult = t.IsFaulted ? default(T) : t.Result;
            Exception exception = t.IsFaulted ? t.Exception.InnerExceptions.First() : null;

            return new TaskResult(tResult, exception);
        }).Result;
}

How is all this different from just task.Result? That it throws only the first exception? You can achieve that by calling task.GetAwaiter().GetResult().

An advantage of that approach is that it also doesn't reset the exception's stack trace, unlike your TaskResult. (If you want to fox that, look into ExceptionDispatchInfo.)

Also, this is likely to cause deadlocks if called from code with synchronization context set (which includes the UI thread of GUI applications and request threads in ASP.NET). To fix this, you would need to use ConfigureAwait(false) for every await in this code.

But an even better option is not to expose synchronous wrappers for asynchronous methods at all.

return await ExecuteCommand(…)

If you remove both the await and the async modifier from the method, the code will work the same, only with slightly less overhead.

This doesn't really sound like Connection, since each command can be to a different database. I would consider either moving the connectionString parameter to the constructor of Connection, or renaming the class (and interface) to something else.

StackExchange Code Review Q#59189, answer score: 2