Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

System.Net.Mail.SmtpClient is marked as obsolete in docs, but not in the source code #2986

Closed
PashaPash opened this issue Apr 11, 2017 · 57 comments
Labels
area-Infrastructure external Not related to content Pri1 Indicates issues/PRs that are high priority
Milestone

Comments

@PashaPash
Copy link

System.Net.Mail.SmtpClient class page is currently states that class is obsolete, and offers https://github.com/jstedfast/MailKit as a replacement.

[System.Obsolete("SmtpClient and its network of types are poorly designed, we strongly recommend you use https://github.com/jstedfast/MailKit and https://github.com/jstedfast/MimeKit instead")]
public class SmtpClient : IDisposable

That class has no Obsolete attribute in both Reference Source for 4.7 and corefx source.

Mono version of SmtpClient was marked as Obsolete few years ago by @migueldeicaza, and then that external lib link somehow made its way to official 4.7 docs. Please remove it.

@jstedfast
Copy link
Member

Sssshhhhhh!!!! This is epic for MimeKit and MailKit 😃

@rpetrusha
Copy link

Thanks, @PashaPash, for pointing this out and providing context on how it might have happened.
Obsolete information (types and members tagged with the Obsolete attribute) is generated by reflection, so we can't simply remove it; this points to some false assumptions in the reflection process. I've opened an internal bug, #968551, to track the issue.

@dend
Copy link
Contributor

dend commented Apr 27, 2017

Thanks @mairaw @rpetrusha @PashaPash - we are investigating. This is indeed a byproduct of how we auto-generate documentation.

Opened a mdoc issue:
mono/api-doc-tools#76

@svick
Copy link
Contributor

svick commented Jun 26, 2017

I think there's another instance of this on Socket.​Duplicate​And​Close. All versions of the docs for that have the MonoLimitation attribute:

[System.MonoLimitation("We do not support passing sockets across processes, we merely allow this API to pass the socket across AppDomains")]

@ocdtrekkie
Copy link

This is actually slightly more baffling if you come from the VB display, where the [System.Obsolete( bit doesn't display. The docs just appear to say "it's obsolete" and fails to offer up an alternative. I came here to seek an answer. So System.Net.Mail is not obsolete?

@mairaw
Copy link
Contributor

mairaw commented Jun 25, 2018

Yes @ocdtrekkie, I was just discussing this with @dend last week. Here's an issue related to that request: mono/api-doc-tools#274

@joelmartinez
Copy link
Contributor

Yes, this change is very close to landing ... will be in the next release of mdoc :)

@ocdtrekkie
Copy link

Great to hear. :)

@Allon-Guralnek
Copy link

Has the obsolete notice been removed from the docs? I can't find it on the linked page.

@mairaw
Copy link
Contributor

mairaw commented Jul 17, 2018

Temporarily removed @Allon-Guralnek. Next time we run CI in our docs this will be added back.
We're still waiting for the fix that can distinguish attributes per .NET implementation.

@joelmartinez
Copy link
Contributor

Just FYI, the next time it's added back, it will have the appropriate attributes (FrameworkAlternate) to indicate that the attribute only exists in a subset of frameworks (the feature became available two releases of mdoc ago) ... however, not sure when the additional upstream work will be completed, so that the front-end properly filters this attribute out for frameworks like .net desktop, and .net core :)

@af4jm
Copy link

af4jm commented Aug 28, 2018

I'm seeing this in .NET Core 2.1.2 today... warning DE0005: SmtpClient is deprecated... does that mean there's also a bug in <PackageReference Include="Microsoft.DotNet.Analyzers.Compatibility" Version="0.2.12-alpha" PrivateAssets="All" />?

@karelz
Copy link
Member

karelz commented Oct 17, 2018

BTW: SmtpClient is obsolete API: https://github.com/dotnet/platform-compat/blob/master/docs/DE0005.md (that is what causes the warning DE0005.
We do not plan to add the Obsolete attribute on it to avoid breaking everyone's compilation (with warnings treated as errors).

@ocdtrekkie
Copy link

