Azure / autorest.csharp

Extension for AutoRest (https://github.com/Azure/autorest) that generates C# code
MIT License
142 stars 166 forks source link

Cannot convert from 'System.Net.Http.HttpClient' to 'System.Net.Http.DelegatingHandler' #166

Open joshuataylor opened 6 years ago

joshuataylor commented 6 years ago

Having the following issue when trying to build:

2018-09-27T09:03:16.5416142Z FoobarAPI.cs(48,82): error CS1503: Argument 1: cannot convert from 'System.Net.Http.HttpClient' to 'System.Net.Http.DelegatingHandler' [/home/deploy/myagent/_work/42/s/Foobar.API/Foobar.API.csproj]
2018-09-27T09:03:16.5441669Z FoobarAPI.cs(48,94): error CS1503: Argument 2: cannot convert from 'bool' to 'System.Net.Http.DelegatingHandler' [/home/deploy/myagent/_work/42/s/Foobar.API/Foobar.API.csproj]

My build steps:

npm install -g autorest@preview
autorest --reset

autorest --input-file=swagger.json --output-folder=Foobar.API --csharp --preview --namespace=Foobar.API

Line 48:

        /// <summary>
        /// Initializes a new instance of the FoobarAPI class.
        /// </summary>
        /// <param name='httpClient'>
        /// HttpClient to be used
        /// </param>
        /// <param name='disposeHttpClient'>
        /// True: will dispose the provided httpClient on calling FoobarAPI.Dispose(). False: will not dispose provided httpClient</param>
        public FoobarAPI(HttpClient httpClient, bool disposeHttpClient) : base(httpClient, disposeHttpClient)
        {
            Initialize();
        }

ASP.Net Core 2.1

.NET Core SDK (reflecting any global.json):
 Version:   2.1.402
 Commit:    3599f217f4

Runtime Environment:
 OS Name:     ubuntu
 OS Version:  18.04
 OS Platform: Linux
 RID:         ubuntu.18.04-x64
 Base Path:   /usr/share/dotnet/sdk/2.1.402/

Host (useful for support):
  Version: 2.1.4
  Commit:  85255dde3e

.NET Core SDKs installed:
  2.1.402 [/usr/share/dotnet/sdk]

.NET Core runtimes installed:
  Microsoft.AspNetCore.All 2.1.4 [/usr/share/dotnet/shared/Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.4 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.1.4 [/usr/share/dotnet/shared/Microsoft.NETCore.App]

To install additional .NET Core runtimes or SDKs:
  https://aka.ms/dotnet-download
jschweda commented 6 years ago

Same issue here: .net core 2.1 Microsoft.Rest.Clientruntime 2.3.3 AutoRest [version: 2.0.4283; node: v10.6.0] AutoRest core (2.0.4289) microsoft.azure/autorest.csharp, 2.3.82 microsoft.azure/autorest.modeler 2.3.55

jschweda commented 6 years ago

Sorry updating Microsoft.Rest.ClientRuntime manually to 2.3.14 fixed this for me.

SeeSherpa commented 6 years ago

I'm hitting the same problem, but I don't have the option of upgrading the Microsoft.Rest.ClientRuntime version, since it's only compatible with .net core.

It started after I upgraded from Autorest 2.0.4166 to 2.0.4283

For now I'm manually removing the new constructor since it wasn't being used earlier anyway, however, I would have preferred if Autorest could have had a flag for disabling the generation of this constructor for legacy clients.

fearthecowboy commented 6 years ago

@SeeSherpa I'm hitting the same problem, but I don't have the option of upgrading the Microsoft.Rest.ClientRuntime version, since it's only compatible with .net core.

Are you sure about that? @shahabhijeet -- that's not true, is it? The latest runtime should work with the desktop .NET, right?

SeeSherpa commented 6 years ago

@fearthecowboy, yup.

Here's what I get when I upgrade and run dotnet restore:

error NU1202: Package Microsoft.Rest.ClientRuntime 2.3.10 is not compatible with net45 (.NETFramework,Version=v4.5). Package Microsoft.Rest.ClientRuntime 2.3.10 supports:

error NU1202: - net452 (.NETFramework,Version=v4.5.2) error NU1202: - netstandard1.4 (.NETStandard,Version=v1.4)

fearthecowboy commented 6 years ago

Hmm. ... net45 .. I suspect that it's net46 or greater.

This is one of those things that make me not like runtimes :D ... the new c# generator will insert generated code right into the target project, no dependencies.

SeeSherpa commented 6 years ago

the new c# generator will insert generated code right into the target project, no dependencies.

@fearthecowboy Sorry I don't understand what you mean by 'the new c# generator". Are you talking about some newer architecture in the coming future, or are you saying you've made a fix for the 'System.Net.Http.HttpClient' to 'System.Net.Http.DelegatingHandler' issue?

If not the latter, would it be possible to have a fix for this? A coworker is using this issue as a reason to move to something called RestEase instead, and I'm trying to convince them otherwise.

Removing the offending constructor manually is just a temporary solution.

fearthecowboy commented 6 years ago

I'm referring to a new generator that I'm building that works fundamentally different.

Right now, the low-level side of it is working, and I've built the new PowerShell generator on top of that. Once that's done, I'll be looping back to making an Consumer SDK generator on top of that which will eventually replace the current C# generator.

The only hack I can offer you at the moment, is to add a configuration directive in a readme.md file that will do a search/replace to remove the offending code


``` yaml $(csharp)

directive: 
  - reason: remove constructor that I don't want.
    from: source-file-csharp
    where: $
    transform: > 
      return $.
        // exchange line endings for a marker
        replace(/\r?\n/g, '«'). 
        // replace double marker for a break+marker
        replace(/««/g, '\r\n«').
        // remove the constructors that refer to HttpClient
        replace(/«\s*\/\/\/.*?FoobarAPI\(.*?HttpClient httpClient.*?\).*/gi, '').
        // restore line endings
        replace(/«/g, '\r\n') 
```
SeeSherpa commented 6 years ago

I'm referring to a new generator that I'm building that works fundamentally different.

Oh that seems like a long ways away. Wouldn't that hit the same problem though?

The only hack I can offer you at the moment

Thanks for the suggestion. I'll look into it but it's going to be a tough sell.

Do you know why we are generating the new constructor now?

fearthecowboy commented 6 years ago

Oh that seems like a long ways away. Wouldn't that hit the same problem though?

Pretty sure that the problem here isn't that the code doesn't work on net45. The problem here is that nuget doesn't know that the code can work on net45 -- the build process changed to support net452 and netstandard 1.4

Do you know why we are generating the new constructor now?

We generate the new constructor to allow people to pass in an HttpClient instead of forcing the creation of a new one.

The build probably changed since net45 is out-of-service now. You really shouldn't be using net45 -- you should at least be on net452.

Thanks for the suggestion. I'll look into it but it's going to be a tough sell.

Huh? Telling the generator to drop those methods before writing it to disk is a tough sell? It's literally the least impactful thing you can do.

The only other workaround, is grab the source for the client runtime, and recompile to net45 yourself.

SeeSherpa commented 6 years ago

The build probably changed since net45 is out-of-service now. You really shouldn't be using net45 -- you should at least be on net452.

We compile for the latest target frameworks as well :) We just have to support our legacy devices and upgrading dotnet on some of them isn't possible due to certain limitations.

We generate the new constructor to allow people to pass in an HttpClient instead of forcing the creation of a new one.

:) Yeah, I remember I had to create a partial class with my own constructor so I could do just that. Only we would manage the lifetime of the client elsewhere. This seems better. Good stuff.

