PR failed to load

[drguthals]

Issue from Twitter:
https://twitter.com/bchavez/status/974374196724736000

Following up with more details. This happens when a PR title is clicked in our GitHub pane.

This appears to be the same as #1532.

How to fix

The important steps for me were:

• Ensure KB3140245 is installed.
• Ensure the EasyFix is applied.
• Also set DefaultSecureProtocols to 0x00000800:

HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\Internet Settings\WinHttp
HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\Microsoft\Windows\CurrentVersion\Internet Settings\WinHttp

Confirmed by @bchavez #1539 (comment)

[bchavez]

Hi, thank you for opening the PR @sguthals.

My Visual Studio version is:

The log file for the GitHub extension per @jcansdale 's request:

extension.log

I started a new log with the current error because the original log was very large and at first glance looked like it contained sensitive information, I still have the original in case your interested.

I did a quick search on the error yesterday and ran across a few hints it might have been related to LibGit2Sharp and the newly imposed SSL/TLS changes on GitHub. Some GoogleFu findings pointed me to an "EasyFix" Microsoft Support site that essentially changed the default ordering of "SSL/TLS" when underlying requests were made with WinHTTP.

Something along these lines:
JuliaLang/julia#26167
https://support.microsoft.com/en-us/help/3140245/update-to-enable-tls-1-1-and-tls-1-2-as-a-default-secure-protocols-in

After trying and installing the EasyFix, I pretty much gave up yesterday.

Some possible issues that may be contributing to this problem:

• I'm running Windows 7 with stock IE 8 installed. I have not installed an updated IE version because:

  1. I don't use IE 8.

  2. I suffer from a visual impairment known as Myopia.
     Basically, any distance beyond my monitor objects begin to look blurry. You can imagine, when ClearType font smoothing is enabled, text with soft & smooth edges on my monitors look blurry. This font smoothing effect drives me nuts, hurts my eyes, and makes me feel like I'm going blind. I need to see jagged edges of text to feel comfortable reading & writing on my computer. So, keeping ClearType and font-smoothing off my computer is a big deal to me. I even fought Google's Chrome Team (and won) on this very subject after they made Google Chrome V31 disrespect ClearType settings.

     Unfortunately, the ActiveX/WebView control that ships with IE9+ and onwards does not respect the operating system ClearType settings on Windows 7. There is no way to disable font-smoothing on IE9, 10, 11 and Edge. The problem is worse in Windows 8 and Windows 10, where WPF and Metro apps have taken font-smoothing to a new level using DirectX DirectDraw. There's just no way to disable any of it short of patching kernel-level API calls to DirectDraw and GDI+.

In summary, I won't be able to update Internet Explorer if IE turns out to be the underlying problem with the VS extension.

It would be nice if your GitHub VS extension worked on Windows 7, but it's not a huge deal for me because I prefer TorutiseGit for everything.

Thanks,
Brian

🚶‍♂️ 🚶‍♀️ Missing Persons - Walking in L.A.

[jcansdale]

@bchavez,

Thanks for the info. Looking at the versions of VS and the extension you're using, they should be ready for the SSL/TLS changes on GitHub.

Here is someone who was seeing a similar exception:
https://mengyiyuanblog.wordpress.com/2016/11/29/configure-bower-with-company-proxy/

Are you working behind a company proxy? If you are, could you try again using a different network?

[bchavez]

Hi @jcansdale,

Nah, I'm not behind any company proxy since I work from home. I do have a firewall on my main dev box that is pretty locked down though. I'll try on a clean Win7 VM install without any firewall rules and try again. Also, it will give me a chance to test the IE8 vs IE9 theory too.

Thanks,
Brian

⚡ 🏃 "I was struck by lighting. Walkin' down the street..."

[bchavez]

Also, FWIW, don't know if this helps, but Fiddler shows communication with github.com looks okay...

[bchavez]

More testing on Win7 VM:

GitHub extension v2.2.0.10 works (the one installed with most recent Visual Studio Installer)

---

Then update GitHub extension to v2.4.3.1737:

And failure. No dice with the latest version of the GitHub extension. Most definitely, the most recent GitHub extension update broke something.

👑 💎 "I know everything that shine ain't always gold..."

[bchavez]

More testing on the Win7 VM:

• Upgraded stock IE8 to IE10 and IE11 and the problem persists with latest v2.4.3.1737 extension installed. My theory is out of the running.
• Updated Windows 7 to latest patch levels with:
  
