Proposal: SmtpClient.SendMailAsync overload with cancellation

[jnm2]

Problem

Right now there's a SendAsyncCancel method which is used to cancel the event-based pre-TPL SendAsync. SendMailAsync is implemented as a wrapper around SendAsync (code) but for some reason neglects to take and apply the customary cancellation token.

This means people who want cancellation currently need to add a using statement:

using (cancellationToken.Register(smtpClient.SendAsyncCancel))
{
    await smtpClient.SendMailAsync(message);
}

Extension method for convenience, in case you want to operate on the original Task without wrapping in an async state machine:

public static Task SendMailAsync(this SmtpClient client, MailMessage message, CancellationToken cancellationToken)
{
    if (client == null) throw new ArgumentNullException(nameof(client));
    if (message == null) throw new ArgumentNullException(nameof(message));

    var cancellationRegistration = cancellationToken.Register(client.SendAsyncCancel);
    var task = client.SendMailAsync(message);
    task.ConfigureAwait(false).GetAwaiter().OnCompleted(cancellationRegistration.Dispose);
    return task;
}

The difficulty is that there's no guarantee that SendMailAsync will continue to be implemented in terms of SendAsync under the covers and that SendAsyncCancel will have any effect. Rather than an extension method, it's more future-proof to bring the responsibility of this coupling much closer to where it belongs: as a detail inside the implementation of SmtpClient.

Proposal

I propose adding these two overloads, copies of the existing overloads but appending a CancellationToken parameter:

class System.Net.Mail.SmtpClient
{
    public Task SendMailAsync(string from, string recipients, string subject, string body)
+   public Task SendMailAsync(string from, string recipients, string subject, string body, CancellationToken cancellationToken)
    public Task SendMailAsync(MailMessage message)
+   public Task SendMailAsync(MailMessage message, CancellationToken cancellationToken)
}

Implementation would likely be straightforward, with the non-cancellable overloads delegating to the cancellable ones and the SendCompletedEventHandler disposing the cancellation registration.

[jnm2]

What does api-needs-work mean? Are you waiting for me to do something?

[hillmarcus]

This is an issue I ran in to today as well. Is there anyway we can get an update on this or re-prioritize it?

[karelz]

@hillmarcus we do not plan to invest into SmtpClient - see:
https://github.com/dotnet/designs/issues/9
https://github.com/dotnet/platform-compat/blob/master/docs/DE0005.md

If there is enough need to have the overloads and the implementation is straightforward, I think we would be ok with taking a contribution. API approval should be formality in such case.

[hillmarcus]

Thanks for the response. I wasn't aware that SmtpClient had reached legacy status. I'll look in to other libraries.

…

On Sun, Nov 25, 2018, 10:49 PM Karel Zikmund ***@***.*** wrote: @hillmarcus <https://github.com/hillmarcus> we do not plan to invest into SmtpClient - see: dotnet/designs#9 <https://github.com/dotnet/designs/issues/9> https://github.com/dotnet/platform-compat/blob/master/docs/DE0005.md If there is enough need to have the overloads and the implementation is straightforward, I think we would be ok with taking a contribution. API approval should be formality in such case. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/dotnet/corefx/issues/24475#issuecomment-441518217>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ALdBEyTLuLOVQp7Ayjf26YVuv_DVhbwbks5uy3LagaJpZM4Pwfvz> .

[terrajobst]

Video

Looks good as proposed.

namespace System.Net.Mail
{
    public partial class SmtpClient
    {
        public Task SendMailAsync(MailMessage message, CancellationToken cancellationToken);
        public Task SendMailAsync(string from, string recipients, string subject, string body, CancellationToken cancellationToken);
    }
}