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

Issues migrating WCF Client from .NET Standard to MultiTarget #5499

Open
bwilliams1 opened this issue Apr 12, 2024 · 11 comments
Open

Issues migrating WCF Client from .NET Standard to MultiTarget #5499

bwilliams1 opened this issue Apr 12, 2024 · 11 comments

Comments

@bwilliams1
Copy link

Describe the bug
Over the last couple years we've migrated all our SOAP clients to Connected Services WCF Client on .NET standard 2.0 referencing the System.ServiceModel.* 4.. NuGet packages for consumption in a .NET framework 4.8 solution with a long term goal of targeting the latest versions of .NET (8 and beyond). I recently stumbled upon the drop of support of .NET standard 2.0 for WCF Client and started to understand what that means for our team. The docs linked in the previous sentence lists multi targeting as the solution.

I started noticing some issues with the migration to a multi target solution.

To Reproduce
Steps to reproduce the behavior:

  1. Create a .net standard 2.0 library project.
  2. Add a Connected Services WCF Service Client using the svc-util "wizard" within Visual Studio 2022 Version 17.8.7
  3. This will Generate a Reference.cs file with an implementation for CloseAsync()
  4. update <TargetFramework>netstandard2.0</TargetFramework> to <TargetFrameworks>net48;net8.0</TargetFrameworks>
  5. replace all the System.ServiceModel PackageReference nodes with conditional statements as seen below:
	<ItemGroup Condition=" '$(TargetFramework)' == 'net48' ">
		<Reference Include="System.ServiceModel" />
	</ItemGroup>
	<ItemGroup Condition=" '$(TargetFramework)' == 'net8.0' ">
		<PackageReference Include="System.ServiceModel.Primitives" Version="8.0.0" />
		<PackageReference Include="System.ServiceModel.Http" Version="8.0.0" />
	</ItemGroup>
  1. build the solution
  2. observe CS1061 build failures 'XXXClient' does not contain a definition for 'CloseAsync' and no accessible extension method 'CloseAsync' accepting a first argument of type 'XXXClient' could be found (are you missing a using directive or an assembly reference?)

Expected behavior
given the Microsoft documentation to simply "multi target" any .NET standard 2.0 projects, I expected a successful build.

Screenshots
n/a

Additional context

I also noticed if I create a brand new .net8.0 class library and try to add a WCF Client, it's still using the 4.10.* version of the System.ServiceModel.* instead of the 8.0.0 versions. In this scenario, it does not generate the CloseAsync methods that failed the build in the multi target scenario.

I also noticed that while using the multi target approach, if I try to create an instance of the XXXClient CloseAsync is an available method in .net4.8 and is not available in .net8.0. I could not find any documentation mentioning this breaking change.

var client = new XXXServiceClient(new BasicHttpBinding(), new EndpointAddress(@"https://foo/foo.svc"));

client.CloseAsync();
@bwilliams1
Copy link
Author

bwilliams1 commented Apr 12, 2024

on Visual Studio Version 17.9.6, the behavior is the same except that adding a WCF Client to a net8.0 project generates version 6.0.0 of the ServiceModel.* projects, however it does not replace System.ServiceModel.Duplex and System.ServiceModel.Security with System.ServiceModel.Primitives nor target the latest version which is 8.0.0

@mconnew
Copy link
Member

mconnew commented Apr 12, 2024

The client generated by dotnet-svcutil (which is the implementation for WCF Connected Services) generates a class which derives from ClientBase<TChannel>. In earlier versions of the WCF Client, the api was a subset of .NET Framework, which meant that ClientBase<TChannel> only had the APM methods for closing asynchronously (BeginClose/EndClose). At that time, we had dotnet-svcutil generate a CloseAsync method in the generated derived class which enabled writing cleaner code and you could just call await client.CloseAsync(). The generated method was a wrapper around Task.Factory.FromAsync which uses the APM methods to do the close. We've since added the CloseAsync method to the base class ClientBase<TChannel>, which then caused a problem for the generated code as it was implementing a method with the same name as the base class and you would get a compilation exception.

