Optimizely Forms: 4.7.0 Too many concurrent connections

A client had a (Optimizely Form) form that sends three different emails after submission. Very often, one of the emails never arrived. The form submission itself worked fine, and the only trace was this in the log.

System.AggregateException: One or more errors occurred. (4.7.0 Too many concurrent connections.)
 ---> MailKit.Net.Smtp.SmtpCommandException: 4.7.0 Too many concurrent connections.
   at MailKit.Net.Smtp.SmtpClient.ConnectAsync(...)
   at EPiServer.Notification.Internal.SmtpClientImplementation.SendAsync(MimeMessage message)
   at EPiServer.Forms.Implementation.Actors.SendEmailAfterSubmissionActor.<>c__DisplayClass13_0.<SendMessage>b__5()

The mail server allows a limited number of concurrent connections per client. So why does one form submission need more than one connection at a time?

What happened?

I decompiled EPiServer.Forms.UI 5.10.6 to find out. SendEmailAfterSubmissionActor loops through the email configurations and calls SendMessage() for each one. SendMessage() ends like this.

lock (_lock)
{
    Task.Run(delegate
    {
        try
        {
            _smtpClient.SendAsync(message).Wait();
        }
        catch (Exception ex)
        {
            _logger.Error("Failed to send email", ex);
        }
    });
}

The lock looks like an attempt to send one email at a time. But it only covers starting the task, not sending the actual email. Task.Run returns immediately, the lock is released, and the next email is started. All the emails are sent in parallel. It's all fire-and-forget.

Then there's SmtpClientImplementation, the default ISmtpClient in CMS 12. For every message it creates a new MailKit SmtpClient, then connects, authenticates, sends and disconnects. No connection is ever reused.

Three email configurations means three SMTP connections, opened at the same moment.

Why didn't anyone notice?

The exception is caught and logged inside the background task. The visitor gets a nice "thank you" message, and the email is gone.

A 4xx reply from an SMTP server means "try again later". That's a job for the sender, and in this case the sender is Optimizely Forms.

The fix

I've reported this to Optimizely. Until it's fixed, you can wrap the ISmtpClient so that only one message is sent at a time, and retry on transient errors.

Something like this.

public class SerializedSmtpClient : ISmtpClient
{
    private const int MaxAttempts = 3;

    private static readonly SemaphoreSlim SendLock = new(1, 1);
    private static readonly ILogger Logger = LogManager.GetLogger(typeof(SerializedSmtpClient));

    private readonly ISmtpClient _inner;

    public SerializedSmtpClient(ISmtpClient inner)
    {
        _inner = inner;
    }

    public async Task SendAsync(MimeMessage message)
    {
        await SendLock.WaitAsync();
        try
        {
            for (var attempt = 1; ; attempt++)
            {
                try
                {
                    await _inner.SendAsync(message);
                    return;
                }
                catch (SmtpCommandException ex) when ((int)ex.StatusCode is >= 400 and < 500 && attempt < MaxAttempts)
                {
                    var delay = TimeSpan.FromSeconds(2 * attempt);
                    Logger.Warning($"SMTP server replied {(int)ex.StatusCode}. Retrying in {delay.TotalSeconds} s.");
                    await Task.Delay(delay);
                }
            }
        }
        finally
        {
            SendLock.Release();
        }
    }
}

The semaphore is static. The client is registered as transient, so each actor gets its own instance, and the lock has to be shared across all of them.

The default ISmtpClient isn't registered in AddCms() but in the initialization module QueryableNotificationUsersInitialization. So I replace it in a configurable module that depends on that one.

[InitializableModule]
[ModuleDependency(typeof(QueryableNotificationUsersInitialization))]
public class SerializedSmtpClientInitialization : IConfigurableModule
{
    public void ConfigureContainer(ServiceConfigurationContext context)
    {
        var services = context.Services;
        var original = services.LastOrDefault(d => d.ServiceType == typeof(ISmtpClient));
        if (original == null)
        {
            return;
        }

        services.Remove(original);
        services.Add(new ServiceDescriptor(
            typeof(ISmtpClient),
            provider => new SerializedSmtpClient(
                (ISmtpClient)ActivatorUtilities.CreateInstance(provider, original.ImplementationType!)),
            original.Lifetime));
    }

    public void Initialize(InitializationEngine context) { }

    public void Uninitialize(InitializationEngine context) { }
}

The emails are now sent one after another, over one connection at a time. Your existing SMTP settings still work, because the original implementation does the actual sending.

To verify, I created a test form that sends three emails. Without the wrapper, I received two of them, every time. With the wrapper, I received all three, every time.

A final word of advice

ISmtpClient lives in EPiServer.Notification.Internal, and so does QueryableNotificationUsersInitialization. This is unsupported API that may change without notice. Remove the workaround as soon as Forms is fixed.

And if emails from your forms sometimes go missing, search your logs for Failed to send email. Forms won't tell you any other way.