@karelz I am very sad if that's the official position of Microsoft, to dump basic Internet functionality off to random third parties. Given examples from other platforms like left-pad, I am extremely hesitant to add random dependencies to software I develop because I have no reason to trust the developer.

I do see in the case of MailKit that it appears to be being maintained by a Microsoft employee, but as a personal project, which means there's really no guarantee of quality or support.

@jstedfast
Copy link
Member

Hi @ocdtrekkie,

I'm the MimeKit and MailKit maintainer.

FWIW, I think you'll find that both projects are very high quality codebases and I am very active with maintenance of both projects. This includes fixing bugs, answering questions, adding new features, etc.

Hope that helps assuage your fears... at least somewhat.

@svick
Copy link
Contributor

svick commented Oct 18, 2018

@ocdtrekkie MailKit is also part of the .Net Foundation, in case that makes a difference for you.

@PashaPash
Copy link
Author

@karelz @jstedfast "doesn't scale to modern requirements of the protocol" is too generic.

Is there any specific reason to use MailKit instead of SmtpClient to send mail over SMTP? Will it be faster or more reliable?
It seems that SmtpClient does not support pipelining and rfc2231 for filenames, but I'm not sure about other protocol features.

@karelz
Copy link
Member

karelz commented Oct 18, 2018

@PashaPash we do not have list of all missing RFCs and features in SmtpClient. The fact is that SmtpClient hasn't been innovated in a long time, while MailKit was.
We've also heard that the performance of SmtpClient is not as good as MailKit.

If you are sending just occasional emails (e.g. alerts, or tooling), then SmtpClient is likely good enough for you. If you need modern, high-perf, latest technology, then I would recommend to use MailKit.

@ocdtrekkie
Copy link

ocdtrekkie commented Oct 18, 2018

@jstedfast My comment wasn't intended to be a slight at your projects, I do hope you didn't take it as one.

But I think there's definitely a strong value to features in .NET Framework code being officially managed and distributed by Microsoft, and of course, that projects (and other dependencies) declaring compatibility with a .NET Framework version are stating compatibility with all of the features included. (Watching Node-based projects talk about what they have to do to upgrade what dependencies is incredible.)

Documentation is, of course, tightly integrated with the the documentation for .NET as a whole as well, etc. And as my project currently is .NET Framework-based, of course, another huge benefit is not carrying much extra code with my program, as it comes with Windows.

In my personal code projects, I've set the general intention to solely use .NET Framework/Core as my sole "dependency" and eschew third party functionality as much as possible. In a few cases, this means I've reinvented the wheel (which has been educational!), but I've heavily relied on things like SmtpClient being taken care of for me. For an example, my project had to implement IMAP since .NET Framework doesn't. MailKit is larger than my entire project, and the rudimentary IMAP functionality I needed is only about three dozen lines of code. 10xing the size of my project and the scope of code I drag along with it for one function would be unfortunate.

I get a little bit of pride every time I can remove a NuGet package, so it's just a little disappointing that Microsoft is moving in the opposite direction. Just something I'm going to have to deal with as everything becomes irritatingly more like the JavaScript world.

