lukencode / FluentEmail

All in one email sender for .NET. Supports popular senders (SendGrid, MailGun, etc) and Razor templates.
https://lukelowrey.com/dotnet-email-guide-2021/
MIT License
3.04k stars 438 forks source link

Exception when sending multiple mails using dependency injection #266

Open IanLedzion opened 3 years ago

IanLedzion commented 3 years ago

I'm using Hangfire hosted in an ASP.NET core site as a background job processor. The email service gets a IFluentEmail instance from the DI container. When multiple email jobs are being processed at the same time I get an exception with the following stack trace:

 ---> System.Net.Mail.SmtpException: Failure sending mail.
 ---> System.InvalidOperationException: An asynchronous call is already in progress. It must be completed or canceled before you can call this method.
   at System.Net.Mail.SmtpClient.SendAsync(MailMessage message, Object userToken)
   --- End of inner exception stack trace ---
   at System.Net.Mail.SmtpClient.SendAsync(MailMessage message, Object userToken)
   at FluentEmail.Smtp.SendMailEx.SendMailExImplAsync(SmtpClient client, MailMessage message, CancellationToken token)
   at FluentEmail.Smtp.SmtpSender.SendAsync(IFluentEmail email, Nullable`1 token)
   --- End of inner exception stack trace ---
   at System.Threading.Tasks.Task`1.GetResultCore(Boolean waitCompletionNotification)
   at System.Threading.Tasks.Task`1.get_Result()
   at FluentEmail.Smtp.SmtpSender.Send(IFluentEmail email, Nullable`1 token)
   at FluentEmail.Core.Email.Send(Nullable`1 token)

The problem is that the AddFluentEmail method seems to be registering the service with a scope other than Transient. The result is that the same service instance is being returned each time with the same underlying SmtpClient. The fix would be to change the registration to Transient so that a new instance is created for each request with a separate SmtpClient.

lkroliko commented 3 years ago

Hello

I had the same problem but I changed

services.AddFluentEmail("example@test.local", "example")
                .AddSmtpSender(new SmtpClient("smtp.example.local", 25));

to

services.AddFluentEmail(configuration["Smtp:From"], configuration["Smtp:FromDisplayName"])
                .AddSmtpSender("smtp.example.local", 25);

This resolved this problem for me. I think FluetEmail can initialize new instance of SmtpClient for each call SendAsync method.

Regads

IanLedzion commented 3 years ago

Thanks, that makes sense. As my SMTP configuration is a bit more complex the Func overload would be more appropriate:

services.AddFluentEmail("noreply@example.local")
    .AddSmtpSender(() => new SmtpClient{/* Options */});

I'll give it a try and see what happens.

mekk1t commented 3 years ago

@lkroliko Thanks for the tip! Indeed, it seems that when we pass a concrete smtp client object, FluentEmail reuses the same instance => exception. @IanLedzion have you tried the fix? Man, thanks for the lambda suggestion. I've got the same errors like you, and that suggestion fixed it. If I come up with any errors again, I'll try not to forget to update status here. I'll keep in mind to always look for the lamda-factories and not passing the concrete objects.

Thank you!

lkroliko commented 3 years ago

I think the best option is use IFluentEmailFactory and create FluentEmail for each send.