7

I don't understand the difference between these two implementations of calling SendMailAsync. Much of the time with the first, I'm receiving a TaskCanceledException, but with the second, everything works as expected.

I thought the two consuming methods would be equivalent, but clearly I'm missing something.

This seems related to, but opposite from: TaskCanceledException when not awaiting

// Common SmtpEmailGateway library
public class SmtpEmailGateway : IEmailGateway
{
    public Task SendEmailAsync(MailMessage mailMessage)
    {
        using (var smtpClient = new SmtpClient())
        {
            return smtpClient.SendMailAsync(mailMessage);
        }
    }
}

// Caller #1 code - Often throws TaskCanceledException
public async Task Caller1()
{
     // setup code here
     var smtpEmailGateway = new SmtpEmailGateway();
     await smtpEmailGateway.SendEmailAsync(myMailMessage);
}

// Caller #2 code - No problems
public Task Caller2()
{
     // setup code here
     var smtpEmailGateway = new SmtpEmailGateway();
     return smtpEmailGateway.SendEmailAsync(myMailMessage);
}

EDIT: It turns out the Caller2 method was also causing exceptions, I just wasn't seeing them due to the WebJobs framework this was being called by. Yuval's explanation cleared all of this up and is correct.

Community
  • 1
  • 1
Brian Vallelunga
  • 9,869
  • 15
  • 60
  • 87

2 Answers2

6

The difference between your two method calls is that the former asynchronously waits using await when being invoked. Thus, the TaskCanceledException propagates from the inner SendEmailAsync call, which is caused by the fact that you're not awaiting the async method in the using scope, which causes a race condition between the disposal of SmtpClient and the end of the async call. While in the latter, the exception is being encapsulated inside the return Task object, which I'm not sure whether you're awaiting on or not. That's why in the former, you see the exception immediately.

The first thing that should be done is to properly await on SendEmailAsync inside the gateway:

public class SmtpEmailGateway : IEmailGateway
{
    public async Task SendEmailAsync(MailMessage mailMessage)
    {
        using (var smtpClient = new SmtpClient())
        {
            return await smtpClient.SendMailAsync(mailMessage);
        }
    }
}

Then, you can use the second method which avoids the overhead of creating the state-machine. The difference is now you're guaranteeing that SmtpClient will only dispose once the async operation completes.

Yuval Itzchakov
  • 146,575
  • 32
  • 257
  • 321
  • If I have this correct: I should never return a Task from an object without awaiting it when inside that object's using statement. This is because the object will likely be disposed of before the task can be completed. Is that right? – Brian Vallelunga Nov 04 '15 at 14:25
  • And it seems my Caller2 method was just also causing exceptions that the I wasn't catching, but the WebJobs framework was catching and retrying. – Brian Vallelunga Nov 04 '15 at 14:28
  • @BrianVallelunga Exactly. That's what I was trying to convey. It was encapsulated inside the `Task`, but anyone awaiting it would make the exception propagate. – Yuval Itzchakov Nov 04 '15 at 14:32
0

I am putting this answer here, not because it's pertinent to the specifics of this question, but is pertinent to the question as asked (also, this is my second time finding this question).

For me, I was just returning the Task from inside a using.

public Task SendEmailAsync( EmailInfo email, string smtpServer, int smtpPort )
{
    using( var client = new SmtpClient( smtpServer, smtpPort ) )
    using( var msg = CreateMailMessageFromEmailInfo( email ) )
        return client.SendMailAsync( msg );
}

So I suspect what was happening here was that the usings were finishing before the Task could be returned, thus canceling the task. I changed the code so that it is async, and awaiting client.SendMailAsync, and it worked.

emery.noel
  • 1,073
  • 1
  • 9
  • 24