Update OS version detection to get version directly from Windows

[JamesNK]

Addresses https://github.com/grpc/grpc-dotnet/pull/2229/files#r1288695799

Environment.OSVersion.Version is a problem on older versions of .NET. It reports the OS based on the compatibility settings of an app, not the real version. This isn't a problem in our unit tests because there isn't a compatibility setting, but will be a problem when Grpc.Net.Client is used in a real-world client app.

For example, an app is installed on Windows 11 but is configured to be compatible with Windows 10. The version reported is Windows 10.

Fix this by getting the correct version directly from Windows. There are two options for doing this: PInvoke Windows API or read from the registry. Using PInvoke as it's simpler.

This issue is fixed in later versions of .NET (5+). Continue to use Environment.OSVersion.Version in .NET 6 and later.

[jtattermusch]

Added a comment

There's also a test failure:

Failed DetectWindowsVersion_Windows_MatchesEnvironment [1 ms]
  Error Message:
   Invalid platform name: Windows

[jtattermusch]

@JamesNK @jskeet since the correct way of getting the OSVersion (needed for evaluating WinHttpHandler compatibility) is getting much more complicated than simply calling Environment.OSVersion.Version, we should revisit the idea of Grpc.Net.Client providing some kind of API to tell us about the WinHttpHandler degree of support on the current system.
FWIW The API can be "unofficial / experts only", but it should be stable nevertheless.
Would it be acceptable to have an API that can only be accessed via reflection (i.e. not publicly exposed) but would have stability guarantees? Grpc.Net.Client needs to contain the detection code anyway and it would be easier for everyone if the detection mechanism would be same for everyone who uses it and also lived in a codebase that has microsoft engineers as maintainers. As you can see, the code we currently have is borderline a hack that that should be only attempted by those who can easily consult its correctness with area experts.

[jskeet]

I'd really prefer not to require reflection to access this - I'm happy with anything public even with a scary sounding "don't use this unless you're absolutely sure you need to" name. But having said that, I'd far rather require reflection than not have it at all, of course :)

[JamesNK]

Grpc.Net.Client providing some kind of API to tell us about the WinHttpHandler degree of support on the current system.
FWIW The API can be "unofficial / experts only", but it should be stable nevertheless.

I don't think it should be in the Grpc.Net.Client package. I don't like the idea of encouraging private reflection.

It could be in an unofficial community package. Or even just a gist. Requiring PInvoke adds to the complexity, but it can still be encapsulated in one file with less than 100 lines.

[jtattermusch]

Grpc.Net.Client providing some kind of API to tell us about the WinHttpHandler degree of support on the current system.
FWIW The API can be "unofficial / experts only", but it should be stable nevertheless.

I don't think it should be in the Grpc.Net.Client package. I don't like the idea of encouraging private reflection.

It could be in an unofficial community package. Or even just a gist. Requiring PInvoke adds to the complexity, but it can still be encapsulated in one file with less than 100 lines.

Regardless of the solution if feels important that there is an authoritative version of the windows version detection code, ideally also together with the logic that translates windows version into its semantic meaning ("full support", "partial support", "no support") for the purposes of using WinHttpHandler. This authoritative version should be maintained by grpc-dotnet team and it should be actually used by Grpc.Net.Client and it needs to be easy to use it in other projects (regardless whether the solution is "use this nuget" or "copy this .cs file into your project and we promise it will work")

Possible solution 1:

• all the logic required is in a single .cs file that lives in Grpc.Net.Client and the file level comment encourages downstream projects to actually copy the file verbatim (while we make some effort to make sure it actually works) and keep the copy of that file in sync with the original (if there are ever any changes).
• IMHO this solution feels dirty and adhoc.

Possible solution 2:

• Create a nuget package with windows version detection code and the related logic for its winhttphandler semantics.
  Grpc.Net.Client will depend on this package and use its logic as needed when targetFramework == net462 (if one wants to force using WinHttpHandler on NET 5+, they can do it but it's their responsibility to ensure that works). Client libraries can depend on the nuget package as well and make sure their detection logic is always consistent with whatever Grpc.Net.Client does.
• The package could be named sth like Grpc.Net.Client.WinHttpHandler.PlatformDetection

[mgravell]

I'm fine with the P/Invoke option, but please properly consider the other perspectives / approaches suggested

[JamesNK]

• Create a nuget package with windows version detection code and the related logic for its winhttphandler semantics.
  Grpc.Net.Client will depend on this package and use its logic as needed when targetFramework == net462 (if one wants to force using WinHttpHandler on NET 5+, they can do it but it's their responsibility to ensure that works). Client libraries can depend on the nuget package as well and make sure their detection logic is always consistent with whatever Grpc.Net.Client does.
• The package could be named sth like Grpc.Net.Client.WinHttpHandler.PlatformDetection

I like to limit to the minimum number of NuGet dependencies possible. Including 100 lines of code in Grpc.Net.Client is a better alternative than the extra effort and overhead of an extra NuGet package and dependency.

Remember that detection is built into Grpc.Net.Client. I just don't think many people also want the ability to check for this manually. If they do, then provide them with a 100-line helper class. Go with an ad-hoc approach until it is shown that more people want this feature and the ad-hoc approach has problems.

[JamesNK]

I created an issue for a creating a sample that includes detection code: #2242