search

OrchardCMS / Orchard

Orchard is a free, open source, community-focused Content Management System built on the ASP.NET MVC platform.
https://orchardproject.net
BSD 3-Clause "New" or "Revised" License
2.38k stars 1.12k forks source link

Mailkit upgrade #8746

Open MatteoPiovanelli-Laser opened 10 months ago

MatteoPiovanelli-Laser commented 10 months ago

Since upgrading to mailkit, we've been having a few issues.

Specifically, it looked like the client (i.e. Orchard, where Mailkit is) didn't wait for the smtp server to acknowledge reception of message data. Often, we would get this error on our SMTP servers:

Client socket is disconnected! Disconnect exception encountered: False, IsDisconnected: True, This message will be rejected.
[...]
This issue is caused by the sending server not waiting for the DATA OK command

We tried to upgrade the version of the NuGet package, but there were several issues with its dependencies (https://github.com/jstedfast/MailKit/issues/1462, https://github.com/jstedfast/MimeKit/issues/872, https://github.com/dotnet/sdk/issues/31147) so we were only able to advance it to 3.4.2.

We then changed the code a bit, to explicitly disconnect from the STMP server and dispose of the smtpclient after sending each email. This seems to have improved things. We couldn't replicate the error 100% of the time, but so far in our tests with this code it hasn't occurred yet.

MatteoPiovanelli-Laser commented 10 months ago

The thing the compile check is failing on is not something I changed in this PR, I think?

sebastienros commented 10 months ago

Try to rerun the build, or maybe check if there is a floating version somehow that would fail without any change.

MatteoPiovanelli-Laser commented 10 months ago

if it still doesn't build, I'll "recreate" the PR on step at a time.

BenedekFarkas commented 8 months ago

System.Net.Http is unfortunately a well-known troublemaker, although the relevant threads I found about it all state that it should stop causing problems with .NET 4.8 (or even 4.7.2). I've seen the same/similar errors before (some combination of Upgrade, Orchard.Workflows and Orchard.Email were involved), but managed to find a stable (at least it looked stable at the time) configuration.

It looks like that MimeKit (or one of its dependencies) causes a conflict again, maybe it requires a more recent version of System.Net.Http, but we restrict it to 4.2.0.0.

I'm working on this too, but in the meanwhile, if you isolate any specific change/version that causes the warning (error), please let me know. This might help: https://nickcraver.com/blog/2020/02/11/binding-redirects/#the-fix-if-youre-using-net-framework

BenedekFarkas commented 8 months ago

I managed to get view compilation pass by upgrading Microsoft.CodeDom.Providers.DotNetCompilerPlatform to the latest version (4.1.0) on top of this PR (we're currently using 2.0.1, which is more than 5 years old). I'm not 100% sure why, but version 2.0.1 actually ships with the .NET 4.6-version of System.Net.Http.dll. If I remove that package (and the associated configuration) from Orchard.Workflows, then Razor compilation passes fine.

Also tested the upgrade to 4.1.0 on current 1.10.x and dev. I'll submit a PR targeting 1.10.x for that soon.

BenedekFarkas commented 7 months ago

Please merge the latest 1.10.x to fix the build error.

MatteoPiovanelli-Laser commented 6 months ago

rebased on 1.10.x