(I'm gonna step off my soapbox now...)

@jstedfast
Copy link
Member

@ocdtrekkie Don't worry, I did not take any offense at your comment :-)

FWIW, I also like to use as few nugets as possible in my projects as well, so I can understand your position on that.

@sonic1981
Copy link

It's been a year and no one seems to of fixed this?! o_O

@karelz
Copy link
Member

karelz commented Mar 14, 2019

@sonic1981 what kind of fix do you expect?
We do not plan to update source code. Is there anything else that should happen?

@ghost
Copy link

ghost commented Mar 14, 2019

@karelz Why would you not update the source code here? Having it marked obsolete in one official reference but not another is very confusing.

@karelz
Copy link
Member

karelz commented Mar 14, 2019

@joelmartinez
Copy link
Contributor

joelmartinez commented Mar 14, 2019

Please note that the docs metadata now correctly reflects the fact that this attribute is only present on the version(s) of the BCL that ship with Xamarin/Mono:

https://github.com/dotnet/dotnet-api-docs/blob/master/xml/System.Net.Mail/SmtpClient.xml#L32-L34

The docs site is currently going through some behind-the-scenes updates, at which point it will display correctly on the website (however I don't have a specific ETA to share) ... so when you've got it filtered to .net framework 4.7 (or another framework/version that doesn't have it), it won't show this attribute.

@ghost
Copy link

ghost commented Mar 14, 2019

So to summarize:

  1. SmtpClient is considered obsolete in Xamarin/Mono. It has the Obsolete attribute in the source code and is documented as obsolete on Microsoft Docs.
  2. SmtpClient is not considered obsolete in .NET Standard. It does not have the Obsolete attribute in BCL or CoreFX source code, and is not documented as obsolete on Microsoft Docs.
  3. SmtpClient is considered obsolete by the Microsoft.DotNet.Analyzers.Compatibility analyzer, which is in preview and not guaranteed to be installed in all projects.

If all the above is true, then I still feel justified in calling this very confusing. @karelz Breaking builds is the risk you take when an API is marked obsolete. If you're willing to take that risk in Xamarin/Mono, why not in .NET Standard (especially for .NET Core where the .NET ecosystem is getting a nice fresh start)?

@karelz
Copy link
Member

karelz commented Mar 14, 2019

Xamarin/Mono has different compat bar than we have in .NET Framework and .NET Core.
I think we are arguing over details here. I agree that things are not perfect, but the bottom line is that SmtpClient is useful API only to certain extent. We do not plan to evolve it, which makes it legacy API (kind of obsolete).

If the Obsolete information is gone from documentation soon (per above comment), we should mention in the docs that the API is not modern and not high-perf and that we recommend use of other projects like MailKit for those purposes
Will that address your concerns?

@ghost
Copy link

ghost commented Mar 14, 2019

I think I see what you're saying. SmtpClient is not truly obsolete because it still has use cases. I.e., you are not trying to migrate all users to use MailKit for every emailing scenario. In that case, then yes I think a blurb in the docs that MailKit is better for modern/high-perf scenarios is sufficient.

However, speaking for myself, I don't want to learn two different mailing APIs; I would rather just learn the modern one that I know will work at scale. Marking SmtpClient as obsolete would give us an Intellisense warning about it. If the .NET devs truly feel that MailKit is a superior API (as they seem to given the Xamarin changes), then they should be trying to migrate all users away from SmtpClient. Just obsolete the old thing and add the new thing to the framework (as was done with Json.Net). Don't take half-measures. 😛

@TravisEz13
Copy link

I saw no firm statement of if this is obsolete in DotNet Core.

For example, are there plans add support to SmtpClient in DotNet Core to Opportunistic TLS and in general keep it up to date?

@karelz
Copy link
Member

karelz commented Mar 20, 2019

@TravisEz13 no, we do not expect any modernization of the API or its implementation. See also link above: https://github.com/dotnet/docs/issues/1876#issuecomment-430805681 pointing to https://github.com/dotnet/platform-compat/blob/master/docs/DE0005.md

@soniczoo
Copy link

It would really be nice to know exactly what's wrong with system.net.mail.smtpclient but I cannot get/find a straight answer. This lack of info and professional courtesy looks bad for Microsoft and the developers of Mailkit (especially if they are affiliated).
Mailkit seems awesome in every way, and it works in the ONLY environment (of 100s) SMTPClient has had a problem in - thank god it's our own QA environment. But getting railroaded into 100s of hours of change-over without knowing why is unfair. And I hope policies change, but many people working in large companies don't even have the option to use open source - so we MUST know what is wrong with SMTPClient. Without that common courtesy, t-shooting is impossible, and a simple fixable config issue may result in all kinds of hurt.

@PashaPash
Copy link
Author

MailKit is part of the .NET Foundation, so if the maintainer decided to abandon it, somebody else should take their place.

Not sure how dotnetfoundation.org membership implies change of project ownership. Dotnetfoundation is about technical services and marketing, not about code contribution.

There is a hope that a second contributor will pick up the project, but for MailKit jstedfast is the only major contributor. 2-nd contributor is Redth, the owner of now abandoned PushSharp. And he has only 9 commits. 3-rd is migueldeicaza, mostly documentation, 6 years ago. It is clear that project is owned and maintained by one person, and no chance someone else can quickly became a full grade maintainer.

@mimisasouvanh
Copy link

@mairaw We will be rolling out the attribute versioning feature later this week.

@GF-Huang
Copy link

So, what should I use now?

@migueldeicaza
Copy link
Contributor

@migueldeicaza what exactly is wrong with SmtpClient in terms of encoding support or protocols support? It does support TLS, it does support UTF-8, that covers 99% of common cases for most of the apps.

Please read the landing page for MailKit to get an idea of how much is missing.

@soniczoo
Copy link

The landing page that says smtpclient doesn’t, and mailkit does these things.. “cross-platform support and ability to save and re-load MIME messages before sending them”? That’s all I see.. not an explanation of issues with smtpclient. The whole point here is that Microsoft needs to support their “supported” software.. and I trust they will, they always have. Mailkit looks/works great. But business doesn’t care about that. It invests in software and platforms to make money. And business expects to have ample sunset warning, so it can plan its next investment. This healthy lifecycle is not happening in the case of System.Net.SmtpClient. Its like someone on a whim just yanked the plug on it.

@mairaw
Copy link
Contributor

mairaw commented Mar 30, 2020

@mimisasouvanh @TianqiZhang the new feature does not display the wrong attribute anymore, but it continues to display the obsolete warning for all versions. That information also needs to be version/target aware.

@migueldeicaza
Copy link
Contributor

Bad code and bad API design exists in the wild. When the world understands those problems, they move to new systems that prevent that.

Just like we moved away from SSL to TLS, and then to version 1.1 and 1.2 and now we are hoping to move to 1.3, there are mistakes in the past that can not be solved. This is one of those.

You should avoid using SmtpClient in my humble opinion, and do your users a favor in the process.

Or you could live dangerously and roll the dice.

@alex-jitbit
Copy link
Contributor

@migueldeicaza you made a claim but then instead of proving it, you send everyone to read some page online.

The burden of proof is on the person who makes a claim

SmtpClient is still a robust working piece of code (tested on our SaaS app used by 1000s of companies).

MimeKit is indeed a fine, fast MIME parsing library for inbound email, that has SMTP as an addon.

@mwwhited
Copy link

mwwhited commented Jul 7, 2020

This is probably going to end up like json.net. An API that everyone love snit Microsoft things they can do better so they create the bastard that is text.json. Horrible library and attempt to blind port people leading to breaking changes everywhere.

@carlossanlop
Copy link
Member

@mimisasouvanh what's the status of this?

@TianqiZhang
Copy link
Contributor

@mimisasouvanh @mairaw now the page (https://docs.microsoft.com/en-us/dotnet/api/System.Net.Mail.SmtpClient?view=net-5.0) displays obsolete messages correctly, the warning will only show for 3 versions ( xamarinandroid-7.1 xamarinios-10.8 xamarinmac-3.0 ).

@mairaw
Copy link
Contributor

mairaw commented Sep 12, 2020

Thanks @TianqiZhang. Closing this.

@mairaw mairaw closed this as completed Sep 12, 2020
@mairaw mairaw removed the blocked Indicates issues or PRs that are blocked for some reason label Sep 12, 2020
@mdell-seradex
Copy link
Contributor

@mimisasouvanh @mairaw now the page (https://docs.microsoft.com/en-us/dotnet/api/System.Net.Mail.SmtpClient?view=net-5.0) displays obsolete messages correctly, the warning will only show for 3 versions ( xamarinandroid-7.1 xamarinios-10.8 xamarinmac-3.0 ).

This is not entirely true because it still has a generic statement at the top that The SmtpClient type is now obsolete.

@mairaw
Copy link
Contributor

mairaw commented Nov 5, 2021

I'll let @BillWagner and @gewarren analyze this. The issue here was that the warning was being displayed all the time. The generic statement at the top was added as part of the summary, so it will always show.
However, even though SmtpClient isn't marked as obsolete in newer platforms, it's not recommended by the team: https://github.com/dotnet/platform-compat/blob/master/docs/DE0005.md

@mdell-seradex
Copy link
Contributor

mdell-seradex commented Nov 5, 2021

@mairaw, there is actually another issue (#4114) where it has been suggested to modify this note.
The suggested change there is to this: The SmtpClient type is now obsolete on some platforms and not recommended on others - see Remarks for details.

I thought it was odd that MailKit, instead of Microsoft.Graph was being recommended as an alternative.
While I do not believe that Microsoft.Graph is specifically an SMTP client per se, however, based on my understanding, it does allow developers to read and create and send email, which I feel generally satisfies the SMTPClient requirement. I think MailKit may actually use Microsoft.Graph under the hood for some things, however I can't be certain as I have read very little about it.

@PashaPash
Copy link
Author

@mairaw could you please update docs with some technical justification of using external library (MailKit) over standard SmtpClient?

All that we have now is:

Statement added as obsolete attribute for Xamarin by one of MimeKit contributors. Kind of "use my favorite lib, it's cool!":

SmtpClient and its network of types are poorly designed, we strongly recommend you use https://github.com/jstedfast/MailKit and https://github.com/jstedfast/MimeKit instead

DE0005 note, with no technical details:

SmtpClient doesn't support many modern protocols. It is compat-only. It's great for one off emails from tools, but doesn't scale to modern requirements of the protocol.

A real list of unsupported modern SMTP protocols and unsupported modern requirements of SMTP protocol would be great.

@mdell-seradex
Copy link
Contributor

@mairaw could you please update docs with some technical justification of using external library (MailKit) over standard SmtpClient?

All that we have now is:

Statement added as obsolete attribute for Xamarin by one of MimeKit contributors. Kind of "use my favorite lib, it's cool!":

SmtpClient and its network of types are poorly designed, we strongly recommend you use https://github.com/jstedfast/MailKit and https://github.com/jstedfast/MimeKit instead

DE0005 note, with no technical details:

SmtpClient doesn't support many modern protocols. It is compat-only. It's great for one off emails from tools, but doesn't scale to modern requirements of the protocol.

A real list of unsupported modern SMTP protocols and unsupported modern requirements of SMTP protocol would be great.

@PashaPash, I think your point may be better suited over here: #7355
That issue is recent and still open. I know that I logged it as a question, but that doesn't mean it could not bring up valid points that cause the documentation to be changed.

@karelz
Copy link
Member

karelz commented Nov 5, 2021

@BillWagner @gewarren @mairaw no need to do any further analysis. As @mdell-seradex mentions in #2986 (comment) - issue #4114 tracks resolution of the concern voiced by them in #2986 (comment) (after the issue was closed)

@PashaPash let's track your concern from #2986 (comment) elsewhere. Not sure if #7355 is the best option - perhaps opening new issue may be better. However, be warned that you might not like the answer -- basically all information we have is already listed in the "generic" statements. We do not have list of missing protocols, etc. The SmtpClient component haven't been touched with new protocol updates in a very, very long time and we know from Mono/Xamarin that it was not very usable already years ago. If someone has more info than that, we can figure out where to keep the details (e.g. some GH issue). Not sure if it belongs into docs.

BTW: IMO Xamarin didn't obsolete the type with "their" favorite cool library, but rather MailKit/MimeKit libraries were created due to shortcomings of SmtpClient to deliver functionality their customers needed ... that is my best guess. It is hard to blame them IMO.

I would recommend to lock this issue in couple of days to prevent accidental piling on discussion here instead of creating new issues where we can have scoped discussions.

@dotnet dotnet locked as resolved and limited conversation to collaborators Nov 5, 2021
@gewarren
Copy link
Contributor

gewarren commented Nov 5, 2021

#7359 clarifies the obsoletion statement in the summary text.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Infrastructure external Not related to content Pri1 Indicates issues/PRs that are high priority
Projects
None yet
Development

No branches or pull requests