• Just a side note: Also seems there's a performance regression with v2.4.3.1737. Visual Studio seems to be putting the GitHub extension on blast. This is the second time I received the performance degraded message with v2.4.3.1737 installed:
  
  I didn't receive these warnings with the stock install v2.2.0.10 of the GitHub extension shipped with the VS 2017 installer. Ten seconds is kind of a big deal.

Hope that helps,
Brian

🍫 🍪 🍭 Ronald Jenkees - Stay Crunchy

[grokys]

@bchavez does this happen with only a single PR, a single repository, or all PRs on all repositories? For example, if you clone this repository (github/VisualStudio) does it fail opening our PRs?

[bchavez]

Hi @grokys , how would you like me to clone the github/VisualStudio repo?

Apparently, the Open in Visual Studio button is broken too.

Wow. I feel really bad for new windows developers who try to figure out this open source GitHub stuff out. FWIW, this testing is happening on a new fresh Windows 7 VM with latest updates.

⌚ 🌆 "I just can't wait... I just can't wait... for saturday night..."

[grokys]

Hmm, just tried opening the PRs that you were having trouble opening and not seeing the problem here:

But I'm on Win10 so I wonder if this is a Win7 issue? It's Saturday so I can't look into it properly today, but we'll look into it on Monday. @meaghanlewis do we test on win7?

