jstedfast / MailKit

A cross-platform .NET library for IMAP, POP3, and SMTP.
http://www.mimekit.net
MIT License
6.19k stars 821 forks source link

Needs better error description #1788

Closed SoftCircuits closed 2 months ago

SoftCircuits commented 2 months ago

After reading the Microsoft's SmtpClient is deprecated, I'm looking at using MailKit. But I ran into a few issues.

First, I got it working on one computer, but then get an error on another computer. The difference are the email settings that it reads from a file. The following error occurs on the line client.Send(message):

MailKit.Net.Smtp.SmtpCommandException: 'Syntax error in parameters or arguments' This exception was originally thrown at this call stack: MailKit.Net.Smtp.SmtpClient.OnSenderNotAccepted(MimeKit.MimeMessage, MimeKit.MailboxAddress, MailKit.Net.Smtp.SmtpResponse) MailKit.Net.Smtp.SmtpClient.ParseMailFromResponse(MimeKit.MimeMessage, MimeKit.MailboxAddress, MailKit.Net.Smtp.SmtpResponse) MailKit.Net.Smtp.SmtpClient.MailFrom(MimeKit.FormatOptions, MimeKit.MimeMessage, MimeKit.MailboxAddress, MailKit.Net.Smtp.SmtpClient.SmtpExtensions, long, bool, System.Threading.CancellationToken) MailKit.Net.Smtp.SmtpClient.Send(MimeKit.FormatOptions, MimeKit.MimeMessage, MimeKit.MailboxAddress, System.Collections.Generic.IList, System.Threading.CancellationToken, MailKit.ITransferProgress) MailKit.Net.Smtp.SmtpClient.Send(MimeKit.FormatOptions, MimeKit.MimeMessage, System.Threading.CancellationToken, MailKit.ITransferProgress) MailKit.MailTransport.Send(MimeKit.MimeMessage, System.Threading.CancellationToken, MailKit.ITransferProgress) Program.

$.__SendEmail|0_1(object) in Program.cs

This really isn't helpful to me. Everything looks right. Which parameter or argument is wrong?

On a side note, setting the email text is a little confusing. Shouldn't there be a shortcut property where I could just set a string if I don't need to set additional options?

And the example shows "plain" as the argument to the TextPart constructor. Should this be a constant member so that I can easily see what other options are accepted here? I realize there are a rich set of features here. But you can use overloading and other techniques to support simplified syntax as well.

And I think the MailboxAddress constructor should have an email-only overload.

Regarding the error above, I don't know that it's a code issue, but my code is below.

try
{
    settings.LogFile.LogInfo("Sending email...");

    MimeMessage message = new();
    message.From.Add(new MailboxAddress(settings.EmailSettings.FromAddress, settings.EmailSettings.FromName));
    message.Subject = "Test Message";
    message.Body = new TextPart("plain") { Text = settings.EmailSettings.FormatMessage() };

    // Recipients
    foreach (string recipient in settings.EmailSettings.Recipients.Split(';', StringSplitOptions.RemoveEmptyEntries))
        message.To.Add(new MailboxAddress(recipient, recipient));

    // Send email
    using SmtpClient client = new();
    client.Connect(settings.EmailSettings.Host, settings.EmailSettings.Port, settings.EmailSettings.EnableSsl);
    client.Authenticate(settings.EmailSettings.UserName, settings.EmailSettings.Password);
    client.Send(message);
    client.Disconnect(true);

    settings.LogFile.LogInfo("Email Sent.");
}
catch (Exception ex)
{
    settings.LogFile.LogError(ex);
}
jstedfast commented 2 months ago

This really isn't helpful to me. Everything looks right. Which parameter or argument is wrong?

I don't know - this error message comes from the SMTP server, not MailKit.

Based on the stacktrace, it looks like the sender address is syntactically invalid but without a protocol log, it's impossible to say for sure.

Should this be a constant member so that I can easily see what other options are accepted here? I realize there are a rich set of features here. But you can use overloading and other techniques to support simplified syntax as well.

There's an overload that takes an enum as well. You could use that instead of the string. There's essentially an indeterminate number of text types out there (text/x-csharp, for example) so it needs to be possible to set pretty much any string as a value.

And I think the MailboxAddress constructor should have an email-only overload.

I tried that originally, but people ended up misusing it (they expected it to parse out a name and address) so I removed it.

Regarding the error above, I don't know that it's a code issue, but my code is below.

This is most likely to be the issue:

message.From.Add(new MailboxAddress(settings.EmailSettings.FromAddress, settings.EmailSettings.FromName));

Those arguments should be reversed.

SoftCircuits commented 2 months ago

Yes, reversing the arguments to MailboxAddress fixed the error. Thanks.

jstedfast commented 2 months ago

Cool, glad that fixed the issue.