1

I have written a simple class for me to send reports which works well, however I now need to incorporate a log to check whether the send was successful or not.

A quick brief on my app: It has a main form that you can manually run the reports via buttons and if the app is run from the command line then the form is hidden.

When the form is shown the SmtpClient.SendComplete event fires, but when the form is hidden it does not and also doesn't send the email. I have tried every which way to get it to work.

public static async Task SendAsync(string sendTo, string subject, string body = "", string attachment = "")
{
#if (DEBUG)
    // If in DEBUG mode then send reports to lee.
    sendTo = "developer";
#endif
    try
    {
        SmtpClient client = new SmtpClient("smtp.server", 587)
        { Credentials = new NetworkCredential("username", "password") };
        MailMessage message = new MailMessage
        { From = new MailAddress("mailFrom") };
        message.To.Add(sendTo);
        message.Subject = subject;
        message.Body = body;
        if (!string.IsNullOrEmpty(attachment))
            message.Attachments.Add(new Attachment(attachment));
        client.SendCompleted += (s,e) =>
            {
                if (e.Error != null)
                {
                    Logger.FileLog(message.Subject, e.Error.ToString());
                }
                else
                {
                    Logger.FileLog(message.Subject, "Message Sent");
                }
                client.Dispose();
                message.Dispose();
            };
        Logger.FileLog(_subject, "Running Async");
        await client.SendMailAsync(message);

        Logger.FileLog(_subject, "Finished");
    }
    catch (Exception e)
    {
        Logger.FileLog(subject, $"Error {e.HResult}, {e.Message}, {e.InnerException.ToString()}");
    }
}

I have tried async Task, async void, SendAsync, SendMailAsync, loops to wait for a callback.

Some of the logging is there just so I can see how far it got when running from the command line and it only ever gets as far as "Running Async" which is before it tries to send the email.

The code for detecting whether to show or hide the form as requested, the original project was written in VB, the above code is in a class library which was working until I tried to run Async.

Private Sub Form1_Load(sender As Object, e As EventArgs) Handles Me.Load
    If CommandLineArgs.Count > 0 Then
        Visible = False
        RunSilent = True
        ParseCommandLine()
        Close()
    Else
        'Update to add from db, XML or Excel
        Dim t As Task = Task.Run(
            Sub()
                LoadReports()
            End Sub)
        t.Wait()
    End If
End Sub

Any help or advice will be greatly appreciated.

Nkosi
  • 235,767
  • 35
  • 427
  • 472
Lee Morgan
  • 112
  • 9
  • See if [this](https://stackoverflow.com/a/28445791/1768303) helps. – noseratio Jul 21 '18 at 10:11
  • It might help if you show us the logic around detecting if it is running from the console and hiding the form. – Anders Carstensen Jul 21 '18 at 10:28
  • The described behavior may be due to a deadlock but without a [mcve], I can't say for sure – Nkosi Jul 21 '18 at 11:11
  • Investigate the parse command method and see if you are mixing async and blocking calls – Nkosi Jul 21 '18 at 11:14
  • thanks, you've all given me something to look at, I will report back what I find. – Lee Morgan Jul 21 '18 at 11:30
  • Okay, I have stepped through running the command line, the app closes at this line `await client.SendMailAsync(message);` which leads me to the UI thread closing, I'm new to async programming, how do I hold the UI thread open? – Lee Morgan Jul 21 '18 at 11:48
  • How and where is the `SendAsync` method being called? – Nkosi Jul 21 '18 at 11:52
  • You close right after processing command line, so chances are that you are not awaiting the method and `Close` is being invoked before the email can be sent. – Nkosi Jul 21 '18 at 11:54

1 Answers1

1

You close right after processing command line, so chances are that you are not awaiting the method and Close is being invoked before the email can be sent.

assuming ParseCommandLine and LoadReports are async functions then you should await them in the event handler

Private Async Sub Form1_Load(sender As Object, e As EventArgs) Handles Me.Load
    If CommandLineArgs.Count > 0 Then
        Visible = False
        RunSilent = True
        Await ParseCommandLine() 'wait for task to complete then close
        Close()
    Else
        'Update to add from db, XML or Excel
        Await LoadReports()
    End If
End Sub

Note the removal of .Wait() blocking call that could cause deadlock when mixed with async calls.

Reference Async/Await - Best Practices in Asynchronous Programming

Nkosi
  • 235,767
  • 35
  • 427
  • 472
  • Ok, I will look at all of the methods and make sure they all use async and await, didn't realise it would be this difficult when I started it:( – Lee Morgan Jul 21 '18 at 12:10
  • @LeeMorgan When choosing to use async/await, it tends to spread to the point where you tend to go async all the way. Once you get the hang of it, things tend to get easier. – Nkosi Jul 21 '18 at 12:11
  • Just so I am clear on it all, any method that calls an async method also needs to be async/await? – Lee Morgan Jul 21 '18 at 12:12
  • @LeeMorgan Yes, if you want to wait on said task. If you do not want to wait you can still call it from a non async method, but it will be running on a separate thread, which can lead to the behavior you are seeing with the close happening before it is desired. – Nkosi Jul 21 '18 at 12:15
  • 1
    Mega, not only have you answered my question but you have also helped me understand it some more. I really appreciate your time and explanation. – Lee Morgan Jul 21 '18 at 12:18