I think the reason you aren't seeing a CloseAsync method for net8.0 is that I believe dotnet-svcutil is generating conditional compilation code in the generated client where it provides a CloseAsync method for .NET Framework, but not .NET. If you update your WCF packages to the 8.0 version, you should see the CloseAsync method. When compiling for .NET Framework, it will be available in the generated code from dotnet-svcutil (conditionally compiled), and when compiling for net8.0, it should see the CloseAsync method implemented on the base ClientBase<TChannel> class. I don't know why it's still using the 4.10.* version of the packages, I suspect it has something to do with picking the version based on the TargetFramework and it's seeing the net462 (or whatever specific version you are using) and picking 4.10.* as that's the only one which works on .NET Framework. I suspect if you changed the order of the tfm's in TargetFrameworks to have net8.0 first that it would pick the later packages.

Having the 6.0.0 versions of System.ServiceModel.Duplex and System.ServiceModel.Security is to avoid breaking any libraries you are referencing which reference the older version (4.10.x) of the packages and haven't been recompiled against newer versions. They contain type forwarding facades which redirect all the types to Primitives. At runtime, if an older referenced library tries to load a type from System.ServiceModel.Duplex, then the runtime will load the type from Primitives as the facade contains the needed type forwards. I understand it can be annoying if doing development where you don't need it, but we took the approach that it was better to still reference it with the small cost (the facades are tiny) than the alternative which is unexpected runtime failures. You may have noticed we didn't release 8.0.0 versions of these packages.

@bwilliams1
Copy link
Author

What is the recommendation for a multi targeted implementation? Do I need to modify the generated code and add a #if NETFRAMEWORK?

#if NETFRAMEWORK
        public virtual System.Threading.Tasks.Task CloseAsync()
        {
            return System.Threading.Tasks.Task.Factory.FromAsync(((System.ServiceModel.ICommunicationObject)(this)).BeginClose(null, null), new System.Action<System.IAsyncResult>(((System.ServiceModel.ICommunicationObject)(this)).EndClose));
        }
#endif

@mjrousos
Copy link
Member

@bwilliams1 - I believe that should be conditional, yes. But in my experience, if I create the auto-generated client after my project is already multi-targeting .NET Framework and .NET Core/Standard, that method is generated with the necessary #if around it already. Does that work in your case?

@bwilliams1
Copy link
Author

it appears to be generating the #if NETFRAMEWORK after upgrading visual studio to 17.9.6. I'm surprised it didn't work with 17.8.7 which was only 2 months old. The drop of support for .net standard 2.0 was done over a year ago.

 #if NETFRAMEWORK
 public virtual System.Threading.Tasks.Task CloseAsync()
 {
     return System.Threading.Tasks.Task.Factory.FromAsync(((System.ServiceModel.ICommunicationObject)(this)).BeginClose(null, null), new System.Action<System.IAsyncResult>(((System.ServiceModel.ICommunicationObject)(this)).EndClose));
 }
 #endif

is the recommendation to regenerate all the bindings for projects "upgrading" from net standard 2.0 to multi target? Is there a command line option to iterate through all the bindings? we have about 35 in one project. or should we manually add the #if NETFRAMEWORK.

@bwilliams1
Copy link
Author

I did notice that after installing the new Reference.cs, it dumped the 6.0 references in the csproj, even though I already had the 8.0 versions.

Before:

<?xml version="1.0" encoding="utf-8"?>
<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFrameworks>net8.0;net48</TargetFrameworks>
  </PropertyGroup>
  <ItemGroup Condition=" '$(TargetFramework)' == 'net48' ">
    <Reference Include="System.ServiceModel" />
  </ItemGroup>
  <ItemGroup Condition=" '$(TargetFramework)' == 'net8.0' ">
    <PackageReference Include="System.ServiceModel.Primitives" Version="8.0.0" />
    <PackageReference Include="System.ServiceModel.Http" Version="8.0.0" />
  </ItemGroup>
 
</Project>

After