Huh? Telling the generator to drop those methods before writing it to disk is a tough sell? It's literally the least impactful thing you can do.

Sorry, I must have misunderstood, I thought you were suggesting something different. I've only even seen readme.md files used just for documentation? eg: https://github.com/Azure/autorest/blob/master/README.md/.

Could you please elaborate how I'd need to do this? I tried looking the term configuration directive up online but I'm not getting any useful results

fearthecowboy commented 6 years ago

Ok,

So you don't have to pass all of the command-line arguments on the actual command-line.

You can create a configuration file for a swagger file that lets you store the settings to generate with.

AutoRest uses a configuration file format we call "Literate Configuration" -- which is a markdown file with code blocks. This allows us to have strong documentation in the same file as the configuration for generating an SDK.

Example: https://raw.githubusercontent.com/Azure/autorest/master/Samples/1a-code-generation-minimal/readme.md

This allows you to set all the stuff on the command-line, in a file, so you don't need a batch file.

There are a ton of things you can do with configuration, most of which is shown in Samples

You can also look at the original docs for literate configuration

There are hundreds of examples in the Azure SDK Specs repo too

skrusty commented 5 years ago

i already have a directive which allows us to use generic lists, how do i add another directive for the code replacement? We have customers who require we stay on 4.5 at the moment, so need to implement your fix. Thanks in advance.

jonmeyerson commented 5 years ago

i hit this problem after updating my autorest version, just add a --version parameter the version you were using before.

pedrommateus92 commented 5 years ago

I have the same problem like you, after updated the Autorest version. Did you find way to fix it?

ddanda commented 5 years ago

I am using Net Standard 2.0 with Microsoft.Rest.ClientRuntime 2.3.19. I was able to successfully build with this configuration.

jimmywim commented 5 years ago

Commenting for benefit of other users after googling this error in dotnet core 2.2...

When you add the Microsoft.Rest.ClientRuntime package, make sure you specify the version just as the nuget page tells you to on the .NET CLI tab. If you omit, it installs V3.something which produces this error.

Run the command as suggested by the .NET CLI tab on the Nuget page for the package:

dotnet add package Microsoft.Rest.ClientRuntime --version 2.3.20

That version works for me, the project with the autorest generated client now builds.

ScottyMac-dev commented 4 years ago

I am getting this error in .NET Core 3.1 using AutoRest 2.0.4413.

rollingage commented 4 years ago

Sorry updating Microsoft.Rest.ClientRuntime manually to 2.3.14 fixed this for me.

This works for me. I downgrade Microsoft.Rest.ClientRuntime manually to 2.3.14 from latest 2.3.22, it works.