Broken System.Net.Http 4.1.1-4.3.0 post-mortem

[karelz]

Issue #18280 caused a lot of problems for a long time. The overall road towards solution (fix in a new NuGet package 4.3.1) was less than ideal.
Let's track the post-mortem here (as initiated in https://github.com/dotnet/corefx/issues/11100#issuecomment-281827797):

High-level plan to cover:

1. How did the issue slip through into release?
   • How to prevent such situation in future?
2. Why did it take 6 months to fix?
   • Why wasn't it treated/communicated/recognized as high-impact issue earlier?
   • How to recognize and react to high-impact issues earlier in future?
3. Other concerns (e.g. overall communication)

See the writeup in https://github.com/dotnet/corefx/issues/17522#issuecomment-338418610

[karelz]

So it has been a month (again) and I still didn't do it. Sigh. I am sorry! There are just so many important things to do.
I am not trying to dodge it, it will happen. After 2.0 ZBB (May 10 - dotnet/corefx#17619) the latest - driving 2.0 and all other fires around just take quite a lot of my time lately :(

[jahmai-ca]

While I understand the competing priorities, I am concerned that you'll "lose the trail" in figuring out what went wrong. People will move on and forget details. Do you yet know who the key people responsible for were involved in the efforts that created the situation yet or is this still in the pre-investigation stage?

[karelz]

It is not about people (I won't point fingers, it is not productive). It is about decisions and motivations. The first decisions that lead to the unfortunate state happened before June 2016, the last decisions (affecting our reaction time) around October 2016. So waiting couple of more weeks won't really make a difference in tracking it down.

[jahmai-ca]

That wasn't what I was implying. I meant do you have the right people to talk to or do you have to go fishing and risk ending up in a situation where you can't get the answers because you've waited too long.

My concern is "a couple of more weeks" will turn into a "few more months". It took a long time to fix the issue and there appears to be even less motivation to figure out what went wrong.

[karelz]

I have the right people. I talked to them (few months ago). I have to sit down, write it down (in a way to avoid finger pointing, just stating the facts), get it reviewed. Fill in additional gaps in the story I might have missed ... solid day or two of work.

On technical side we already took steps to avoid the problems in future (esp. 2.0):

1. We're talking more about these things.
2. We added Desktop testing into our signoff matrix
3. We reviewed all 'problematic' Desktop-only packages (TestSendAsync_DelayedConsume test failed in CI #16823) and we are assessing what to do about them in 2.0 -- Desktop OOB System.Net.Http - don't ship desktop implementation #20502 and Get rid of System.IO.Compression OOB - .NET Standard 2.0 should forward to Desktop Framework #20074 (note this started ~3 months ago)
   • Note: We will likely backout the changes in Get rid of System.IO.Compression OOB - .NET Standard 2.0 should forward to Desktop Framework #20074, because it brings furher unpleasant side-effects in the end-to-end story (I still have to get educated a bit more about it) -- so we will still run with a small risk we're missing some details in the whole story and it may turn bad, but we at least have immediate fix ready from Get rid of System.IO.Compression OOB - .NET Standard 2.0 should forward to Desktop Framework #20074 (it's current status).
   • This is a result of dozens of discussions with the experts. It is a hard problem (everytime I understand a bit more and I lost hope I will understand all teeny-tiny details any time soon). Plenty of tradeoffs to make, not a black-and-white problem and not even a one-dimensional problem.
4. We also reviewed and did our best for related end-to-end story with BindingRedirects: .Net Plaform Standard implementation on .Net 4.6.1 has version inconsistency for System.Net.Http #17770 and Add dll to netfx references to force bindingRedirects corefx#18300 (I still need to follow up where exactly we landed and which scenarios are now fixed / still broken)

[gulbanana]

1 and 2 are the most important - having recognised that corefx packages are used on netfx and need to be tested on netfx will go a long way to preventing this class of problem. When the answer to "how did this make it to production, did anybody test this" is "no" you're in constant danger!

The approach in dotnet/corefx#18300 is interesting, but i hope it doesn't become a license to require more and more binding redirects. There are still environments where you simply cannot specify them, or at least where no machinery exists to generate them. Consider msbuild tasks, non-exe unit tests, plugins loaded with Assembly.LoadFrom...

[sandersaares]

I hope you get a task force standing by to pounce on such issues with immediate response time if they keep occurring. Right now I just hit Update in NuGet and suddenly I start getting "Type System.X.Y.Z does not match constraint ABC" style exceptions that I suspect are exactly because of some .NET Core cancer I do not even care about ruining my .NET Framework app.

[karelz]

@sandersaares .NET Core "cancer" won't affect .NET Framework apps. Whatever we shipped (and broke) on .NET Framework as NuGet packages was because we wanted to deliver additional value to .NET Framework developers.
Our main approach from now on is "don't ship anything replacing .NET Framework via NuGet, unless we really, really have to". That will take care of that.

[bgever]

It's good to see the lessons learned from this package dependency complexity, and the focus to fix this with .NET Standard 2.0.

However, what about libraries that target versions before .NET 4.6.1 (.NET Standard 2 compatible baseline), and may not be able to switch to the .NET Standard 2 target?

This is what originally caused this issue; where some libraries target the framework version, and other libraries then target newer Out-of-Band (OOB) NuGet variants of the same libraries. The combination of the two in an app requires binding redirects, where the consumer (developer) has to make the trade-off between either the lower or upper version.

Isn't there a limbo for libraries that use any OOB NuGet that isn't targeting .NET Standard 2 yet? And those libraries cause the same binding redirect issues in the apps with framework targets?

So:

• If I create a .NET 4.5 app, I'd have to ensure that all NuGet packages do not use an OOB NuGet dependency (like System.Net.Http). Otherwise I have to deal with binding redirects, which is the root of all agony.
• If I create a 4.6.1 app, I'd have to ensure that all NuGet packages target at least .NET Standard 2. To avoid dealing with binding redirects.
• If I create a .NET Core 2.0 app, I'd have to ensure that all packages I depend on can target at least target .NET Standard 2.

Correct?

[karelz]

the focus to fix this with .NET Standard 2.0

The focus is to fix it in all NuGet packages (not scoped to .NET Standard 2.0).

... some libraries target the framework version, and other libraries then target newer Out-of-Band (OOB) NuGet variants of the same libraries. The combination of the two in an app requires binding redirects, where the consumer (developer) has to make the trade-off between either the lower or upper version.

That is correct and it will never go away, until we either stop shipping those OOB entirely (my original plan in #20502 and #20074 - which we are most likely going to back out), or until we change .NET Framework loader to follow .NET Core loader policy to upgrade version to latest available without any bindingRedirects (which is being considered, but also very tricky to do - the code (Fusion), is extremely challenging and it is easy to break other things with any change to it, or make it function only in some scenarios).

If you create .NET 4.5 or 4.6.1 app, and your dependencies target 2 different versions of the same package, you will have to make a choice - to upgrade to latest or downgrade to lowest. Via bindingRedirects. dotnet/corefx#18300 will help, but as @gulbanana points out, it doesn't solve all scenarios.

We do not plan to ship more (replacement) NuGet packages except the 2 we ship today.

If you create .NET Core 2.0 app, then you don't have to ensure anything - things will just work out for you. Of course unless you start referencing (indirectly) packages from higher (2.1+) .NET Core versions, while trying to force them to run on .NET Core 2.0, then you might need some bindingRedirects (or whatever the alternative in Core is).

[oliverjanik]

Update: Ignore this comment (dumb oversight)

Were the issues resolved? I'm still experiencing very weird issues with HttpClient, like doing PostAsync and it does a "GET" call...

I'm targeting net core 1.1 the library that wraps HttpClient is NET standard 1.3

[karelz]

Yes, all known problems were resolved. If you see any new issues, please file a new bug with description what happens when. Thank you!

[jahmai-ca]

@karelz Guess what I would like to know ;-) Hint: It's been 4 weeks since last update.

[karelz]

@jahmai know what?
Re: Hint: Yep, understood. Should happen before 2.0 ships. After the Ask mode & bug driving madness (it takes a LOT of my time).