<?xml version="1.0" encoding="utf-8"?>
<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFrameworks>net8.0;net48</TargetFrameworks>
  </PropertyGroup>
  <ItemGroup Condition=" '$(TargetFramework)' == 'net48' ">
    <Reference Include="System.ServiceModel" />
  </ItemGroup>
  <ItemGroup Condition=" '$(TargetFramework)' == 'net8.0' ">
    <PackageReference Include="System.ServiceModel.Primitives" Version="8.0.0" />
    <PackageReference Include="System.ServiceModel.Http" Version="8.0.0" />
  </ItemGroup>
  <ItemGroup Condition="'$(TargetFramework)' != 'net48'">
    <PackageReference Include="System.ServiceModel.Duplex" Version="6.0.*" />
    <PackageReference Include="System.ServiceModel.NetTcp" Version="6.0.*" />
    <PackageReference Include="System.ServiceModel.Security" Version="6.0.*" />
    <PackageReference Include="System.ServiceModel.Federation" Version="6.0.*" />
  </ItemGroup>
</Project>

@bwilliams1
Copy link
Author

I did notice that Visual Studio is using a newer version of Svcutil now.

Before:

  [System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Tools.ServiceModel.Svcutil", "2.1.0")]

After:

[System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Tools.ServiceModel.Svcutil", "2.2.0-preview1.23462.5")]

@mjrousos
Copy link
Member

@bwilliams1 - yes, the current version of dotnet-svcutil uses 6.0 packages but 8.0 will be a bit more up-to-date. I expect either should work fine for .NET Framework upgrade scenarios (6.0 is still in support and 8.0 doesn't add anything that you would need coming from NetFx), but if you want the latest you will want to update to 8.0 after running this tool. This shouldn't be necessary once dotnet-svcutil updates.

@mconnew
Copy link
Member

mconnew commented Apr 17, 2024

Just an FYI/small tip, I usually do the conditional referencing like this as it works for multiple .NET Framework versions and isn't fragile to .NET version updates, e.g. when .NET 9 is released, your conditional referencing will need to be modified if you start targeting it.

       <ItemGroup Condition=" $(TargetFramework.StartsWith('net4')) ">
              <Reference Include="System.ServiceModel" />
       </ItemGroup>
       <ItemGroup Condition=" !$(TargetFramework.StartsWith('net4')) ">
              <PackageReference Include="System.ServiceModel.Primitives" Version="8.0.0" />
              <PackageReference Include="System.ServiceModel.Http" Version="8.0.0" />
       </ItemGroup>

@HongGit HongGit assigned imcarolwang and unassigned imcarolwang Apr 18, 2024
@bwilliams1
Copy link
Author

Just an FYI/small tip, I usually do the conditional referencing like this as it works for multiple .NET Framework versions and isn't fragile to .NET version updates, e.g. when .NET 9 is released, your conditional referencing will need to be modified if you start targeting it.

       <ItemGroup Condition=" $(TargetFramework.StartsWith('net4')) ">
              <Reference Include="System.ServiceModel" />
       </ItemGroup>
       <ItemGroup Condition=" !$(TargetFramework.StartsWith('net4')) ">
              <PackageReference Include="System.ServiceModel.Primitives" Version="8.0.0" />
              <PackageReference Include="System.ServiceModel.Http" Version="8.0.0" />
       </ItemGroup>

thanks! At first I was looking for generic monikers like we have for preprocessor symbols used in #if directives like NETFRAMEWORK and NET. When those failed I jumped to "make it work" mode and ended up with these.

does this mean Microsoft promises to skip .NET 40.0? otherwise $(TargetFramework.StartsWith('net4')) will match both net48 and net40.0

@bwilliams1
Copy link
Author

@mconnew @mjrousos this might have gotten lost above so I'll re-ask it:

is the recommendation to regenerate all the bindings for projects "upgrading" from net standard 2.0 to multi target? Is there a command line option to iterate through all the bindings? we have about 35 in one project.

or is the recommendation to manually modify the existing Reference.cs and wrap CloseAsync with #if NETFRAMEWORK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants