-2

When I do the following, I get a warning that not all code paths return a value if the catch block doesn't return an int; when that catch block returns an int, the capturedException test for null becomes unreachable unless it is placed inside a finally block. Is putting the throw inside finally{} acceptable? Is the connection closed automatically as it would be in a synchronous method that employs the using syntax?

public async Task<int> Write2Log()
{
  ExceptionDispatchInfo capturedException = null;
  using (SqlConnection conn = new SqlConnection(connectionString))
  {                   
      using (SqlCommand cmd = new SqlCommand(commandText, conn))
      {
         try
           {
         await cmd.Connection.OpenAsync();
         return await cmd.ExecuteNonQueryAsync();
            }
           catch (Exception ex)
             {
               capturedException=ExceptionDispatchInfo(ex);
               return -1;
             }

             // unreachable unless placed inside `finally` block
            if (capturedException != null)
              {
                   capturedException.Throw();
              }

      }


  }
}
Tim
  • 8,669
  • 31
  • 105
  • 183
  • 3
    Where is the `try`? Please make sure the code is correct. – Yacoub Massad Feb 01 '16 at 17:56
  • I'm not very familiar with the concept of asynchronously calling sql connections, but are you supposed to be returning the `ExecuteNonQueryAsync()` _there_? – Eric Wu Feb 01 '16 at 17:56
  • 1
    Can you also explain why do you need to catch the exception? – Yacoub Massad Feb 01 '16 at 17:59
  • Also, it __is__ supposed to be `// unreachable unless placed inside `finally` block` – Eric Wu Feb 01 '16 at 17:59
  • @Eric Wu: I have not done any work so far with the async methods in the SQLClient library either. This is new territory for me. I would like to achieve this (pseudocode) pattern `Connection.OpenAsync().Success( cmd.ExecuteNonQueryAsync().Success(doSomething() ).Failure( doSomethingElse() )`. – Tim Feb 04 '16 at 13:45
  • The whole trick with `await` is that it makes your code perfectly synchronous, while the execution is asynchronous. In the usual scenario, the difference is only in the use of `await` - everything else behaves almost exactly the same as if you used simple synchronous/blocking code. There are gotchas, but the basic approach mirrors C#'s "The simplest solution should also be the most correct solution". – Luaan Feb 04 '16 at 14:28

2 Answers2

0

your code in question seems to be incorrect but anyway you need not return in catch.....you can return in end of the method. you know that successful scenario will never reach end of method.

public async Task<int> Write2Log()
{
  ExceptionDispatchInfo capturedException = null;
  using (SqlConnection conn = new SqlConnection(connectionString))
  {                   
      using (SqlCommand cmd = new SqlCommand(commandText, conn))
      {

          try
{
         await cmd.Connection.OpenAsync();
         return await cmd.ExecuteNonQueryAsync();
}
           catch (Exception ex)
             {
               capturedException=ExceptionDispatchInfo(ex);

             }

             // unreachable unless placed inside `finally` block
            if (capturedException != null)
              {
                   capturedException.Throw();
              }

      }


  }
   return -1;
}
Viru
  • 2,228
  • 2
  • 17
  • 28
0

The code you're clumsily trying to write is equivalent to the much simpler

public async Task<int> Write2Log()
{
  using (var conn = new SqlConnection(connectionString))
  {
    using (var cmd = new SqlCommand(commandText, conn)
    {
      await conn.OpenAsync();
      return await cmd.ExecuteNonQueryAsync();
    }
  }
}

If you don't see why, you might want to refresh your knowledge about how .NET exceptions work, and how await works (and in particular, how it handles exceptions).

If you also want to observe the exception (e.g. for logging), you can use throw; to rethrow the exception at the end of the catch. There's no need to use ExceptionDispatchInfo, and there's no point in storing the exception in a local to rethrow later.

If you're instead trying to return -1 when there's an error, just do

public async Task<int> Write2Log()
{
  using (var conn = new SqlConnection(connectionString))
  {
    using (var cmd = new SqlCommand(commandText, conn)
    {
      try
      {
        await conn.OpenAsync();
        return await cmd.ExecuteNonQueryAsync();
      }
      catch
      {
        return -1;
      }
    }
  }
}

A method cannot throw an exception and return a value at the same time. You have to choose one.

Luaan
  • 62,244
  • 7
  • 97
  • 116
  • I admit to being new to async coding, and so clumsiness is to be expected. With `ExceptionDispatchInfo` I was relying upon a heavily upvoted answer here on SO for guidance. – Tim Feb 04 '16 at 14:24
  • @Tim Careful about answers you don't understand - it's very likely that there was some specific reason why the solution is complicated. `ExceptionDispatchInfo` is useful when you *don't* `await` the task. When you *do* `await` (or use `GetAwaiter().GetResult()`), it's handled automatically for you - it appears as if the code was perfectly synchronous. It's very much possible that the question you referenced is trying to work with pre-`await` code. – Luaan Feb 04 '16 at 14:27