We should open separate issues for the Open in Visual Studio and performance regression (which again, I'm not seeing here).

[bchavez]

Correct. Just tested in a Win10 VM and the PRs load fine.

However, I still get the performance regression in the Win10 VM:

Maybe it's because these machines are running inside a VM.

Also, found it strange with some Windows 10 themes the GitHub extension for highlighting selection items just doesn't seem right. The highlight color and foreground text font blend in together so much you can't even read the highlighted item.

To summarize, PRs loading correctly:

Version	Windows 7	Windows 10
v2.2.0.10	✔️	✔️
v2.4.3.1737	❌	✔️

[meaghanlewis]

Hey @grokys, no I only test in Windows 10, but sounds like it would be valuable to also test in Windows 7. I'll set up a VM with Windows 7 this week.

I also haven't been able to reproduce these issues (viewing pull requests, opening a repo in Visual Studio, or the unreadable highlighted items) on Windows 10.

@bchavez could you open up separate issues for the open in Visual Studio and highlight problem. Also, could you specify the Windows 10 themes that show the highlight problem?

[grokys]

I suspect this may be caused by TLS1 being turned off on github.com:

libgit2/libgit2sharp#1524 (comment)

The difference between [Windows] 7 and 10 is what the default value for that key is if it isn't set. The patch for 7 added the key, but didn't actually make TLS 1.1 and 1.2 enabled by default, which is why you still have to add the key to enable them.

However, Windows 10 does enable them by default, so you don't need the key to override the defaults.

[jcansdale]

@grokys, @bchavez mentioned that he tried the EasyFix as also mentioned in the issue you linked:
libgit2/libgit2sharp#1524 (comment)

I wonder why that didn't fix it? 😕

@bchavez we'll do some testing on Windows 7 and get back to you. Thanks for your quality issue report and research!

[bchavez]

The peculiar thing I find interesting about this bug on Windows 7 is that:

• This bug does not exist in v2.2.0.10 (shipped with VS installer). A PR's description loads fine in v2.2.0.10 (see above). However, if you update the GitHub extension to v2.4.3.1737 PRs fail to load.

  Nothing about the underlying OS network stack changes between v2.2.0.10 and v2.4.3.1737.

• Additionally, when the troubled v2.4.3.1737 extension is installed, the Fiddler output (see above) does show successful communication with GitHub's API, I can see actual data cross the wire. Seems like, to me, if data payloads are being successfully transmitted from GitHub to the VM the TLS connection should already be setup and ready to go, perhaps negating "it's a TLS/OS network stack bug".

• Another interesting observation, when the v2.4.3.1737 extension is installed, if you press the "Refresh" you can get a glimpse & flash of the PR details that was selected, but shortly after, the "The Pull Request failed" shows up. Again, kinda jives with the prior point, that data is being successfully transferred between GitHub and the VM.

Thanks for the info. Looking at the versions of VS and the extension you're using, they should be ready for the SSL/TLS changes on GitHub.

Maybe, maybe, are you doing any extra HTTPS or LibGit requests AFTER the initial PR data is downloaded that isn't setup for SSL/TLS?

Almost seems like the v2.4.3.1737 GH extension is doing something a little extra under the hood after the initial PR data is downloaded. Then an exception is thrown, and the exception bubbles up the stack that makes "Failed to load Pull Request" display. The DIFF between v2.2.0.10 and v2.4.3.1737 just after the PR data is downloaded would be an interesting read.

Just some food for thought.

💥 💫 "Crashing, hit a wall. Right now I need a miracle..."

[bchavez]

MmmHmm... that suspicious SSLv3 Handshake Failure:

Next question is, who is 192.30.255.113 on your network?

⏳ 🔍 "But I still haven't found what I'm for..."

[grokys]

To give a bit of background to this, between v2.2.0.10 and v2.4.3.1737 two things changed:

1. We upgraded our version of https://github.com/libgit2/libgit2sharp
2. We do a git fetch after loading the PR details from the API (this is needed because to get line numbers of PR review comments, we need to do diffs, and to do diffs we need to make sure that the PR HEAD and BASE are fetched)

So what is happening, is that the initial load of the PR is working, but then the git fetch fails and the error message is shown.

It would appear to be failing due to TLS1 being turned off and something in either Windows 7 or libgit2sharp (or more precisely a combination of the two) isn't working with later TLS version. Exactly why that is, I'm still not sure.

[jcansdale]

@bchavez I initially thought the following was similar to what you'd tried, but looking at the registry entries I think it might be different.

If you are using Windows 7 or Windows Server 2008 R2, the TLS 1.2 protocol will need to be enabled at the operating system level for .NET Framework (and therefore Visual Studio and Git Credential Manager) to be able to make use of it. These settings are described here and below and in the TLS 1.2 Settings documentation (https://docs.microsoft.com/en-us/windows-server/security/tls/tls-registry-settings#tls-12

reg add "HKLM\SYSTEM\CurrentControlSet\Control\SecurityProviders\SCHANNEL\Protocols\TLS 1.2\Client" /v DisabledByDefault /t REG_DWORD /d 0 /f
reg add "HKLM\SYSTEM\CurrentControlSet\Control\SecurityProviders\SCHANNEL\Protocols\TLS 1.2\Client" /v Enabled /t REG_DWORD /d 1 /f

Source: https://developercommunity.visualstudio.com/content/problem/201457/unable-to-connect-to-github-due-to-tls-12-only-cha.html

Could you give that a try and let me know if it improves things? 🤞

[bchavez]

@jcansdale Ran the commands as administrator:

No dice, the problem persists. My bet is on that libgit2sharp dependency update. 😕

🏖️ 🎺 Beach Boys - Good Vibrations (Nick Warren bootleg)

[jcansdale]

@bchavez It looks like @nk111 and @ethomson might be ahead of us. 😉

You'd need to add a certificate validation callback to your code. But if you're using the GitHub for Visual Studio 2017, then you need to open an issue in that project.

libgit2/libgit2sharp#1546

@grokys do you know anything about certificate validation callbacks?

[nk111]

Also discussed here:

#1532

[nk111]

I think, what's needed is kind of a dialog which lets the user decide if a remote certificate is trusted if validation fails...

https://msdn.microsoft.com/de-de/library/system.net.security.remotecertificatevalidationcallback%28v=vs.110%29.aspx?f=255&MSPPError=-2147217396

https://github.com/libgit2/libgit2sharp/blob/6854a1786fd77e5246a0f688a8a66c46c4ea5d3a/LibGit2Sharp.Tests/desktop/SmartSubtransportFixture.cs

Maybe win7 workstations like mine might have a problem using TLS 1.2 required by github?

[nk111]

and check this out. There is a possible fix mentioned:

rust-lang/cargo#5066

[bchavez]

Hi @nk111,

Thanks for that Rust/Cargo link! Read through the entire thread and finally got it to work!

Finally.

The important steps for me were:

• Ensure KB3140245 is installed.
• Ensure the EasyFix is applied.
• Also set DefaultSecureProtocols to 0x00000800:

HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\CurrentVersion\Internet Settings\WinHttp
HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\Microsoft\Windows\CurrentVersion\Internet Settings\WinHttp

Thanks a bunch! That works!

🎻 🌴 "'Cause it's a bittersweet symphony, this life..."

[jcansdale]

@bchavez @nk111 thanks for your research one this! It's great to have a confirmed fix. I'll add that to the description at the top.

@ethomson any chance you could give us some pointers on how to fix using using Libgit2Sharp?

[ethomson]

@jcansdale Should be fixed in the 0.25 prerelease builds... we should have an 0.25 release out directly

[jcansdale]

@ethomson Excellent, thanks for the heads up! How soon do you think we should push a 0.25 version? How stable are the current 0.25 prerelease builds?

[meaghanlewis]

Closing this issue out since a solution has been found for this scenario.