Send email when using SmtpClient

Question

i have created loop of sending emails if they fail

I have read that is not the best practise to have thread.sleep in code as this can lead to problems. However there is using for smtp client, which dispose of the transaction before the message is send. How can i make this better this is going to be used in web application?

using (var SmtpClient = new SmtpClient()) {
    bool success = false;
    int attemts = 0;
    const int maxAttempts = 5;
    do {
        try {
            SmtpClient.Send(mailMessage);
            System.Threading.Thread.Sleep(400);
            success = true;
        } catch{
            // ok wait for request
            success = false;
            attemts ++;
        }
    } while (success && (attemts == maxAttempts));
}

Answer 1

I want to just complement Dennis's answer.

Here the simplified implementation of the SmtpClient wrapper which allows you to use retry count parameter with SendAsync method:

public class EmailSender {

    private int _currentRetryCount;

    private int _maxRetryCount;

    private MailMessage _mailMessage;

    private bool _isAlreadyRun;

    public event SendEmailCompletedEventHandler SendEmailCompleted;

    public void SendEmailAsync(MailMessage message, int retryCount) {

        if (_isAlreadyRun) {
            throw new InvalidOperationException(
              "EmailSender doesn't support multiple concurrent invocations."
            );
        }

        _isAlreadyRun = true;
        _maxRetryCount = retryCount;
        _mailMessage = message;

        SmtpClient client = new SmtpClient();
        client.SendCompleted += SmtpClientSendCompleted;

        SendMessage(client);
    }

    private void SendMessage(SmtpClient client) {
        try {
            client.SendAsync(_mailMessage, Guid.NewGuid());
        } catch (Exception exception) {
            EndProcessing(client);
        }
    }

    private void EndProcessing (SmtpClient client) {

        if (_mailMessage != null) {
            _mailMessage.Dispose();
        }

        if (client != null) {
            client.SendCompleted -= SmtpClientSendCompleted;
            client.Dispose();
        }

        OnSendCompleted(
           new SendEmailCompletedEventArgs(null, false, null, _currentRetryCount)
        );

        _isAlreadyRun = false;
        _currentRetryCount = 0;
    }

    private void SmtpClientSendCompleted(object sender, AsyncCompletedEventArgs e) {
        var smtpClient = (SmtpClient)sender;

        if(e.Error == null || _currentRetryCount >= _maxRetryCount) {
            EndProcessing(smtpClient);
        } else {
            _currentRetryCount++;
            SendMessage(smtpClient);
        }
    }

    protected virtual void OnSendCompleted(SendEmailCompletedEventArgs args) {
        var handler = SendEmailCompleted;
        if (handler != null) {
            handler(this, args);
        }
    }

}


public delegate void SendEmailCompletedEventHandler(
   object sender, SendEmailCompletedEventArgs e);

public class SendEmailCompletedEventArgs : AsyncCompletedEventArgs {
    public SendEmailCompletedEventArgs(
        Exception error,
        bool canceled,
        object userState,
        int retryCount)
        : base(error, canceled, userState) {
        RetryCount = retryCount;
    }

    public int RetryCount { get; set; }
}}

Also below is the consumer code example:

        var sender = new EmailSender();

        sender.SendEmailCompleted += (o, eventArgs) 
            => Console.WriteLine(eventArgs.RetryCount);

        sender.SendEmailAsync(new MailMessage(), 5);

There are a lot of simplifications in the code fragment above, but you should understand the main idea.